feat(rm): Add --target flag (#711)

This included splitting `Manifest::get_table_mut` into two functions with inserting and erroring (non-inserting) variants to allow for nested lookup for `rm`

README.md

@@ -156,15 +156,18 @@ ARGS:
     <CRATE>...    Crates to be removed
 
 OPTIONS:
-    -B, --build                   Remove crate as build dependency
-    -D, --dev                     Remove crate as development dependency
     -h, --help                    Print help information
         --manifest-path <PATH>    Path to the manifest to remove a dependency from
     -p, --package <PKGID>         Package id of the crate to remove this dependency from
     -q, --quiet                   Do not print any output in case of success
     -V, --version                 Print version information
     -Z <FLAG>                     Unstable (nightly-only) flags
 
+SECTION:
+    -B, --build              Remove crate as build dependency
+    -D, --dev                Remove crate as development dependency
+        --target <TARGET>    Remove as dependency from the given target platform
+
 ```
 
 ### `cargo upgrade`

src/bin/add/add.rs

@@ -618,7 +618,7 @@ fn exec(mut args: AddArgs) -> CargoResult<()> {
 
     if was_sorted {
         if let Some(table) = manifest
-            .get_table_mut(&args.get_section())
+            .get_or_insert_table_mut(&args.get_section())
             .ok()
             .and_then(TomlItem::as_table_like_mut)
         {

src/bin/rm/rm.rs

@@ -15,13 +15,17 @@ pub struct RmArgs {
     crates: Vec<String>,
 
     /// Remove crate as development dependency.
-    #[clap(long, short = 'D', conflicts_with = "build")]
+    #[clap(long, short = 'D', conflicts_with = "build", help_heading = "SECTION")]
     dev: bool,
 
     /// Remove crate as build dependency.
-    #[clap(long, short = 'B', conflicts_with = "dev")]
+    #[clap(long, short = 'B', conflicts_with = "dev", help_heading = "SECTION")]
     build: bool,
 
+    /// Remove as dependency from the given target platform.
+    #[clap(long, forbid_empty_values = true, help_heading = "SECTION")]
+    target: Option<String>,
+
     /// Path to the manifest to remove a dependency from.
     #[clap(
         long,
@@ -54,27 +58,40 @@ impl RmArgs {
         exec(self)
     }
 
-    /// Get depenency section
-    pub fn get_section(&self) -> &'static str {
-        if self.dev {
+    /// Get dependency section
+    pub fn get_section(&self) -> Vec<String> {
+        let section_name = if self.dev {
             "dev-dependencies"
         } else if self.build {
             "build-dependencies"
         } else {
             "dependencies"
+        };
+
+        if let Some(ref target) = self.target {
+            assert!(!target.is_empty(), "Target specification may not be empty");
+
+            vec!["target".to_owned(), target.clone(), section_name.to_owned()]
+        } else {
+            vec![section_name.to_owned()]
         }
     }
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, clap::ArgEnum)]
 enum UnstableOptions {}
 
-fn print_msg(name: &str, section: &str) -> CargoResult<()> {
+fn print_msg(name: &str, section: &[String]) -> CargoResult<()> {
     let colorchoice = colorize_stderr();
     let mut output = StandardStream::stderr(colorchoice);
     output.set_color(ColorSpec::new().set_fg(Some(Color::Green)).set_bold(true))?;
     write!(output, "{:>12}", "Removing")?;
     output.reset()?;
+    let section = if section.len() == 1 {
+        section[0].clone()
+    } else {
+        format!("{} for target `{}`", &section[2], &section[1])
+    };
     writeln!(output, " {} from {}", name, section)?;
     Ok(())
 }
@@ -92,10 +109,10 @@ fn exec(args: &RmArgs) -> CargoResult<()> {
     deps.iter()
         .map(|dep| {
             if !args.quiet {
-                print_msg(dep, args.get_section())?;
+                print_msg(dep, &args.get_section())?;
             }
             let result = manifest
-                .remove_from_table(args.get_section(), dep)
+                .remove_from_table(&args.get_section(), dep)
                 .map_err(Into::into);
 
             // Now that we have removed the crate, if that was the last reference to that crate,

src/manifest.rs

@@ -1,5 +1,5 @@
 use std::collections::BTreeMap;
-use std::fs::{self};
+use std::fs;
 use std::io::Write;
 use std::ops::{Deref, DerefMut};
 use std::path::{Path, PathBuf};
@@ -57,29 +57,23 @@ impl Manifest {
     }
 
     /// Get the specified table from the manifest.
+    ///
+    /// If there is no table at the specified path, then a non-existent table
+    /// error will be returned.
     pub fn get_table_mut<'a>(
         &'a mut self,
         table_path: &[String],
     ) -> CargoResult<&'a mut toml_edit::Item> {
-        /// Descend into a manifest until the required table is found.
-        fn descend<'a>(
-            input: &'a mut toml_edit::Item,
-            path: &[String],
-        ) -> CargoResult<&'a mut toml_edit::Item> {
-            if let Some(segment) = path.get(0) {
-                let value = input[&segment].or_insert(toml_edit::table());
-
-                if value.is_table_like() {
-                    descend(value, &path[1..])
-                } else {
-                    Err(non_existent_table_err(segment))
-                }
-            } else {
-                Ok(input)
-            }
-        }
+        self.get_table_mut_internal(table_path, false)
+    }
 
-        descend(self.data.as_item_mut(), table_path)
+    /// Get the specified table from the manifest, inserting it if it does not
+    /// exist.
+    pub fn get_or_insert_table_mut<'a>(
+        &'a mut self,
+        table_path: &[String],
+    ) -> CargoResult<&'a mut toml_edit::Item> {
+        self.get_table_mut_internal(table_path, true)
     }
 
     /// Get all sections in the manifest that exist and might contain dependencies.
@@ -175,6 +169,39 @@ impl Manifest {
 
         Ok(features)
     }
+
+    fn get_table_mut_internal<'a>(
+        &'a mut self,
+        table_path: &[String],
+        insert_if_not_exists: bool,
+    ) -> CargoResult<&'a mut toml_edit::Item> {
+        /// Descend into a manifest until the required table is found.
+        fn descend<'a>(
+            input: &'a mut toml_edit::Item,
+            path: &[String],
+            insert_if_not_exists: bool,
+        ) -> CargoResult<&'a mut toml_edit::Item> {
+            if let Some(segment) = path.get(0) {
+                let value = if insert_if_not_exists {
+                    input[&segment].or_insert(toml_edit::table())
+                } else {
+                    input
+                        .get_mut(&segment)
+                        .ok_or_else(|| non_existent_table_err(segment))?
+                };
+
+                if value.is_table_like() {
+                    descend(value, &path[1..], insert_if_not_exists)
+                } else {
+                    Err(non_existent_table_err(segment))
+                }
+            } else {
+                Ok(input)
+            }
+        }
+
+        descend(self.data.as_item_mut(), table_path, insert_if_not_exists)
+    }
 }
 
 impl str::FromStr for Manifest {
@@ -376,7 +403,7 @@ impl LocalManifest {
             .to_owned();
         let dep_key = dep.toml_key();
 
-        let table = self.get_table_mut(table_path)?;
+        let table = self.get_or_insert_table_mut(table_path)?;
         if let Some(dep_item) = table.as_table_like_mut().unwrap().get_mut(dep_key) {
             dep.update_toml(&crate_root, dep_item);
         } else {
@@ -413,7 +440,7 @@ impl LocalManifest {
             .parent()
             .expect("manifest path is absolute")
             .to_owned();
-        let table = self.get_table_mut(table_path)?;
+        let table = self.get_or_insert_table_mut(table_path)?;
 
         // If (and only if) there is an old entry, merge the new one in.
         if table.as_table_like().unwrap().contains_key(dep_key) {
@@ -447,22 +474,18 @@ impl LocalManifest {
     ///   let mut manifest = LocalManifest { path, manifest: Manifest { data: toml_edit::Document::new() } };
     ///   let dep = Dependency::new("cargo-edit").set_version("0.1.0");
     ///   let _ = manifest.insert_into_table(&vec!["dependencies".to_owned()], &dep);
-    ///   assert!(manifest.remove_from_table("dependencies", &dep.name).is_ok());
-    ///   assert!(manifest.remove_from_table("dependencies", &dep.name).is_err());
+    ///   assert!(manifest.remove_from_table(&["dependencies".to_owned()], &dep.name).is_ok());
+    ///   assert!(manifest.remove_from_table(&["dependencies".to_owned()], &dep.name).is_err());
     ///   assert!(!manifest.data.contains_key("dependencies"));
     /// ```
-    pub fn remove_from_table(&mut self, table: &str, name: &str) -> CargoResult<()> {
-        let parent_table = self
-            .data
-            .get_mut(table)
-            .filter(|t| t.is_table_like())
-            .ok_or_else(|| non_existent_table_err(table))?;
+    pub fn remove_from_table(&mut self, table_path: &[String], name: &str) -> CargoResult<()> {
+        let parent_table = self.get_table_mut(table_path)?;
 
         {
             let dep = parent_table
                 .get_mut(name)
                 .filter(|t| !t.is_none())
-                .ok_or_else(|| non_existent_dependency_err(name, table))?;
+                .ok_or_else(|| non_existent_dependency_err(name, table_path.join(".")))?;
             // remove the dependency
             *dep = toml_edit::Item::None;
         }
@@ -721,7 +744,7 @@ mod tests {
         let dep = Dependency::new("cargo-edit").set_version("0.1.0");
         let _ = manifest.insert_into_table(&["dependencies".to_owned()], &dep);
         assert!(manifest
-            .remove_from_table("dependencies", &dep.name)
+            .remove_from_table(&["dependencies".to_owned()], &dep.name)
             .is_ok());
         assert_eq!(manifest.data.to_string(), clone.data.to_string());
     }
@@ -780,7 +803,7 @@ mod tests {
         };
         let dep = Dependency::new("cargo-edit").set_version("0.1.0");
         assert!(manifest
-            .remove_from_table("dependencies", &dep.name)
+            .remove_from_table(&["dependencies".to_owned()], &dep.name)
             .is_err());
     }
 
@@ -799,7 +822,7 @@ mod tests {
             .insert_into_table(&["dependencies".to_owned()], &other_dep)
             .is_ok());
         assert!(manifest
-            .remove_from_table("dependencies", &dep.name)
+            .remove_from_table(&["dependencies".to_owned()], &dep.name)
             .is_err());
     }
 

tests/cmd/rm/invalid_rm_target.in

@@ -0,0 +1 @@
+rm-target.in

tests/cmd/rm/invalid_rm_target.out/Cargo.toml

@@ -0,0 +1,35 @@
+[package]
+name = "cargo-rm-target-test-fixture"
+version = "0.1.0"
+
+[[bin]]
+name = "main"
+path = "src/main.rs"
+
+[target.x86_64-unknown-freebsd.build-dependencies]
+semver = "0.1.0"
+
+[target.x86_64-unknown-linux-gnu.build-dependencies]
+semver = "0.1.0"
+
+[dependencies]
+docopt = "0.6"
+pad = "0.1"
+rustc-serialize = "0.3"
+semver = "0.1"
+toml = "0.1"
+clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true}
+
+[target.x86_64-unknown-linux-gnu.dependencies]
+dbus = "0.9.5"
+
+[dev-dependencies]
+regex = "0.1.41"
+serde = "1.0.90"
+
+[target.x86_64-unknown-linux-gnu.dev-dependencies]
+ncurses = "5.101"
+
+[features]
+std = ["serde/std", "semver/std"]
+annoy = ["clippy"]

