Simplify generate_dependency_changes

src/commands/init.rs

@@ -35,15 +35,20 @@ impl InitSubcommand {
 
         match fs_err::metadata(&manifest_path) {
             Ok(_) => bail!(
-                "There is already a Wally project in this directory. Manifest file ({}) already exists.",
+                "There is already a Wally project in this directory. Manifest file ({}) already \
+                 exists.",
                 MANIFEST_FILE_NAME
             ),
 
             Err(err) => {
                 if err.kind() == std::io::ErrorKind::NotFound {
                     // Perfect! This is the state that we want
                 } else {
-                    bail!("Error accessing manifest file ({}): {}", MANIFEST_FILE_NAME, err);
+                    bail!(
+                        "Error accessing manifest file ({}): {}",
+                        MANIFEST_FILE_NAME,
+                        err
+                    );
                 }
             }
         }

src/commands/install.rs

@@ -1,6 +1,5 @@
 use std::collections::BTreeSet;
 
-
 use std::io::Write;
 use std::path::PathBuf;
 use std::time::Duration;

src/commands/utils.rs

@@ -13,60 +13,47 @@ pub(crate) fn generate_dependency_changes(
     old_dependencies: &BTreeSet<PackageId>,
     new_dependencies: &BTreeSet<PackageId>,
 ) -> Vec<DependencyChange> {
-    let added_packages = new_dependencies.difference(old_dependencies);
-    let removed_packages = old_dependencies.difference(new_dependencies);
-    let changed_dependencies: BTreeSet<&PackageName> = added_packages
-        .clone()
-        .chain(removed_packages.clone())
-        .map(|package| package.name())
+    let changed_dependencies: BTreeSet<&PackageName> = old_dependencies
+        .symmetric_difference(new_dependencies)
+        .map(|changed_package| changed_package.name())
         .collect();
 
     let mut dependency = Vec::new();
 
-    for dependency_name in changed_dependencies {
-        let matching_packages_removed = removed_packages
-            .clone()
-            .filter(|x| *x.name() == *dependency_name);
-        let matching_packages_added = added_packages
-            .clone()
-            .filter(|x| *x.name() == *dependency_name);
+    for changed_dependency_name in changed_dependencies {
+        let match_package_ids_by_name =
+            |maybe_matching: &&PackageId| maybe_matching.name() == changed_dependency_name;
 
-        match (
-            matching_packages_added.clone().count(),
-            matching_packages_removed.clone().count(),
-        ) {
-            (1, 1) => {
-                // We know for certain that there is only one item in the iterator.
-                let package_added = matching_packages_added.last().unwrap();
-                let package_removed = matching_packages_removed.last().unwrap();
+        let mut old_matches = old_dependencies.iter().filter(match_package_ids_by_name);
+        let total_old_matches = old_matches.clone().count();
 
-                if package_added.gt(package_removed) {
-                    dependency.push(DependencyChange::Updated {
-                        from: package_removed.clone(),
-                        to: package_added.clone(),
-                    });
-                } else {
-                    dependency.push(DependencyChange::Downgrade {
-                        from: package_added.clone(),
-                        to: package_removed.clone(),
-                    });
-                }
-            }
-            (0, 1) => {
-                // We know for certain that there is only one item in the iterator.
-                let package_removed = matching_packages_removed.last().unwrap();
-                dependency.push(DependencyChange::Removed(package_removed.clone()));
-            }
-            (1, 0) => {
-                // We know for certain that there is only one item in the iterator.
-                let package_added = matching_packages_added.last().unwrap();
-                dependency.push(DependencyChange::Added(package_added.clone()));
-            }
-            (0, 0) => panic!("Impossible for the package name {} to not be removed or added if found in earlier.", dependency_name),
-            (_, _) => {
-                dependency.extend(matching_packages_added.map(|package| DependencyChange::Added(package.clone())));
-                dependency.extend(matching_packages_removed.map(|package| DependencyChange::Removed(package.clone())));
-            }
+        let mut new_matches = new_dependencies.iter().filter(match_package_ids_by_name);
+        let total_new_matches = new_matches.clone().count();
+
+        // If there's more than one new or old matches, then we do the simple route of listing the exact versions removed/added.
+        if total_new_matches > 1 || total_old_matches > 1 {
+            dependency
+                .extend(old_matches.map(|package| DependencyChange::Removed(package.clone())));
+            dependency.extend(new_matches.map(|package| DependencyChange::Added(package.clone())));
+        } else {
+            // Otherwise, we can try being more specific about what changed.
+            dependency.push(
+                match (old_matches.next().cloned(), new_matches.next().cloned()) {
+                    (Some(old), Some(new)) if old.le(&new) => {
+                        DependencyChange::Updated { from: old, to: new }
+                    }
+                    (Some(old), Some(new)) => DependencyChange::Downgrade { from: old, to: new },
+
+                    // Or, there's been a singular removal/addition.
+                    (Some(old), None) => DependencyChange::Removed(old),
+                    (None, Some(new)) => DependencyChange::Added(new),
+                    (None, None) => panic!(
+                        "Impossible for the package name {} to not be removed or added if found \
+                     in earlier.",
+                        changed_dependency_name
+                    ),
+                },
+            )
         }
     }
 

src/package_contents.rs

@@ -64,8 +64,8 @@ impl PackageContents {
                     if project_name != package_name {
                         log::info!(
                             "The project and package names are mismatched. The project name in \
-                            `default.project.json` has been renamed to '{}' in the uploaded package \
-                            to match the name provided by `wally.toml`",
+                             `default.project.json` has been renamed to '{}' in the uploaded \
+                             package to match the name provided by `wally.toml`",
                             package_name
                         );
 

src/package_name.rs

@@ -53,7 +53,8 @@ fn validate_scope(scope: &str) -> anyhow::Result<()> {
 
     ensure!(
         only_valid_chars,
-        "package scope '{}' is invalid (scopes can only contain lowercase characters, digits and '-')",
+        "package scope '{}' is invalid (scopes can only contain lowercase characters, digits and \
+         '-')",
         scope
     );
     ensure!(scope.len() > 0, "package scopes cannot be empty");
@@ -72,7 +73,8 @@ fn validate_name(name: &str) -> anyhow::Result<()> {
 
     ensure!(
         only_valid_chars,
-        "package name '{}' is invalid (names can only contain lowercase characters, digits and '-')",
+        "package name '{}' is invalid (names can only contain lowercase characters, digits and \
+         '-')",
         name
     );
     ensure!(name.len() > 0, "package names cannot be empty");

src/resolution.rs

@@ -273,8 +273,8 @@ pub fn resolve(
 
         if conflicting.is_empty() {
             bail!(
-                "No packages were found that matched ({req_realm:?}) {req}.\
-                \nAre you sure this is a {req_realm:?} dependency?",
+                "No packages were found that matched ({req_realm:?}) {req}.\nAre you sure this is \
+                 a {req_realm:?} dependency?",
                 req_realm = dependency_request.request_realm,
                 req = dependency_request.package_req,
             );
@@ -285,9 +285,9 @@ pub fn resolve(
                 .collect();
 
             bail!(
-                "All possible candidates for package {req} ({req_realm:?}) \
-                 conflicted with other packages that were already installed. \
-                 These packages were previously selected: {conflicting}",
+                "All possible candidates for package {req} ({req_realm:?}) conflicted with other \
+                 packages that were already installed. These packages were previously selected: \
+                 {conflicting}",
                 req = dependency_request.package_req,
                 req_realm = dependency_request.request_realm,
                 conflicting = conflicting_debug.join(", "),