From 863c00a5415994adf18a36295f302b5b946f9745 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Wed, 20 Apr 2022 09:53:35 -0500
Subject: [PATCH] feat(install): Support `foo@version` like cargo-add

In #10472, cargo-add was merged with support for an inline version
syntax of `foo@version`.  That also served as the change proposal for
extending that syntax to `cargo install` for convinience and consistency.

While both commands are specifying a version-req, `cargo-install` has an
implicit-but-required `=` operand while `cargo-add` allows any operand.

This doesn't use the full `pkgid` syntax because that allows syntax that
is unsupported here.

This doesn't use `cargo-add`s parser because that is for version reqs.

I held off on reusing the parser from `cargo-yank` because they had
different type system needs and the level of duplication didn't seem
worth it (see Rule of Three).
---
 src/bin/cargo/commands/install.rs | 22 ++++++++++++--
 tests/testsuite/install.rs        | 49 ++++++++++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs
index f8410e62517..82dc31fc158 100644
--- a/src/bin/cargo/commands/install.rs
+++ b/src/bin/cargo/commands/install.rs
@@ -101,8 +101,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let krates = args
         .values_of("crate")
         .unwrap_or_default()
-        .map(|k| (k, version))
-        .collect::<Vec<_>>();
+        .map(|k| resolve_crate(k, version))
+        .collect::<crate::CargoResult<Vec<_>>>()?;
 
     let mut from_cwd = false;
 
@@ -174,3 +174,21 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     }
     Ok(())
 }
+
+fn resolve_crate<'k>(
+    mut krate: &'k str,
+    mut version: Option<&'k str>,
+) -> crate::CargoResult<(&'k str, Option<&'k str>)> {
+    if let Some((k, v)) = krate.split_once('@') {
+        if version.is_some() {
+            anyhow::bail!("cannot specify both `@{v}` and `--version`");
+        }
+        if k.is_empty() {
+            // by convention, arguments starting with `@` are response files
+            anyhow::bail!("missing crate name for `@{v}`");
+        }
+        krate = k;
+        version = Some(v);
+    }
+    Ok((krate, version))
+}
diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs
index c23d9ba5357..eee7c7a563b 100644
--- a/tests/testsuite/install.rs
+++ b/tests/testsuite/install.rs
@@ -1382,7 +1382,7 @@ fn vers_precise() {
 }
 
 #[cargo_test]
-fn version_too() {
+fn version_precise() {
     pkg("foo", "0.1.1");
     pkg("foo", "0.1.2");
 
@@ -1391,6 +1391,53 @@ fn version_too() {
         .run();
 }
 
+#[cargo_test]
+fn inline_version_precise() {
+    pkg("foo", "0.1.1");
+    pkg("foo", "0.1.2");
+
+    cargo_process("install foo@0.1.1")
+        .with_stderr_contains("[DOWNLOADED] foo v0.1.1 (registry [..])")
+        .run();
+}
+
+#[cargo_test]
+fn inline_version_multiple() {
+    pkg("foo", "0.1.0");
+    pkg("foo", "0.1.1");
+    pkg("foo", "0.1.2");
+    pkg("bar", "0.2.0");
+    pkg("bar", "0.2.1");
+    pkg("bar", "0.2.2");
+
+    cargo_process("install foo@0.1.1 bar@0.2.1")
+        .with_stderr_contains("[DOWNLOADED] foo v0.1.1 (registry [..])")
+        .with_stderr_contains("[DOWNLOADED] bar v0.2.1 (registry [..])")
+        .run();
+}
+
+#[cargo_test]
+fn inline_version_without_name() {
+    pkg("foo", "0.1.1");
+    pkg("foo", "0.1.2");
+
+    cargo_process("install @0.1.1")
+        .with_status(101)
+        .with_stderr("error: missing crate name for `@0.1.1`")
+        .run();
+}
+
+#[cargo_test]
+fn inline_and_explicit_version() {
+    pkg("foo", "0.1.1");
+    pkg("foo", "0.1.2");
+
+    cargo_process("install foo@0.1.1 --version 0.1.1")
+        .with_status(101)
+        .with_stderr("error: cannot specify both `@0.1.1` and `--version`")
+        .run();
+}
+
 #[cargo_test]
 fn not_both_vers_and_version() {
     pkg("foo", "0.1.1");