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,4 +1,5 @@
 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;
@@ -88,8 +89,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     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 +122,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 and replace sections of a 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 +133,72 @@ 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);
-        }
-    }
+    gc_workspace_dependencies(&mut manifest, &members);
+    gc_replace(&mut manifest, &members);
 
     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)
+/// Clean up the workspace.dependencies section in a root manifest.
+fn gc_workspace_dependencies(manifest: &mut toml_edit::Document, members: &[LocalManifest]) {
+    let in_workspace = |dep: &str| {
+        members.iter().any(|manifest| {
+            manifest.get_sections().iter().any(|(_, table)| {
+                let dep_item = table.as_table_like().unwrap().get(dep);
+                dep_item
+                    .and_then(|t| t.get("workspace"))
+                    .and_then(|i| i.as_bool())
+                    .unwrap_or(false)
+            })
         })
-    })
-}
+    };
 
-/// 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
+    if let Some(toml_edit::Item::Table(table)) = manifest
         .get_mut("workspace")
         .and_then(|t| t.get_mut("dependencies"))
     {
         table.set_implicit(true);
-        table.remove(dep);
+
+        for (key, item) in table.iter_mut() {
+            if !in_workspace(key.get()) {
+                *item = toml_edit::Item::None;
+            }
+        }
+    }
+}
+
+/// Clean up the replace section in a root manifest.
+fn gc_replace(manifest: &mut toml_edit::Document, members: &[LocalManifest]) {
+    let in_workspace = |dep: &str, version: &Option<&semver::Version>| {
+        members.iter().any(|manifest| {
+            let mut deps = manifest
+                .get_dependency_versions(dep)
+                .filter_map(|(_, d)| d.ok())
+                .filter(|d| d.name == dep);
+
+            if let Some(version) = version {
+                deps.any(|d| {
+                    d.version()
+                        .and_then(|v| semver::VersionReq::parse(v).ok())
+                        .map(|vq| vq.matches(version))
+                        .unwrap_or(false)
+                })
+            } else {
+                deps.next().is_some()
+            }
+        })
+    };
+
+    if let Some(toml_edit::Item::Table(table)) = manifest.get_mut("replace") {
+        table.set_implicit(true);
+        for (key, item) in table.iter_mut() {
+            if let Ok(spec) = PackageIdSpec::parse(key.get()) {
+                if !in_workspace(spec.name().as_str(), &spec.version()) {
+                    *item = toml_edit::Item::None;
+                }
+            }
+        }
     }
 }

src/cargo/util/toml_mut/manifest.rs

@@ -393,6 +393,7 @@ impl LocalManifest {
         let explicit_dep_activation = self.is_explicit_dep_activation(dep_key);
         let status = self.dep_status(dep_key);
 
+        // clean up features
         if let Some(toml_edit::Item::Table(feature_table)) =
             self.data.as_table_mut().get_mut("features")
         {
@@ -409,6 +410,42 @@ impl LocalManifest {
                 }
             }
         }
+
+        // continue only if the dep is fully removed
+        if status != DependencyStatus::None {
+            return;
+        }
+
+        // clean up profile sections
+        if let Some(toml_edit::Item::Table(profile_table)) = self.data.get_mut("profile") {
+            profile_table.set_implicit(true);
+            for (_, item) in profile_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);
+                        package_table.remove(dep_key);
+                    }
+                }
+            }
+        }
+
+        // clean up patch sections
+        if let Some(toml_edit::Item::Table(patch_table)) = self.data.get_mut("patch") {
+            patch_table.set_implicit(true);
+            for (_, item) in patch_table.iter_mut() {
+                if let toml_edit::Item::Table(table) = item {
+                    table.set_implicit(true);
+                    for (key, item) in table.iter_mut() {
+                        if key.get() == dep_key {
+                            *item = toml_edit::Item::None;
+                        }
+                    }
+                }
+            }
+        }
     }
 
     fn is_explicit_dep_activation(&self, dep_key: &str) -> bool {
@@ -500,7 +537,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 @@
+

tests/testsuite/cargo_remove/gc_profile/in/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(["toml"])
+        .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/gc_profile/in/src/lib.rs

@@ -0,0 +1,27 @@
+[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"
+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

tests/testsuite/cargo_remove/gc_profile/mod.rs

@@ -0,0 +1,2 @@
+    Removing toml from dependencies
+    Updating `dummy-registry` index

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

@@ -0,0 +1,5 @@
+[workspace]
+members = [ "my-package" ]
+
+[replace]
+"toml:0.1.0" = { path = "../toml" }

tests/testsuite/cargo_remove/gc_profile/stderr.log

@@ -0,0 +1,25 @@
+[package]
+name = "my-package"
+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"]

tests/testsuite/cargo_remove/gc_profile/stdout.log

@@ -0,0 +1 @@
+