Clean up workspace dependencies after cargo remove

Author: cassaundra

src/bin/cargo/commands/remove.rs

@@ -77,6 +77,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
 
     let options = RemoveOptions {
         config,
+        workspace: &workspace,
         spec,
         dependencies,
         section,

src/cargo/ops/cargo_remove.rs

@@ -1,6 +1,8 @@
 //! Core of cargo-remove command
 
+use crate::core::Dependency;
 use crate::core::Package;
+use crate::core::Workspace;
 use crate::util::toml_mut::manifest::DepTable;
 use crate::util::toml_mut::manifest::LocalManifest;
 use crate::CargoResult;
@@ -11,6 +13,8 @@ use crate::Config;
 pub struct RemoveOptions<'a> {
     /// Configuration information for Cargo operations
     pub config: &'a Config,
+    /// Workspace in which the operation is occurring.
+    pub workspace: &'a Workspace<'a>,
     /// Package to remove dependencies from
     pub spec: &'a Package,
     /// Dependencies to remove
@@ -33,7 +37,15 @@ pub fn remove(options: &RemoveOptions<'_>) -> CargoResult<()> {
     let manifest_path = options.spec.manifest_path().to_path_buf();
     let mut manifest = LocalManifest::try_new(&manifest_path)?;
 
+    let mut to_remove_workspace = vec![];
+
     for dep in &options.dependencies {
+        // If this was the last instance of a workspace dependency, queue it to be removed from the
+        // workspace dependencies.
+        if dep_is_workspace(dep, &dep_table, &manifest) {
+            to_remove_workspace.push(dep);
+        }
+
         let section = if dep_table.len() >= 3 {
             format!("{} for target `{}`", &dep_table[2], &dep_table[1])
         } else {
@@ -46,20 +58,118 @@ pub fn remove(options: &RemoveOptions<'_>) -> CargoResult<()> {
 
         manifest.remove_from_table(&dep_table, dep)?;
 
-        // Now that we have removed the crate, if that was the last reference to that
-        // crate, then we need to drop any explicitly activated features on
-        // that crate.
+        // Now that we have removed the crate, if that was the last reference to that crate, then we
+        // need to drop any explicitly activated features on that crate.
         manifest.gc_dep(dep);
     }
 
+    // remove from workspace dependencies
+
+    let mut ws_manifest = None;
+
+    if !to_remove_workspace.is_empty() {
+        // read the packages in the workspace, substituting in the modified manifest
+        let members = options
+            .workspace
+            .members()
+            .map(|p| {
+                if p.manifest_path() == manifest_path {
+                    Ok((p, manifest.clone()))
+                } else {
+                    LocalManifest::try_new(p.manifest_path()).map(|m| (p, m))
+                }
+            })
+            .collect::<CargoResult<Vec<_>>>()?;
+
+        if options.workspace.root_manifest() != manifest_path {
+            let manifest: toml_edit::Document =
+                cargo_util::paths::read(options.workspace.root_manifest())?.parse()?;
+            ws_manifest = Some(manifest);
+        }
+
+        let ws_manifest = match &mut ws_manifest {
+            Some(manifest) => manifest,
+            None => &mut manifest.data,
+        };
+
+        for dep in to_remove_workspace {
+            if !dep_in_workspace(dep, &members) {
+                remove_workspace_dep(dep, ws_manifest);
+                options
+                    .config
+                    .shell()
+                    .status("Removing", format!("{dep} from workspace dependencies"))?;
+            }
+        }
+    }
+
     if options.dry_run {
         options
             .config
             .shell()
             .warn("aborting remove due to dry run")?;
     } else {
         manifest.write()?;
+
+        if let Some(ws_manifest) = ws_manifest {
+            cargo_util::paths::write(
+                options.workspace.root_manifest(),
+                ws_manifest.to_string().as_bytes(),
+            )?;
+        }
     }
 
     Ok(())
 }
+
+/// Get whether or not a dependency is marked as `workspace`.
+fn dep_is_workspace(dep: &str, dep_table: &[String], manifest: &LocalManifest) -> bool {
+    if let Ok(toml_edit::Item::Table(table)) = manifest.get_table(dep_table) {
+        let value = table.get(dep).and_then(|i| i.get("workspace"));
+        if let Some(toml_edit::Item::Value(value)) = value {
+            return value.as_bool() == Some(true);
+        }
+    }
+
+    false
+}
+
+/// Get whether or not a dependency is depended upon in a workspace.
+fn dep_in_workspace(dep: &str, members: &[(&Package, LocalManifest)]) -> bool {
+    members.iter().any(|(pkg, manifest)| {
+        pkg.dependencies()
+            .iter()
+            .filter(|d| d.name_in_toml() == dep)
+            .map(|d| (d, dep_to_table(d)))
+            .any(|(d, t)| {
+                // Information about workspace dependencies is not preserved at this stage, so we
+                // have to manually read from the TOML manifest
+                let dep_table = t
+                    .to_table()
+                    .into_iter()
+                    .map(String::from)
+                    .collect::<Vec<_>>();
+                dep_is_workspace(&d.package_name(), &dep_table, manifest)
+            })
+    })
+}
+
+/// Find the corresponding [`DepTable`] for a [`Dependency`].
+fn dep_to_table(dep: &Dependency) -> DepTable {
+    let mut dep_table = DepTable::new().set_kind(dep.kind());
+    if let Some(target) = dep.platform().map(|p| p.to_string()) {
+        dep_table = dep_table.set_target(target);
+    }
+    dep_table
+}
+
+/// Remove a dependency from a workspace manifest.
+fn remove_workspace_dep(dep: &str, ws_manifest: &mut toml_edit::Document) {
+    if let Some(toml_edit::Item::Table(table)) = ws_manifest
+        .get_mut("workspace")
+        .and_then(|t| t.get_mut("dependencies"))
+    {
+        table.set_implicit(true);
+        table.remove(dep);
+    }
+}

src/cargo/util/toml_mut/manifest.rs

@@ -246,7 +246,7 @@ impl std::fmt::Display for Manifest {
 }
 
 /// An editable Cargo manifest that is available locally.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct LocalManifest {
     /// Path to the manifest.
     pub path: PathBuf,

tests/testsuite/cargo_remove/mod.rs

@@ -22,6 +22,8 @@ mod target;
 mod target_build;
 mod target_dev;
 mod update_lock_file;
+mod virtual_workspace;
+mod workspace;
 
 fn init_registry() {
     cargo_test_support::registry::init();

tests/testsuite/cargo_remove/virtual_workspace/in/Cargo.toml

@@ -0,0 +1,5 @@
+[workspace]
+members = [ "my-package" ]
+
+[workspace.dependencies]
+semver = "0.1.0"

tests/testsuite/cargo_remove/virtual_workspace/in/my-package/Cargo.toml

@@ -0,0 +1,24 @@
+[package]
+name = "my-package"
+version = "0.1.0"
+
+[[bin]]
+name = "main"
+path = "src/main.rs"
+
+[build-dependencies]
+semver = { workspace = true }
+
+[dependencies]
+docopt = "0.6"
+rustc-serialize = "0.4"
+semver = "0.1"
+toml = "0.1"
+clippy = "0.4"
+
+[dev-dependencies]
+regex = "0.1.1"
+serde = "1.0.90"
+
+[features]
+std = ["serde/std", "semver/std"]

tests/testsuite/cargo_remove/virtual_workspace/in/my-package/src/lib.rs

@@ -0,0 +1 @@
+

tests/testsuite/cargo_remove/virtual_workspace/mod.rs

@@ -0,0 +1,25 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::curr_dir;
+use cargo_test_support::CargoCommand;
+use cargo_test_support::Project;
+
+use crate::cargo_remove::init_registry;
+
+#[cargo_test]
+fn case() {
+    init_registry();
+    let project = Project::from_template(curr_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("remove")
+        .args(["--package", "my-package", "--build", "semver"])
+        .current_dir(cwd)
+        .assert()
+        .success()
+        .stdout_matches_path(curr_dir!().join("stdout.log"))
+        .stderr_matches_path(curr_dir!().join("stderr.log"));
+
+    assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
+}

tests/testsuite/cargo_remove/virtual_workspace/out/Cargo.toml

@@ -0,0 +1,2 @@
+[workspace]
+members = [ "my-package" ]

tests/testsuite/cargo_remove/virtual_workspace/out/my-package/Cargo.toml

@@ -0,0 +1,21 @@
+[package]
+name = "my-package"
+version = "0.1.0"
+
+[[bin]]
+name = "main"
+path = "src/main.rs"
+
+[dependencies]
+docopt = "0.6"
+rustc-serialize = "0.4"
+semver = "0.1"
+toml = "0.1"
+clippy = "0.4"
+
+[dev-dependencies]
+regex = "0.1.1"
+serde = "1.0.90"
+
+[features]
+std = ["serde/std", "semver/std"]

tests/testsuite/cargo_remove/virtual_workspace/out/my-package/src/lib.rs

@@ -0,0 +1 @@
+

tests/testsuite/cargo_remove/virtual_workspace/stderr.log

@@ -0,0 +1,3 @@
+    Removing semver from build-dependencies
+    Removing semver from workspace dependencies
+    Updating `dummy-registry` index

tests/testsuite/cargo_remove/virtual_workspace/stdout.log

@@ -0,0 +1,30 @@
+[workspace]
+members = [ "my-member" ]
+
+[workspace.dependencies]
+semver = "0.1.0"
+
+[package]
+name = "my-package"
+version = "0.1.0"
+
+[[bin]]
+name = "main"
+path = "src/main.rs"
+
+[build-dependencies]
+semver = { workspace = true }
+
+[dependencies]
+docopt = "0.6"
+rustc-serialize = "0.4"
+semver = "0.1"
+toml = "0.1"
+clippy = "0.4"
+
+[dev-dependencies]
+regex = "0.1.1"
+serde = "1.0.90"
+
+[features]
+std = ["serde/std", "semver/std"]

tests/testsuite/cargo_remove/workspace/in/Cargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "my-member"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]

tests/testsuite/cargo_remove/workspace/in/my-member/Cargo.toml

@@ -0,0 +1,25 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::curr_dir;
+use cargo_test_support::CargoCommand;
+use cargo_test_support::Project;
+
+use crate::cargo_remove::init_registry;
+
+#[cargo_test]
+fn case() {
+    init_registry();
+    let project = Project::from_template(curr_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("remove")
+        .args(["--package", "my-package", "--build", "semver"])
+        .current_dir(cwd)
+        .assert()
+        .success()
+        .stdout_matches_path(curr_dir!().join("stdout.log"))
+        .stderr_matches_path(curr_dir!().join("stderr.log"));
+
+    assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
+}

tests/testsuite/cargo_remove/workspace/in/my-member/src/main.rs

@@ -0,0 +1,24 @@
+[workspace]
+members = [ "my-member" ]
+
+[package]
+name = "my-package"
+version = "0.1.0"
+
+[[bin]]
+name = "main"
+path = "src/main.rs"
+
+[dependencies]
+docopt = "0.6"
+rustc-serialize = "0.4"
+semver = "0.1"
+toml = "0.1"
+clippy = "0.4"
+
+[dev-dependencies]
+regex = "0.1.1"
+serde = "1.0.90"
+
+[features]
+std = ["serde/std", "semver/std"]

tests/testsuite/cargo_remove/workspace/mod.rs

@@ -0,0 +1,6 @@
+[package]
+name = "my-member"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]

tests/testsuite/cargo_remove/workspace/out/Cargo.toml

@@ -0,0 +1,3 @@
+    Removing semver from build-dependencies
+    Removing semver from workspace dependencies
+    Updating `dummy-registry` index