Clean profile, patch, and replace in cargo remove

After a successful removal of a dependency, clean up the profile, patch, and
replace sections to remove all references to the dependency.

Author: cassaundra

src/bin/cargo/commands/remove.rs

@@ -1,9 +1,12 @@
 use cargo::core::dependency::DepKind;
+use cargo::core::PackageIdSpec;
 use cargo::core::Workspace;
 use cargo::ops::cargo_remove::remove;
 use cargo::ops::cargo_remove::RemoveOptions;
 use cargo::ops::resolve_ws;
 use cargo::util::command_prelude::*;
+use cargo::util::toml_mut::dependency::Dependency;
+use cargo::util::toml_mut::dependency::Source;
 use cargo::util::toml_mut::manifest::DepTable;
 use cargo::util::toml_mut::manifest::LocalManifest;
 use cargo::CargoResult;
@@ -74,22 +77,22 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
         .get_many::<String>("dependencies")
         .expect("required(true)")
         .cloned()
-        .collect();
+        .collect::<Vec<_>>();
 
     let section = parse_section(args);
 
     let options = RemoveOptions {
         config,
         spec,
-        dependencies,
+        dependencies: &dependencies,
         section,
         dry_run,
     };
     remove(&options)?;
 
     if !dry_run {
-        // Clean up workspace dependencies
-        gc_workspace(&workspace, &options.dependencies)?;
+        // Clean up the workspace
+        gc_workspace(&workspace)?;
 
         // Reload the workspace since we've changed dependencies
         let ws = args.workspace(config)?;
@@ -121,8 +124,9 @@ fn parse_section(args: &ArgMatches) -> DepTable {
     table
 }
 
-/// Clean up workspace dependencies which no longer have a reference to them.
-fn gc_workspace(workspace: &Workspace<'_>, dependencies: &[String]) -> CargoResult<()> {
+/// Clean up the workspace.dependencies, profile, patch, and replace sections of the root manifest
+/// by removing dependencies which no longer have a reference to them.
+fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
     let mut manifest: toml_edit::Document =
         cargo_util::paths::read(workspace.root_manifest())?.parse()?;
 
@@ -131,39 +135,91 @@ fn gc_workspace(workspace: &Workspace<'_>, dependencies: &[String]) -> CargoResu
         .map(|p| LocalManifest::try_new(p.manifest_path()))
         .collect::<CargoResult<Vec<_>>>()?;
 
-    for dep in dependencies {
-        if !dep_in_workspace(dep, &members) {
-            remove_workspace_dep(dep, &mut manifest);
-        }
-    }
-
-    cargo_util::paths::write(workspace.root_manifest(), manifest.to_string().as_bytes())?;
-
-    Ok(())
-}
-
-/// Get whether or not a dependency is depended upon in a workspace.
-fn dep_in_workspace(dep: &str, members: &[LocalManifest]) -> bool {
-    members.iter().any(|manifest| {
-        manifest.get_sections().iter().any(|(_, table)| {
-            table
-                .as_table_like()
-                .unwrap()
-                .get(dep)
-                .and_then(|t| t.get("workspace"))
-                .and_then(|v| v.as_bool())
-                .unwrap_or(false)
+    let dependencies = members
+        .iter()
+        .flat_map(|manifest| {
+            manifest.get_sections().into_iter().flat_map(|(_, table)| {
+                table
+                    .as_table_like()
+                    .unwrap()
+                    .iter()
+                    .map(|(key, item)| Dependency::from_toml(&manifest.path, key, item))
+                    .collect::<Vec<_>>()
+            })
         })
-    })
-}
+        .collect::<CargoResult<Vec<_>>>()?;
 
-/// 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
+    // clean up workspace.dependencies
+    if let Some(toml_edit::Item::Table(deps_table)) = manifest
         .get_mut("workspace")
         .and_then(|t| t.get_mut("dependencies"))
     {
+        deps_table.set_implicit(true);
+        for (key, item) in deps_table.iter_mut() {
+            if !dependencies.iter().any(|d| {
+                d.toml_key() == key.get() && matches!(d.source(), Some(Source::Workspace(_)))
+            }) {
+                *item = toml_edit::Item::None;
+            }
+        }
+    }
+
+    // clean up the profile section
+    if let Some(toml_edit::Item::Table(profile_section_table)) = manifest.get_mut("profile") {
+        profile_section_table.set_implicit(true);
+        for (_, item) in profile_section_table.iter_mut() {
+            if let toml_edit::Item::Table(profile_table) = item {
+                profile_table.set_implicit(true);
+                if let Some(toml_edit::Item::Table(package_table)) =
+                    profile_table.get_mut("package")
+                {
+                    package_table.set_implicit(true);
+                    for (key, item) in package_table.iter_mut() {
+                        if let Ok(spec) = PackageIdSpec::parse(key.get()) {
+                            if !dependencies.iter().any(|d| d.name == spec.name().as_str()) {
+                                *item = toml_edit::Item::None;
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    // clean up patch section
+    if let Some(toml_edit::Item::Table(patch_section_table)) = manifest.get_mut("patch") {
+        patch_section_table.set_implicit(true);
+        for (_, item) in patch_section_table.iter_mut() {
+            if let toml_edit::Item::Table(patch_table) = item {
+                patch_table.set_implicit(true);
+                for (key, item) in patch_table.iter_mut() {
+                    if !dependencies.iter().any(|d| d.name == key.get()) {
+                        *item = toml_edit::Item::None;
+                    }
+                }
+            }
+        }
+    }
+
+    // clean up replace section
+    if let Some(toml_edit::Item::Table(table)) = manifest.get_mut("replace") {
         table.set_implicit(true);
-        table.remove(dep);
+        for (key, item) in table.iter_mut() {
+            if let Ok(spec) = PackageIdSpec::parse(key.get()) {
+                if !dependencies.iter().any(|d| {
+                    d.name == spec.name().as_str()
+                        && d.version()
+                            .and_then(|v| semver::VersionReq::parse(v).ok())
+                            .and_then(|vq| spec.version().map(|v| vq.matches(v)))
+                            .unwrap_or(true)
+                }) {
+                    *item = toml_edit::Item::None;
+                }
+            }
+        }
     }
+
+    cargo_util::paths::write(workspace.root_manifest(), manifest.to_string().as_bytes())?;
+
+    Ok(())
 }

src/cargo/ops/cargo_remove.rs

@@ -14,7 +14,7 @@ pub struct RemoveOptions<'a> {
     /// Package to remove dependencies from
     pub spec: &'a Package,
     /// Dependencies to remove
-    pub dependencies: Vec<String>,
+    pub dependencies: &'a [String],
     /// Which dependency section to remove these from
     pub section: DepTable,
     /// Whether or not to actually write the manifest
@@ -33,7 +33,7 @@ pub fn remove(options: &RemoveOptions<'_>) -> CargoResult<()> {
     let manifest_path = options.spec.manifest_path().to_path_buf();
     let mut manifest = LocalManifest::try_new(&manifest_path)?;
 
-    for dep in &options.dependencies {
+    for dep in options.dependencies {
         let section = if dep_table.len() >= 3 {
             format!("{} for target `{}`", &dep_table[2], &dep_table[1])
         } else {

src/cargo/util/toml_mut/manifest.rs

@@ -500,7 +500,7 @@ fn fix_feature_activations(
         })
         .collect();
 
-    // Remove found idx in revers order so we don't invalidate the idx.
+    // Remove found idx in reverse order so we don't invalidate the idx.
     for idx in remove_list.iter().rev() {
         feature_values.remove(*idx);
     }

tests/testsuite/cargo_remove/gc_patch/mod.rs

@@ -0,0 +1,56 @@
+use cargo_test_support::basic_manifest;
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::curr_dir;
+use cargo_test_support::git;
+use cargo_test_support::project;
+use cargo_test_support::CargoCommand;
+
+use crate::cargo_remove::init_registry;
+
+#[cargo_test]
+fn case() {
+    init_registry();
+
+    let git_project1 = git::new("bar1", |project| {
+        project
+            .file("Cargo.toml", &basic_manifest("bar", "0.1.0"))
+            .file("src/lib.rs", "")
+    })
+    .url();
+
+    let git_project2 = git::new("bar2", |project| {
+        project
+            .file("Cargo.toml", &basic_manifest("bar", "0.1.0"))
+            .file("src/lib.rs", "")
+    })
+    .url();
+
+    let in_project = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                "[package]\n\
+                 name = \"my-project\"\n\
+                 version = \"0.1.0\"\n\
+                 \n\
+                 [dependencies]\n\
+                 bar = {{ git = \"{git_project1}\" }}\n\
+                 \n\
+                 [patch.\"{git_project1}\"]\n\
+                 bar = {{ git = \"{git_project2}\" }}\n",
+            ),
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("remove")
+        .args(["bar"])
+        .current_dir(&in_project.root())
+        .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"), &in_project.root());
+}

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

@@ -0,0 +1,3 @@
+[package]
+name = "my-project"
+version = "0.1.0"

tests/testsuite/cargo_remove/gc_patch/out/src/lib.rs

@@ -0,0 +1 @@
+    Removing bar from dependencies

tests/testsuite/cargo_remove/gc_patch/stderr.log

@@ -0,0 +1,35 @@
+[package]
+name = "cargo-remove-test-fixture"
+version = "0.1.0"
+
+[[bin]]
+name = "main"
+path = "src/main.rs"
+
+[build-dependencies]
+semver = "0.1.0"
+
+[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"
+docopt = "0.6"
+
+[features]
+std = ["serde/std", "semver/std"]
+
+[profile.dev.package.docopt]
+opt-level = 3
+
+[profile.dev.package.toml]
+opt-level = 3
+
+[profile.release.package.toml]
+opt-level = 1
+overflow-checks = false

tests/testsuite/cargo_remove/gc_patch/stdout.log

@@ -0,0 +1 @@
+