Clean profile, patch, and replace in cargo remove

src/bin/cargo/commands/remove.rs

@@ -1,10 +1,14 @@
 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::print_available_packages;
+use cargo::util::toml_mut::dependency::Dependency;
+use cargo::util::toml_mut::dependency::MaybeWorkspace;
+use cargo::util::toml_mut::dependency::Source;
 use cargo::util::toml_mut::manifest::DepTable;
 use cargo::util::toml_mut::manifest::LocalManifest;
 use cargo::CargoResult;
@@ -86,7 +90,7 @@ 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);
 
@@ -100,8 +104,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)?;
@@ -133,49 +137,208 @@ 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()?;
+    let mut is_modified = true;
 
     let members = workspace
         .members()
         .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);
+    let mut 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<_>>>()?;
+
+    // Clean up the workspace.dependencies section and replace instances of
+    // workspace dependencies with their definitions
+    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() {
+            let ws_dep = Dependency::from_toml(&workspace.root(), key.get(), item)?;
+
+            // search for uses of this workspace dependency
+            let mut is_used = false;
+            for dep in dependencies.iter_mut().filter(|d| {
+                d.toml_key() == key.get() && matches!(d.source(), Some(Source::Workspace(_)))
+            }) {
+                // HACK: Replace workspace references in `dependencies` to simplify later GC steps:
+                // 1. Avoid having to look it up again to determine the dependency source / spec
+                // 2. The entry might get deleted, preventing us from looking it up again
+                //
+                // This does lose extra information, like features enabled, but that shouldn't be a
+                // problem for GC
+                *dep = ws_dep.clone();
+
+                is_used = true;
+            }
+
+            if !is_used {
+                *item = toml_edit::Item::None;
+                is_modified = true;
+            }
         }
     }
 
-    cargo_util::paths::write(workspace.root_manifest(), manifest.to_string().as_bytes())?;
+    // Clean up the profile section
+    //
+    // Example tables:
+    // - profile.dev.package.foo
+    // - profile.release.package."*"
+    // - profile.release.package."foo:2.1.0"
+    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 !spec_has_match(
+                            &PackageIdSpec::parse(key.get())?,
+                            &dependencies,
+                            workspace.config(),
+                        )? {
+                            *item = toml_edit::Item::None;
+                            is_modified = true;
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    // Clean up the patch section
+    if let Some(toml_edit::Item::Table(patch_section_table)) = manifest.get_mut("patch") {
+        patch_section_table.set_implicit(true);
+
+        // The key in each of the subtables is a source (either a registry or a URL)
+        for (source, 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() {
+                    let package_name =
+                        Dependency::from_toml(&workspace.root_manifest(), key.get(), item)?.name;
+                    if !source_has_match(
+                        &package_name,
+                        source.get(),
+                        &dependencies,
+                        workspace.config(),
+                    )? {
+                        *item = toml_edit::Item::None;
+                    }
+                }
+            }
+        }
+    }
+
+    // Clean up the replace section
+    if let Some(toml_edit::Item::Table(table)) = manifest.get_mut("replace") {
+        table.set_implicit(true);
+
+        for (key, item) in table.iter_mut() {
+            if !spec_has_match(
+                &PackageIdSpec::parse(key.get())?,
+                &dependencies,
+                workspace.config(),
+            )? {
+                *item = toml_edit::Item::None;
+                is_modified = true;
+            }
+        }
+    }
+
+    if is_modified {
+        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)
-        })
-    })
+/// Check whether or not a package ID spec matches any non-workspace dependencies.
+fn spec_has_match(
+    spec: &PackageIdSpec,
+    dependencies: &[Dependency],
+    config: &Config,
+) -> CargoResult<bool> {
+    for dep in dependencies {
+        if spec.name().as_str() != &dep.name {
+            continue;
+        }
+
+        let version_matches = match (spec.version(), dep.version()) {
+            (Some(v), Some(vq)) => semver::VersionReq::parse(vq)?.matches(v),
+            (Some(_), None) => false,
+            (None, None | Some(_)) => true,
+        };
+        if !version_matches {
+            continue;
+        }
+
+        match dep.source_id(config)? {
+            MaybeWorkspace::Other(source_id) => {
+                if spec.url().map(|u| u == source_id.url()).unwrap_or(true) {
+                    return Ok(true);
+                }
+            }
+            MaybeWorkspace::Workspace(_) => {}
+        }
+    }
+
+    Ok(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
-        .get_mut("workspace")
-        .and_then(|t| t.get_mut("dependencies"))
-    {
-        table.set_implicit(true);
-        table.remove(dep);
+/// Check whether or not a source (URL or registry name) matches any non-workspace dependencies.
+fn source_has_match(
+    name: &str,
+    source: &str,
+    dependencies: &[Dependency],
+    config: &Config,
+) -> CargoResult<bool> {
+    for dep in dependencies {
+        if &dep.name != name {
+            continue;
+        }
+
+        match dep.source_id(config)? {
+            MaybeWorkspace::Other(source_id) => {
+                if source_id.is_registry() {
+                    if source_id.display_registry_name() == source
+                        || source_id.url().as_str() == source
+                    {
+                        return Ok(true);
+                    }
+                } else if source_id.is_git() {
+                    if source_id.url().as_str() == source {
+                        return Ok(true);
+                    }
+                }
+            }
+            MaybeWorkspace::Workspace(_) => {}
+        }
     }
+
+    Ok(false)
 }

tests/testsuite/cargo_remove/gc_patch/mod.rs

@@ -0,0 +1,72 @@
+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!(
+                "[workspace]\n\
+                 members = [ \"my-member\" ]\n\
+                 \n\
+                 [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\
+                 \n\
+                 [patch.crates-io]\n\
+                 bar = {{ git = \"{git_project2}\" }}\n",
+            ),
+        )
+        .file("src/lib.rs", "")
+        .file(
+            "my-member/Cargo.toml",
+            "[package]\n\
+               name = \"my-member\"\n\
+               version = \"0.1.0\"\n\
+               \n\
+               [dependencies]\n\
+               bar = \"0.1.0\"\n",
+        )
+        .file("my-member/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,9 @@
+[workspace]
+members = [ "my-member" ]
+
+[package]
+name = "my-project"
+version = "0.1.0"
+
+[patch.crates-io]
+bar = { git = "[ROOTURL]/bar2" }

tests/testsuite/cargo_remove/gc_patch/stderr.log

@@ -0,0 +1,3 @@
+    Removing bar from dependencies
+    Updating git repository `[ROOTURL]/bar2`
+    Updating `dummy-registry` index

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

@@ -0,0 +1,36 @@
+[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"
+toml = "0.2.3"
+docopt = "0.6"
+
+[features]
+std = ["serde/std", "semver/std"]
+
+[profile.dev.package.docopt]
+opt-level = 3
+
+[profile.dev.package."toml@0.1.0"]
+opt-level = 3
+
+[profile.release.package.toml]
+opt-level = 1
+overflow-checks = false

tests/testsuite/cargo_remove/gc_profile/in/src/lib.rs

@@ -0,0 +1 @@
+