fix(upgrade): Exclusively operate on full workspaces

This came from the internals discussion on what `cargo upgrade` should
look like in cargo.

See https://internals.rust-lang.org/t/feedback-on-cargo-upgrade-to-prepare-it-for-merging/17101

Benefits
- Removes the UI conflict in selecting manifests to edit and selecting
  dependencies to upgrade
- Will work naturally with `[workspace.dependencies]`
- If we merge `upgrade` into `update` or supersede it, we'll already be
  aligned on behavior

README.md

@@ -140,25 +140,20 @@ cargo-upgrade [..]
 Upgrade dependency version requirements in Cargo.toml manifest files
 
 USAGE:
-    cargo upgrade [OPTIONS] [DEP_ID]...
-
-ARGS:
-    <DEP_ID>...    Crates to be upgraded
+    cargo upgrade [OPTIONS]
 
 OPTIONS:
-        --all                     [deprecated in favor of `--workspace`]
         --dry-run                 Print changes to be made without making them
         --exclude <EXCLUDE>       Crates to exclude and not upgrade
     -h, --help                    Print help information
         --locked                  Require `Cargo.toml` to be up to date
         --manifest-path <PATH>    Path to the manifest to upgrade
         --offline                 Run without accessing the network
-    -p, --package <PKGID>         Package id of the crate to add this dependency to
+    -p, --package <PKGID>         Crate to be upgraded
         --pinned                  Upgrade dependencies pinned in the manifest
         --to-lockfile             Upgrade all packages to the version in the lockfile
     -v, --verbose                 Use verbose output
     -V, --version                 Print version information
-        --workspace               Upgrade all packages in the workspace
     -Z <FLAG>                     Unstable (nightly-only) flags
 
 To only update Cargo.lock, see `cargo update`.

src/bin/upgrade/upgrade.rs

@@ -1,17 +1,17 @@
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
-use std::io::Write;
+use std::path::Path;
 use std::path::PathBuf;
 
 use cargo_edit::{
-    colorize_stderr, find, get_latest_dependency, registry_url, resolve_manifests, set_dep_version,
-    shell_note, shell_status, shell_warn, shell_write_stderr, update_registry_index, CargoResult,
-    Context, CrateSpec, Dependency, LocalManifest,
+    find, get_latest_dependency, registry_url, set_dep_version, shell_note, shell_status,
+    shell_warn, shell_write_stderr, update_registry_index, CargoResult, CrateSpec, Dependency,
+    LocalManifest,
 };
 use clap::Args;
 use indexmap::IndexMap;
 use semver::{Op, VersionReq};
-use termcolor::{Color, ColorSpec, StandardStream, WriteColor};
+use termcolor::{Color, ColorSpec};
 
 /// Upgrade dependency version requirements in Cargo.toml manifest files
 #[derive(Debug, Args)]
@@ -24,36 +24,17 @@ version as recorded in the Cargo.lock file. This flag requires that the Cargo.lo
 up-to-date. If the lock file is missing, or it needs to be updated, cargo-upgrade will exit with \
 an error.")]
 pub struct UpgradeArgs {
-    /// Crates to be upgraded.
-    #[clap(value_name = "DEP_ID")]
-    dependency: Vec<String>,
-
     /// Path to the manifest to upgrade
     #[clap(long, value_name = "PATH", action)]
     manifest_path: Option<PathBuf>,
 
-    /// Package id of the crate to add this dependency to.
-    #[clap(
-        long = "package",
-        short = 'p',
-        value_name = "PKGID",
-        conflicts_with = "all",
-        conflicts_with = "workspace"
-    )]
-    pkgid: Vec<String>,
-
-    /// Upgrade all packages in the workspace.
-    #[clap(
-        long,
-        help = "[deprecated in favor of `--workspace`]",
-        conflicts_with = "workspace",
-        conflicts_with = "pkgid"
-    )]
-    all: bool,
-
-    /// Upgrade all packages in the workspace.
-    #[clap(long, conflicts_with = "all", conflicts_with = "pkgid")]
-    workspace: bool,
+    /// Crate to be upgraded
+    #[clap(long, short, value_name = "PKGID")]
+    package: Vec<String>,
+
+    /// Crates to exclude and not upgrade.
+    #[clap(long)]
+    exclude: Vec<String>,
 
     /// Print changes to be made without making them.
     #[clap(long)]
