refactor: replace copied versions of package arg with completer argument

Author: jakobhellermann

src/bin/cargo/commands/clean.rs

@@ -6,14 +6,18 @@ use cargo::core::global_cache_tracker::GlobalCacheTracker;
 use cargo::ops::CleanContext;
 use cargo::ops::{self, CleanOptions};
 use cargo::util::print_available_packages;
+use clap_complete::ArgValueCandidates;
 use std::time::Duration;
 
 pub fn cli() -> Command {
     subcommand("clean")
         .about("Remove artifacts that cargo has generated in the past")
         .arg_doc("Whether or not to clean just the documentation directory")
         .arg_silent_suggestion()
-        .arg_clean_package_spec_simple("Package to clean artifacts for")
+        .arg_package_spec_simple(
+            "Package to clean artifacts for",
+            ArgValueCandidates::new(Vec::new), // TODO(completions): implement dynamic completion for `cargo clean -p`
+        )
         .arg_release("Whether or not to clean release artifacts")
         .arg_profile("Clean artifacts of the specified profile")
         .arg_target_triple("Target triple to clean output for")

src/bin/cargo/commands/package.rs

@@ -3,6 +3,7 @@ use crate::command_prelude::*;
 use cargo::ops;
 use cargo::ops::PackageMessageFormat;
 use cargo::ops::PackageOpts;
+use clap_complete::ArgValueCandidates;
 
 pub fn cli() -> Command {
     subcommand("package")
@@ -44,6 +45,7 @@ pub fn cli() -> Command {
             "Package(s) to assemble",
             "Assemble all packages in the workspace",
             "Don't assemble specified packages",
+            ArgValueCandidates::new(get_ws_member_candidates),
         )
         .arg_features()
         .arg_target_triple("Build for the target triple")

src/bin/cargo/commands/publish.rs

@@ -2,6 +2,7 @@ use crate::command_prelude::*;
 
 use cargo::ops::{self, PublishOpts};
 use cargo_credential::Secret;
+use clap_complete::ArgValueCandidates;
 
 pub fn cli() -> Command {
     subcommand("publish")
@@ -27,6 +28,7 @@ pub fn cli() -> Command {
             "Package(s) to publish",
             "Publish all packages in the workspace",
             "Don't publish specified packages",
+            ArgValueCandidates::new(get_ws_member_candidates),
         )
         .arg_features()
         .arg_parallel()

src/bin/cargo/commands/tree.rs

@@ -7,6 +7,7 @@ use cargo::ops::Packages;
 use cargo::ops::tree::{self, DisplayDepth, EdgeKind};
 use cargo::util::CargoResult;
 use cargo::util::print_available_packages;
+use clap_complete::ArgValueCandidates;
 use std::collections::HashSet;
 use std::str::FromStr;
 
@@ -87,10 +88,11 @@ pub fn cli() -> Command {
                 .short('V')
                 .hide(true),
         )
-        .arg_dependency_package_spec_no_all(
+        .arg_package_spec_no_all(
             "Package to be used as the root of the tree",
             "Display the tree for all packages in the workspace",
             "Exclude specific workspace members",
+            ArgValueCandidates::new(get_dependency_package_candidates),
         )
         .arg_features()
         .arg(flag("all-targets", "Deprecated, use --target=all instead").hide(true))

src/bin/cargo/commands/uninstall.rs

@@ -1,6 +1,9 @@
 use crate::command_prelude::*;
 
-use cargo::ops;
+use cargo::{CargoResult, core::PackageId, ops};
+use clap_complete::ArgValueCandidates;
+
+use std::collections::BTreeSet;
 
 pub fn cli() -> Command {
     subcommand("uninstall")
@@ -15,7 +18,10 @@ pub fn cli() -> Command {
         )
         .arg(opt("root", "Directory to uninstall packages from").value_name("DIR"))
         .arg_silent_suggestion()
-        .arg_installed_package_spec_simple("Package to uninstall")
+        .arg_package_spec_simple(
+            "Package to uninstall",
+            ArgValueCandidates::new(get_installed_package_candidates),
+        )
         .arg(
             multi_opt("bin", "NAME", "Only uninstall the binary NAME")
                 .help_heading(heading::TARGET_SELECTION),
@@ -52,7 +58,7 @@ fn get_installed_crates() -> Vec<clap_complete::CompletionCandidate> {
 fn get_installed_crates_() -> Option<Vec<clap_complete::CompletionCandidate>> {
     let mut candidates = Vec::new();
 
-    let gctx = GlobalContext::default().ok()?;
+    let gctx = new_gctx_for_completions().ok()?;
 
     let root = ops::resolve_root(None, &gctx).ok()?;
 
@@ -66,3 +72,32 @@ fn get_installed_crates_() -> Option<Vec<clap_complete::CompletionCandidate>> {
 
     Some(candidates)
 }
+
+fn get_installed_packages() -> CargoResult<Vec<(PackageId, BTreeSet<String>)>> {
+    let gctx = new_gctx_for_completions()?;
+    let root = ops::resolve_root(None, &gctx)?;
+
+    let tracker = ops::InstallTracker::load(&gctx, &root)?;
+    Ok(tracker
+        .all_installed_bins()
+        .map(|(package_id, bins)| (*package_id, bins.clone()))
+        .collect())
+}
+fn get_installed_package_candidates() -> Vec<clap_complete::CompletionCandidate> {
+    get_installed_packages()
+        .unwrap_or_default()
+        .into_iter()
+        .map(|(pkg, bins)| {
+            let single_binary = bins.iter().next().take_if(|_| bins.len() == 1);
+
+            let help = if single_binary.is_some_and(|bin| bin == pkg.name().as_str()) {
+                None
+            } else {
+                let binaries = bins.into_iter().collect::<Vec<_>>().as_slice().join(", ");
+                Some(binaries)
+            };
+
+            clap_complete::CompletionCandidate::new(pkg.name().as_str()).help(help.map(From::from))
+        })
+        .collect()
+}

src/cargo/util/command_prelude.rs

@@ -1,16 +1,13 @@
 use crate::CargoResult;
+use crate::core::Dependency;
 use crate::core::compiler::{
     BuildConfig, CompileKind, MessageFormat, RustcTargetData, TimingOutput,
 };
 use crate::core::resolver::{CliFeatures, ForceAllTargets, HasDevUnits};
-use crate::core::{Dependency, PackageId};
 use crate::core::{Edition, Package, TargetKind, Workspace, profiles::Profiles, shell};
 use crate::ops::lockfile::LOCKFILE_NAME;
 use crate::ops::registry::RegistryOrIndex;
-use crate::ops::{
-    self, CompileFilter, CompileOptions, InstallTracker, NewOptions, Packages, VersionControl,
-    resolve_root,
-};
+use crate::ops::{self, CompileFilter, CompileOptions, NewOptions, Packages, VersionControl};
 use crate::util::important_paths::find_root_manifest_for_wd;
 use crate::util::interning::InternedString;
 use crate::util::is_rustup;
@@ -26,11 +23,12 @@ use cargo_util_schemas::manifest::ProfileName;
 use cargo_util_schemas::manifest::RegistryName;
 use cargo_util_schemas::manifest::StringOrVec;
 use clap::builder::UnknownArgumentValueParser;
+use clap_complete::ArgValueCandidates;
 use home::cargo_home_with_cwd;
 use indexmap::IndexSet;
 use itertools::Itertools;
 use semver::Version;
-use std::collections::{BTreeSet, HashMap, HashSet};
+use std::collections::{HashMap, HashSet};
 use std::ffi::{OsStr, OsString};
 use std::path::Path;
 use std::path::PathBuf;
@@ -63,7 +61,13 @@ pub trait CommandExt: Sized {
         all: &'static str,
         exclude: &'static str,
     ) -> Self {
-        self.arg_package_spec_no_all(package, all, exclude)._arg(
+        self.arg_package_spec_no_all(
+            package,
+            all,
+            exclude,
+            ArgValueCandidates::new(get_ws_member_candidates),
+        )
+        ._arg(
             flag("all", "Alias for --workspace (deprecated)")
                 .help_heading(heading::PACKAGE_SELECTION),
         )
@@ -77,6 +81,7 @@ pub trait CommandExt: Sized {
         package: &'static str,
         all: &'static str,
         exclude: &'static str,
+        package_completion: ArgValueCandidates,
     ) -> Self {
         let unsupported_short_arg = {
             let value_parser = UnknownArgumentValueParser::suggest_arg("--exclude");
@@ -87,7 +92,7 @@ pub trait CommandExt: Sized {
                 .action(ArgAction::SetTrue)
                 .hide(true)
         };
-        self.arg_package_spec_simple(package)
+        self.arg_package_spec_simple(package, package_completion)
             ._arg(flag("workspace", all).help_heading(heading::PACKAGE_SELECTION))
             ._arg(
                 multi_opt("exclude", "SPEC", exclude)
@@ -99,75 +104,16 @@ pub trait CommandExt: Sized {
             ._arg(unsupported_short_arg)
     }
 
-    fn arg_dependency_package_spec_no_all(
+    fn arg_package_spec_simple(
         self,
         package: &'static str,
-        all: &'static str,
-        exclude: &'static str,
+        package_completion: ArgValueCandidates,
     ) -> Self {
-        let unsupported_short_arg = {
-            let value_parser = UnknownArgumentValueParser::suggest_arg("--exclude");
-            Arg::new("unsupported-short-exclude-flag")
-                .help("")
-                .short('x')
-                .value_parser(value_parser)
-                .action(ArgAction::SetTrue)
-                .hide(true)
-        };
-        self.arg_dependency_package_spec_simple(package)
-            ._arg(flag("workspace", all).help_heading(heading::PACKAGE_SELECTION))
-            ._arg(
-                multi_opt("exclude", "SPEC", exclude)
-                    .help_heading(heading::PACKAGE_SELECTION)
-                    .add(clap_complete::ArgValueCandidates::new(
-                        get_ws_member_candidates,
-                    )),
-            )
-            ._arg(unsupported_short_arg)
-    }
-
-    fn arg_package_spec_simple(self, package: &'static str) -> Self {
         self._arg(
             optional_multi_opt("package", "SPEC", package)
                 .short('p')
                 .help_heading(heading::PACKAGE_SELECTION)
-                .add(clap_complete::ArgValueCandidates::new(
-                    get_ws_member_candidates,
-                )),
-        )
-    }
-
-    fn arg_installed_package_spec_simple(self, package: &'static str) -> Self {
-        self._arg(
-            optional_multi_opt("package", "SPEC", package)
-                .short('p')
-                .help_heading(heading::PACKAGE_SELECTION)
-                .add(clap_complete::ArgValueCandidates::new(
-                    get_installed_package_candidates,
-                )),
-        )
-    }
-
-    fn arg_dependency_package_spec_simple(self, package: &'static str) -> Self {
-        self._arg(
-            optional_multi_opt("package", "SPEC", package)
-                .short('p')
-                .help_heading(heading::PACKAGE_SELECTION)
-                .add(clap_complete::ArgValueCandidates::new(
-                    get_dependency_package_candidates,
-                )),
-        )
-    }
-
-    fn arg_clean_package_spec_simple(self, package: &'static str) -> Self {
-        // TODO(completions): add dynamic completions for `cargo clean -p`
-        self._arg(
-            optional_multi_opt("package", "SPEC", package)
-                .short('p')
-                .help_heading(heading::PACKAGE_SELECTION)
-                .add(
-                    clap_complete::ArgValueCandidates::new(Vec::new), // TODO(complete): implement dynamic completion for `cargo clean -p`
-                ),
+                .add(package_completion),
         )
     }
 
@@ -1566,7 +1512,7 @@ fn get_ws_member_packages() -> CargoResult<Vec<Package>> {
     Ok(packages)
 }
 
-fn get_ws_member_candidates() -> Vec<clap_complete::CompletionCandidate> {
+pub fn get_ws_member_candidates() -> Vec<clap_complete::CompletionCandidate> {
     get_ws_member_packages()
         .unwrap_or_default()
         .into_iter()
@@ -1582,35 +1528,6 @@ fn get_ws_member_candidates() -> Vec<clap_complete::CompletionCandidate> {
         .collect::<Vec<_>>()
 }
 
-fn get_installed_packages() -> CargoResult<Vec<(PackageId, BTreeSet<String>)>> {
-    let gctx = new_gctx_for_completions()?;
-    let root = resolve_root(None, &gctx)?;
-
-    let tracker = InstallTracker::load(&gctx, &root)?;
-    Ok(tracker
-        .all_installed_bins()
-        .map(|(package_id, bins)| (*package_id, bins.clone()))
-        .collect())
-}
-fn get_installed_package_candidates() -> Vec<clap_complete::CompletionCandidate> {
-    get_installed_packages()
-        .unwrap_or_default()
-        .into_iter()
-        .map(|(pkg, bins)| {
-            let single_binary = bins.iter().next().take_if(|_| bins.len() == 1);
-
-            let help = if single_binary.is_some_and(|bin| bin == pkg.name().as_str()) {
-                None
-            } else {
-                let binaries = bins.into_iter().collect::<Vec<_>>().as_slice().join(", ");
-                Some(binaries)
-            };
-
-            clap_complete::CompletionCandidate::new(pkg.name().as_str()).help(help.map(From::from))
-        })
-        .collect()
-}
-
 pub fn get_dependency_package_candidates() -> Vec<clap_complete::CompletionCandidate> {
     get_packages()
         .unwrap_or_default()