Merge #342

342: Handle renamed dependencies r=ordian a=stiiifff

Fixes #273 

Co-authored-by: Steve Degosserie <[email protected]>
Co-authored-by: Steve Degosserie <[email protected]>

src/bin/upgrade/main.rs

@@ -160,7 +160,13 @@ impl Manifests {
                 .iter()
                 .flat_map(|&(_, ref package)| package.dependencies.clone())
                 .filter(is_version_dep)
-                .map(|dependency| (dependency.name, None))
+                .map(|dependency| {
+                    let mut dep = Dependency::new(&dependency.name);
+                    if let Some(rename) = dependency.rename {
+                        dep = dep.set_rename(&rename);
+                    }
+                    (dep, None)
+                })
                 .collect()
         } else {
             only_update
@@ -174,6 +180,7 @@ impl Manifests {
                     } else {
                         Ok((name, None))
                     }
+                    .map(move |(name, version)| (Dependency::new(&name), version))
                 })
                 .collect::<Result<_>>()?
         }))
@@ -202,8 +209,12 @@ impl Manifests {
         for (mut manifest, package) in self.0 {
             println!("{}:", package.name);
 
-            for (name, version) in &upgraded_deps.0 {
-                manifest.upgrade(&Dependency::new(name).set_version(version), dry_run)?;
+            for (dep, version) in &upgraded_deps.0 {
+                let mut new_dep = Dependency::new(&dep.name).set_version(version);
+                if let Some(rename) = dep.rename() {
+                    new_dep = new_dep.set_rename(&rename);
+                }
+                manifest.upgrade(&new_dep, dry_run)?;
             }
         }
 
@@ -212,26 +223,26 @@ impl Manifests {
 }
 
 /// The set of dependencies to be upgraded, alongside desired versions, if specified by the user.
-struct DesiredUpgrades(HashMap<String, Option<String>>);
+struct DesiredUpgrades(HashMap<Dependency, Option<String>>);
 
 /// The complete specification of the upgrades that will be performed. Map of the dependency names
 /// to the new versions.
-struct ActualUpgrades(HashMap<String, String>);
+struct ActualUpgrades(HashMap<Dependency, String>);
 
 impl DesiredUpgrades {
     /// Transform the dependencies into their upgraded forms. If a version is specified, all
     /// dependencies will get that version.
     fn get_upgraded(self, allow_prerelease: bool, manifest_path: &Path) -> Result<ActualUpgrades> {
         self.0
             .into_iter()
-            .map(|(name, version)| {
+            .map(|(dep, version)| {
                 if let Some(v) = version {
-                    Ok((name, v))
+                    Ok((dep, v))
                 } else {
-                    get_latest_dependency(&name, allow_prerelease, manifest_path)
+                    get_latest_dependency(&dep.name, allow_prerelease, manifest_path)
                         .map(|new_dep| {
                             (
-                                name,
+                                dep,
                                 new_dep
                                     .version()
                                     .expect("Invalid dependency type")

src/dependency.rs

@@ -17,12 +17,16 @@ pub struct Dependency {
     optional: bool,
     default_features: bool,
     source: DependencySource,
+    /// If the dependency is renamed, this is the new name for the dependency
+    /// as a string.  None if it is not renamed.
+    rename: Option<String>,
 }
 
 impl Default for Dependency {
     fn default() -> Dependency {
         Dependency {
             name: "".into(),
+            rename: None,
             optional: false,
             default_features: true,
             source: DependencySource::Version {
@@ -92,6 +96,19 @@ impl Dependency {
         self
     }
 
+    /// Set the alias for the dependency
+    pub fn set_rename(mut self, rename: &str) -> Dependency {
+        self.rename = Some(rename.into());
+        self
+    }
+
+    /// Get the dependency name as defined in the manifest,
+    /// that is, either the alias (rename field if Some),
+    /// or the official package name (name field).
+    pub fn name_in_manifest(&self) -> &str {
+        &self.rename().unwrap_or(&self.name)
+    }
+
     /// Get version of dependency
     pub fn version(&self) -> Option<&str> {
         if let DependencySource::Version {
@@ -105,53 +122,166 @@ impl Dependency {
         }
     }
 
+    /// Get the alias for the dependency (if any)
+    pub fn rename(&self) -> Option<&str> {
+        match &self.rename {
+            Some(rename) => Some(&rename),
+            None => None,
+        }
+    }
+
     /// Convert dependency to TOML
     ///
     /// Returns a tuple with the dependency's name and either the version as a `String`
     /// or the path/git repository as an `InlineTable`.
     /// (If the dependency is set as `optional` or `default-features` is set to `false`,
     /// an `InlineTable` is returned in any case.)
     pub fn to_toml(&self) -> (String, toml_edit::Item) {
-        let data: toml_edit::Item =
-            match (self.optional, self.default_features, self.source.clone()) {
-                // Extra short when version flag only
-                (
-                    false,
-                    true,
-                    DependencySource::Version {
-                        version: Some(v),
-                        path: None,
-                    },
-                ) => toml_edit::value(v),
-                // Other cases are represented as an inline table
-                (optional, default_features, source) => {
-                    let mut data = toml_edit::InlineTable::default();
-
-                    match source {
-                        DependencySource::Version { version, path } => {
-                            if let Some(v) = version {
-                                data.get_or_insert("version", v);
-                            }
-                            if let Some(p) = path {
-                                data.get_or_insert("path", p);
-                            }
+        let data: toml_edit::Item = match (
+            self.optional,
+            self.default_features,
+            self.source.clone(),
+            self.rename.as_ref(),
+        ) {
+            // Extra short when version flag only
+            (
+                false,
+                true,
+                DependencySource::Version {
+                    version: Some(v),
+                    path: None,
+                },
+                None,
+            ) => toml_edit::value(v),
+            // Other cases are represented as an inline table
+            (optional, default_features, source, rename) => {
+                let mut data = toml_edit::InlineTable::default();
+
+                match source {
+                    DependencySource::Version { version, path } => {
+                        if let Some(v) = version {
+                            data.get_or_insert("version", v);
                         }
-                        DependencySource::Git(v) => {
-                            data.get_or_insert("git", v);
+                        if let Some(p) = path {
+                            data.get_or_insert("path", p);
                         }
                     }
-                    if self.optional {
-                        data.get_or_insert("optional", optional);
+                    DependencySource::Git(v) => {
+                        data.get_or_insert("git", v);
                     }
-                    if !self.default_features {
-                        data.get_or_insert("default-features", default_features);
-                    }
-
-                    data.fmt();
-                    toml_edit::value(toml_edit::Value::InlineTable(data))
                 }
-            };
+                if self.optional {
+                    data.get_or_insert("optional", optional);
+                }
+                if !self.default_features {
+                    data.get_or_insert("default-features", default_features);
+                }
+                if rename.is_some() {
+                    data.get_or_insert("package", self.name.clone());
+                }
+
+                data.fmt();
+                toml_edit::value(toml_edit::Value::InlineTable(data))
+            }
+        };
+
+        (self.name_in_manifest().to_string(), data)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::dependency::Dependency;
+
+    #[test]
+    fn to_toml_simple_dep() {
+        let toml = Dependency::new("dep").to_toml();
+
+        assert_eq!(toml.0, "dep".to_owned());
+    }
+
+    #[test]
+    fn to_toml_simple_dep_with_version() {
+        let toml = Dependency::new("dep").set_version("1.0").to_toml();
+
+        assert_eq!(toml.0, "dep".to_owned());
+        assert_eq!(toml.1.as_str(), Some("1.0"));
+    }
+
+    #[test]
+    fn to_toml_optional_dep() {
+        let toml = Dependency::new("dep").set_optional(true).to_toml();
+
+        assert_eq!(toml.0, "dep".to_owned());
+        assert!(toml.1.is_inline_table());
+
+        let dep = toml.1.as_inline_table().unwrap();
+        assert_eq!(dep.get("optional").unwrap().as_bool(), Some(true));
+    }
+
+    #[test]
+    fn to_toml_dep_without_default_features() {
+        let toml = Dependency::new("dep").set_default_features(false).to_toml();
+
+        assert_eq!(toml.0, "dep".to_owned());
+        assert!(toml.1.is_inline_table());
+
+        let dep = toml.1.as_inline_table().unwrap();
+        assert_eq!(dep.get("default-features").unwrap().as_bool(), Some(false));
+    }
+
+    #[test]
+    fn to_toml_dep_with_path_source() {
+        let toml = Dependency::new("dep").set_path("~/foo/bar").to_toml();
+
+        assert_eq!(toml.0, "dep".to_owned());
+        assert!(toml.1.is_inline_table());
+
+        let dep = toml.1.as_inline_table().unwrap();
+        assert_eq!(dep.get("path").unwrap().as_str(), Some("~/foo/bar"));
+    }
+
+    #[test]
+    fn to_toml_dep_with_git_source() {
+        let toml = Dependency::new("dep")
+            .set_git("https://foor/bar.git")
+            .to_toml();
+
+        assert_eq!(toml.0, "dep".to_owned());
+        assert!(toml.1.is_inline_table());
+
+        let dep = toml.1.as_inline_table().unwrap();
+        assert_eq!(
+            dep.get("git").unwrap().as_str(),
+            Some("https://foor/bar.git")
+        );
+    }
+
+    #[test]
+    fn to_toml_renamed_dep() {
+        let toml = Dependency::new("dep").set_rename("d").to_toml();
+
+        assert_eq!(toml.0, "d".to_owned());
+        assert!(toml.1.is_inline_table());
+
+        let dep = toml.1.as_inline_table().unwrap();
+        assert_eq!(dep.get("package").unwrap().as_str(), Some("dep"));
+    }
+
+    #[test]
+    fn to_toml_complex_dep() {
+        let toml = Dependency::new("dep")
+            .set_version("1.0")
+            .set_default_features(false)
+            .set_rename("d")
+            .to_toml();
+
+        assert_eq!(toml.0, "d".to_owned());
+        assert!(toml.1.is_inline_table());
 
-        (self.name.clone(), data)
+        let dep = toml.1.as_inline_table().unwrap();
+        assert_eq!(dep.get("package").unwrap().as_str(), Some("dep"));
+        assert_eq!(dep.get("version").unwrap().as_str(), Some("1.0"));
+        assert_eq!(dep.get("default-features").unwrap().as_bool(), Some(false));
     }
 }

src/manifest.rs

@@ -272,17 +272,28 @@ impl Manifest {
         table_path: &[String],
         dep: &Dependency,
         dry_run: bool,
+    ) -> Result<()> {
+        self.update_table_named_entry(table_path, dep.name_in_manifest(), dep, dry_run)
+    }
+
+    /// Update an entry with a specified name in Cargo.toml.
+    pub fn update_table_named_entry(
+        &mut self,
+        table_path: &[String],
+        item_name: &str,
+        dep: &Dependency,
+        dry_run: bool,
     ) -> Result<()> {
         let table = self.get_table(table_path)?;
         let new_dep = dep.to_toml().1;
 
         // If (and only if) there is an old entry, merge the new one in.
-        if !table[&dep.name].is_none() {
-            if let Err(e) = print_upgrade_if_necessary(&dep.name, &table[&dep.name], &new_dep) {
+        if !table[item_name].is_none() {
+            if let Err(e) = print_upgrade_if_necessary(&dep.name, &table[item_name], &new_dep) {
                 eprintln!("Error while displaying upgrade message, {}", e);
             }
             if !dry_run {
-                merge_dependencies(&mut table[&dep.name], dep);
+                merge_dependencies(&mut table[item_name], dep);
                 if let Some(t) = table.as_inline_table_mut() {
                     t.fmt()
                 }
@@ -395,10 +406,18 @@ impl LocalManifest {
     pub fn upgrade(&mut self, dependency: &Dependency, dry_run: bool) -> Result<()> {
         for (table_path, table) in self.get_sections() {
             let table_like = table.as_table_like().expect("Unexpected non-table");
-            for (name, _old_value) in table_like.iter() {
-                if name == dependency.name {
-                    self.manifest
-                        .update_table_entry(&table_path, dependency, dry_run)?;
+            for (name, toml_item) in table_like.iter() {
+                let dep_name = toml_item
+                    .as_table_like()
+                    .and_then(|t| t.get("package").and_then(|p| p.as_str()))
+                    .unwrap_or(name);
+                if dep_name == dependency.name {
+                    self.manifest.update_table_named_entry(
+                        &table_path,
+                        &name,
+                        dependency,
+                        dry_run,
+                    )?;
                 }
             }
         }