@@ -71,10 +52,6 @@ pub struct UpgradeArgs {
     #[clap(long)]
     to_lockfile: bool,
 
-    /// Crates to exclude and not upgrade.
-    #[clap(long)]
-    exclude: Vec<String>,
-
     /// Require `Cargo.toml` to be up to date
     #[clap(long)]
     locked: bool,
@@ -93,18 +70,6 @@ impl UpgradeArgs {
         exec(self)
     }
 
-    fn workspace(&self) -> bool {
-        self.all || self.workspace
-    }
-
-    fn resolve_targets(&self) -> CargoResult<Vec<cargo_metadata::Package>> {
-        resolve_manifests(
-            self.manifest_path.as_deref(),
-            self.workspace(),
-            self.pkgid.iter().map(|s| s.as_str()).collect::<Vec<_>>(),
-        )
-    }
-
     fn verbose<F>(&self, mut callback: F) -> CargoResult<()>
     where
         F: FnMut() -> CargoResult<()>,
@@ -123,20 +88,18 @@ enum UnstableOptions {}
 /// Main processing function. Allows us to return a `Result` so that `main` can print pretty error
 /// messages.
 fn exec(args: UpgradeArgs) -> CargoResult<()> {
-    if args.all {
-        deprecated_message("The flag `--all` has been deprecated in favor of `--workspace`")?;
-    }
-
     if !args.offline && !args.to_lockfile {
         let url = registry_url(&find(args.manifest_path.as_deref())?, None)?;
         update_registry_index(&url, false)?;
     }
 
-    let manifests = args.resolve_targets()?;
-    let locked = load_lockfile(&manifests, args.locked, args.offline).unwrap_or_default();
+    let metadata = resolve_ws(args.manifest_path.as_deref(), args.locked, args.offline)?;
+    let manifest_path = metadata.workspace_root.as_std_path().join("Cargo.toml");
+    let manifests = find_ws_members(&metadata);
+    let locked = metadata.packages.clone();
 
     let selected_dependencies = args
-        .dependency
+        .package
         .iter()
         .map(|name| {
             let spec = CrateSpec::resolve(name)?;
@@ -324,7 +287,7 @@ fn exec(args: UpgradeArgs) -> CargoResult<()> {
         if args.locked {
             anyhow::bail!("cannot upgrade due to `--locked`");
         } else {
-            load_lockfile(&manifests, args.locked, args.offline)?;
+            resolve_ws(Some(&manifest_path), args.locked, args.offline)?;
         }
     }
 
@@ -353,19 +316,15 @@ fn exec(args: UpgradeArgs) -> CargoResult<()> {
     Ok(())
 }
 
-fn load_lockfile(
-    targets: &[cargo_metadata::Package],
+fn resolve_ws(
+    manifest_path: Option<&Path>,
     locked: bool,
     offline: bool,
-) -> CargoResult<Vec<cargo_metadata::Package>> {
-    // Get locked dependencies. For workspaces with multiple Cargo.toml
-    // files, there is only a single lockfile, so it suffices to get
-    // metadata for any one of Cargo.toml files.
-    let package = targets
-        .get(0)
-        .ok_or_else(|| anyhow::format_err!("Invalid cargo config"))?;
+) -> CargoResult<cargo_metadata::Metadata> {
     let mut cmd = cargo_metadata::MetadataCommand::new();
-    cmd.manifest_path(package.manifest_path.clone());
+    if let Some(manifest_path) = manifest_path {
+        cmd.manifest_path(manifest_path);
+    }
     cmd.features(cargo_metadata::CargoOpt::AllFeatures);
     let mut other = Vec::new();
     if locked {
@@ -376,11 +335,20 @@ fn load_lockfile(
     }
     cmd.other_options(other);
 
-    let result = cmd.exec()?;
-
-    let locked = result.packages;
+    let ws = cmd.exec().or_else(|_| {
+        cmd.no_deps();
+        cmd.exec()
+    })?;
+    Ok(ws)
+}
 
-    Ok(locked)
+fn find_ws_members(ws: &cargo_metadata::Metadata) -> Vec<cargo_metadata::Package> {
+    let workspace_members: std::collections::HashSet<_> = ws.workspace_members.iter().collect();
+    ws.packages
+        .iter()
+        .filter(|p| workspace_members.contains(&p.id))
+        .cloned()
+        .collect()
 }
 
 fn find_locked_version(
@@ -427,19 +395,6 @@ fn is_pinned_req(old_version_req: &str) -> bool {
     }
 }
 
-fn deprecated_message(message: &str) -> CargoResult<()> {
-    let colorchoice = colorize_stderr();
-    let mut output = StandardStream::stderr(colorchoice);
-    output
-        .set_color(ColorSpec::new().set_fg(Some(Color::Red)).set_bold(true))
-        .with_context(|| "Failed to set output colour")?;
-    writeln!(output, "{}", message).with_context(|| "Failed to write deprecated message")?;
-    output
-        .set_color(&ColorSpec::new())
-        .with_context(|| "Failed to clear output colour")?;
-    Ok(())
-}
-
 struct Dep {
     name: String,
     old_version_req: String,

tests/cargo-upgrade/invalid_dep/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["failure"])
+        .args(["--package", "failure"])
         .current_dir(cwd)
         .assert()
         .failure()

tests/cargo-upgrade/invalid_flag/stderr.log

@@ -3,6 +3,6 @@ error: Found argument '--flag' which wasn't expected, or isn't valid in this con
 	If you tried to supply `--flag` as a value rather than a flag, use `-- --flag`
 
 USAGE:
-    cargo upgrade [OPTIONS] [DEP_ID]...
+    cargo upgrade [OPTIONS]
 
 For more information try --help

tests/cargo-upgrade/invalid_manifest/stderr.log

@@ -1,16 +1,13 @@
     Updating '[ROOTURL]/registry' index
-Error: Invalid manifest
+Error: `cargo metadata` exited with an error: error: failed to parse manifest at `[ROOT]/case/Cargo.toml`
 
 Caused by:
-    `cargo metadata` exited with an error: error: failed to parse manifest at `[ROOT]/case/Cargo.toml`
-
-    Caused by:
-      could not parse input as TOML
-
-    Caused by:
-      TOML parse error at line 1, column 6
-        |
-      1 | This is clearly not a valid Cargo.toml.
-        |      ^
-      Unexpected `i`
-      Expected `.` or `=`
+  could not parse input as TOML
+
+Caused by:
+  TOML parse error at line 1, column 6
+    |
+  1 | This is clearly not a valid Cargo.toml.
+    |      ^
+  Unexpected `i`
+  Expected `.` or `=`

tests/cargo-upgrade/invalid_workspace_root_manifest/mod.rs

@@ -14,7 +14,6 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["--workspace"])
         .current_dir(cwd)
         .assert()
         .code(1)

tests/cargo-upgrade/invalid_workspace_root_manifest/stderr.log

@@ -1,11 +1,6 @@
     Updating '[ROOTURL]/registry' index
-Error: Invalid manifest
+Error: `cargo metadata` exited with an error: error: failed to parse manifest at `[ROOT]/case/Cargo.toml`
 
 Caused by:
-    `cargo metadata` exited with an error: error: failed to parse manifest at `[ROOT]/case/Cargo.toml`
-
-    Caused by:
-      could not parse input as TOML
-
-    Caused by:
+  could not parse input as TOML
 ...

tests/cargo-upgrade/preserve_precision_major/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["--pinned", "--verbose"])
+        .args(["--verbose", "--pinned"])
         .current_dir(cwd)
         .assert()
         .success()

tests/cargo-upgrade/preserve_precision_minor/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["--pinned", "--verbose"])
+        .args(["--verbose", "--pinned"])
         .current_dir(cwd)
         .assert()
         .success()

tests/cargo-upgrade/preserve_precision_patch/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["--pinned", "--verbose"])
+        .args(["--verbose", "--pinned"])
         .current_dir(cwd)
         .assert()
         .success()

tests/cargo-upgrade/specified/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["my-package1"])
+        .args(["--package", "my-package1"])
         .current_dir(cwd)
         .assert()
         .success()

tests/cargo-upgrade/to_lockfile/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["--workspace", "--to-lockfile", "--verbose"])
+        .args(["--to-lockfile", "--verbose"])
         .current_dir(cwd)
         .assert()
         .success()

