From ae1aa6540761e54a76b8f7984cf93cd3a0d011d0 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sun, 3 May 2026 11:55:03 +0900 Subject: refactor: switch internal maps/sets from HashMap to IndexMap Adopt indexmap workspace-wide so iteration order is deterministic and follows insertion order. The non-deterministic order of std HashMap otherwise leaks into resolver decisions when multiple valid solutions exist (e.g. cyclic require pairs under prefer-lowest), making behavior flaky and divergent from Composer's PHP-array semantics. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/mozart/src/commands/update.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'crates/mozart/src/commands/update.rs') diff --git a/crates/mozart/src/commands/update.rs b/crates/mozart/src/commands/update.rs index 17b7c97..0c25a9e 100644 --- a/crates/mozart/src/commands/update.rs +++ b/crates/mozart/src/commands/update.rs @@ -1,10 +1,10 @@ use clap::Args; +use indexmap::{IndexMap, IndexSet}; use mozart_core::console; use mozart_core::console_format; use mozart_core::package::{self, Stability}; use mozart_registry::lockfile; use mozart_registry::resolver::{self, PlatformConfig, ResolveRequest, ResolvedPackage}; -use std::collections::{HashMap, HashSet}; #[derive(Args)] pub struct UpdateArgs { @@ -198,7 +198,7 @@ pub fn compute_update_changes( dev_mode: bool, ) -> Vec { // Build map of old lock packages keyed by lowercase name -> version string - let mut old_map: HashMap = HashMap::new(); + let mut old_map: IndexMap = IndexMap::new(); if let Some(old) = old_lock { for pkg in &old.packages { old_map.insert(pkg.name.to_lowercase(), pkg.version.clone()); @@ -211,7 +211,7 @@ pub fn compute_update_changes( } // Build map of new lock packages keyed by lowercase name -> version string - let mut new_map: HashMap = HashMap::new(); + let mut new_map: IndexMap = IndexMap::new(); for pkg in &new_lock.packages { new_map.insert(pkg.name.to_lowercase(), pkg.version.clone()); } @@ -302,11 +302,11 @@ pub fn apply_partial_update( update_packages: &[String], ) -> Vec { // Build a set of normalized package names we want to update - let update_set: std::collections::HashSet = + let update_set: indexmap::IndexSet = update_packages.iter().map(|s| s.to_lowercase()).collect(); // Build a map of old locked packages by name -> (version, version_normalized, is_dev) - let mut old_pkg_map: HashMap = HashMap::new(); + let mut old_pkg_map: IndexMap = IndexMap::new(); for pkg in &old_lock.packages { old_pkg_map.insert(pkg.name.to_lowercase(), pkg); } @@ -414,7 +414,7 @@ pub fn expand_wildcards( .collect(); let mut result: Vec = Vec::new(); - let mut seen: HashSet = HashSet::new(); + let mut seen: IndexSet = IndexSet::new(); for spec in specifiers { if spec.contains('*') { @@ -448,8 +448,8 @@ pub fn expand_wildcards( // ───────────────────────────────────────────────────────────────────────────── /// Build a lookup map from package name (lowercase) to its LockedPackage. -fn build_lock_map(lock: &lockfile::LockFile) -> HashMap { - let mut map = HashMap::new(); +fn build_lock_map(lock: &lockfile::LockFile) -> IndexMap { + let mut map = IndexMap::new(); for pkg in &lock.packages { map.insert(pkg.name.to_lowercase(), pkg); } @@ -468,7 +468,7 @@ pub fn expand_with_direct_dependencies( lock: &lockfile::LockFile, ) -> Vec { let lock_map = build_lock_map(lock); - let mut result_set: HashSet = packages.iter().cloned().collect(); + let mut result_set: IndexSet = packages.iter().cloned().collect(); let mut result: Vec = packages; for name in result.clone() { @@ -503,7 +503,7 @@ pub fn expand_with_all_dependencies( lock: &lockfile::LockFile, ) -> Vec { let lock_map = build_lock_map(lock); - let mut result_set: HashSet = packages.iter().cloned().collect(); + let mut result_set: IndexSet = packages.iter().cloned().collect(); let mut queue: Vec = packages.clone(); let mut result: Vec = packages; @@ -665,7 +665,7 @@ pub fn apply_patch_only( resolved: Vec, old_lock: &lockfile::LockFile, ) -> Vec { - let mut old_pkg_map: HashMap = HashMap::new(); + let mut old_pkg_map: IndexMap = IndexMap::new(); for pkg in &old_lock.packages { old_pkg_map.insert(pkg.name.to_lowercase(), pkg); } @@ -806,7 +806,7 @@ pub async fn run( let dev_mode = !args.no_dev; // Fix 1C + Fix 2: Parse --with constraints and inline constraint shorthand. - let mut temporary_constraints: HashMap = HashMap::new(); + let mut temporary_constraints: IndexMap = IndexMap::new(); // Parse --with constraints (format: "vendor/package:constraint") for with_entry in &args.with { @@ -887,7 +887,7 @@ pub async fn run( require_dev, include_dev: dev_mode, minimum_stability, - stability_flags: HashMap::new(), + stability_flags: IndexMap::new(), prefer_stable, prefer_lowest: args.prefer_lowest, platform, @@ -1166,7 +1166,7 @@ pub async fn run( let bump_require_dev = mode == "all" || mode == "dev"; // Build locked versions map from the new lock - let mut locked_versions: HashMap)> = HashMap::new(); + let mut locked_versions: IndexMap)> = IndexMap::new(); for pkg in &new_lock.packages { locked_versions.insert( pkg.name.to_lowercase(), @@ -1997,7 +1997,7 @@ mod tests { require_dev: vec![], include_dev: false, minimum_stability: Stability::Stable, - stability_flags: HashMap::new(), + stability_flags: IndexMap::new(), prefer_stable: true, prefer_lowest: false, platform: PlatformConfig::new(), @@ -2011,10 +2011,10 @@ mod tests { ), ), ), - temporary_constraints: HashMap::new(), + temporary_constraints: IndexMap::new(), raw_repositories: vec![], - root_provide: HashMap::new(), - root_replace: HashMap::new(), + root_provide: IndexMap::new(), + root_replace: IndexMap::new(), }; let resolved = resolve(&request).await.expect("Resolution should succeed"); -- cgit v1.3.1