tests/cmd/rm/invalid_rm_target.toml

@@ -0,0 +1,15 @@
+bin.name = "cargo-rm"
+args = ["rm", "--target", "powerpc-unknown-linux-gnu", "dbus"]
+status.code = 1
+stdout = ""
+stderr = """
+    Removing dbus from dependencies for target `powerpc-unknown-linux-gnu`
+Could not edit `Cargo.toml`.
+
+ERROR: The table `powerpc-unknown-linux-gnu` could not be found.
+Error: The table `powerpc-unknown-linux-gnu` could not be found.
+"""
+fs.sandbox = true
+
+[env.add]
+CARGO_IS_TEST="1"

tests/cmd/rm/invalid_rm_target_dep.in

@@ -0,0 +1 @@
+rm-target.in

tests/cmd/rm/invalid_rm_target_dep.out/Cargo.toml

@@ -0,0 +1,35 @@
+[package]
+name = "cargo-rm-target-test-fixture"
+version = "0.1.0"
+
+[[bin]]
+name = "main"
+path = "src/main.rs"
+
+[target.x86_64-unknown-freebsd.build-dependencies]
+semver = "0.1.0"
+
+[target.x86_64-unknown-linux-gnu.build-dependencies]
+semver = "0.1.0"
+
+[dependencies]
+docopt = "0.6"
+pad = "0.1"
+rustc-serialize = "0.3"
+semver = "0.1"
+toml = "0.1"
+clippy = {git = "https://github.com/Manishearth/rust-clippy.git", optional = true}
+
+[target.x86_64-unknown-linux-gnu.dependencies]
+dbus = "0.9.5"
+
+[dev-dependencies]
+regex = "0.1.41"
+serde = "1.0.90"
+
+[target.x86_64-unknown-linux-gnu.dev-dependencies]
+ncurses = "5.101"
+
+[features]
+std = ["serde/std", "semver/std"]
+annoy = ["clippy"]

tests/cmd/rm/invalid_rm_target_dep.toml

@@ -0,0 +1,15 @@
+bin.name = "cargo-rm"
+args = ["rm", "--target", "x86_64-unknown-linux-gnu", "toml"]
+status.code = 1
+stdout = ""
+stderr = """
+    Removing toml from dependencies for target `x86_64-unknown-linux-gnu`
+Could not edit `Cargo.toml`.
+
+ERROR: The dependency `toml` could not be found in `target.x86_64-unknown-linux-gnu.dependencies`.
+Error: The dependency `toml` could not be found in `target.x86_64-unknown-linux-gnu.dependencies`.
+"""
+fs.sandbox = true
+
+[env.add]
+CARGO_IS_TEST="1"