tests/cargo-upgrade/to_lockfile/stderr.log

@@ -1,3 +1,7 @@
+    Checking four's dependencies
+name       old req locked latest    new req
+====       ======= ====== ======    =======
+my-package 0.2.0   0.2.3  99999.0.0 0.2.3  
     Checking one's dependencies
 name       old req locked latest    new req
 ====       ======= ====== ======    =======
@@ -10,8 +14,4 @@ my-package 0.2.0   0.2.3  99999.0.0 0.2.3
     Checking two's dependencies
 name       old req locked latest    new req
 ====       ======= ====== ======    =======
-my-package 0.2.0   0.2.3  99999.0.0 0.2.3  
-    Checking four's dependencies
-name       old req locked latest    new req
-====       ======= ====== ======    =======
 my-package 0.2.0   0.2.3  99999.0.0 0.2.3  

tests/cargo-upgrade/to_version/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["[email protected]", "--verbose"])
+        .args(["--package", "[email protected]", "--verbose"])
         .current_dir(cwd)
         .assert()
         .success()

tests/cargo-upgrade/upgrade_all/mod.rs

@@ -14,7 +14,7 @@ fn case() {
 
     snapbox::cmd::Command::cargo_ui()
         .arg("upgrade")
-        .args(["--all", "--verbose"])
+        .args(["--verbose"])
         .current_dir(cwd)
         .assert()
         .success()

tests/cargo-upgrade/upgrade_all/stderr.log

@@ -1,5 +1,8 @@
-The flag `--all` has been deprecated in favor of `--workspace`
     Updating '[ROOTURL]/registry' index
+    Checking four's dependencies
+name       old req locked latest    new req  
+====       ======= ====== ======    =======  
+my-package 0.2.0   0.2.3  99999.0.0 99999.0.0
     Checking one's dependencies
 name       old req locked latest    new req  
 ====       ======= ====== ======    =======  
@@ -12,8 +15,4 @@ my-package 0.2.0   0.2.3  99999.0.0 99999.0.0
     Checking two's dependencies
 name       old req locked latest    new req  
 ====       ======= ====== ======    =======  
-my-package 0.2.0   0.2.3  99999.0.0 99999.0.0
-    Checking four's dependencies
-name       old req locked latest    new req  
-====       ======= ====== ======    =======  
 my-package 0.2.0   0.2.3  99999.0.0 99999.0.0