From 77f54cc658322e5922d623099ab81167847ded64 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 21 Apr 2025 18:03:48 +0200 Subject: [PATCH 1/4] test(add): add test to show current behaviour of cargo install when @ symbol is missing for the version --- tests/testsuite/install.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 714ef7e5b1c..123958740d8 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -396,6 +396,19 @@ fn bad_version() { .run(); } +#[cargo_test] +fn missing_at_symbol_before_version() { + pkg("foo", "0.0.1"); + cargo_process("install foo=0.2.0") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[ERROR] could not find `foo=0.2.0` in registry `crates-io` with version `*` + +"#]]) + .run(); +} + #[cargo_test] fn bad_paths() { cargo_process("install") From ab0d6a7bd298acbb2c6ab1689be49cee753cf409 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 21 Apr 2025 18:05:02 +0200 Subject: [PATCH 2/4] test(add): add test to show current behaviour of cargo add when @ symbol is missing for the version --- .../cargo_add/missing_at_in_crate_spec/in | 1 + .../cargo_add/missing_at_in_crate_spec/mod.rs | 24 +++++++++++++++++ .../missing_at_in_crate_spec/out/Cargo.toml | 6 +++++ .../missing_at_in_crate_spec/stderr.term.svg | 27 +++++++++++++++++++ tests/testsuite/cargo_add/mod.rs | 1 + 5 files changed, 59 insertions(+) create mode 120000 tests/testsuite/cargo_add/missing_at_in_crate_spec/in create mode 100644 tests/testsuite/cargo_add/missing_at_in_crate_spec/mod.rs create mode 100644 tests/testsuite/cargo_add/missing_at_in_crate_spec/out/Cargo.toml create mode 100644 tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg diff --git a/tests/testsuite/cargo_add/missing_at_in_crate_spec/in b/tests/testsuite/cargo_add/missing_at_in_crate_spec/in new file mode 120000 index 00000000000..7c0824e33ec --- /dev/null +++ b/tests/testsuite/cargo_add/missing_at_in_crate_spec/in @@ -0,0 +1 @@ +../add-basic.in/ \ No newline at end of file diff --git a/tests/testsuite/cargo_add/missing_at_in_crate_spec/mod.rs b/tests/testsuite/cargo_add/missing_at_in_crate_spec/mod.rs new file mode 100644 index 00000000000..7ccc5d7aabc --- /dev/null +++ b/tests/testsuite/cargo_add/missing_at_in_crate_spec/mod.rs @@ -0,0 +1,24 @@ +use cargo_test_support::compare::assert_ui; +use cargo_test_support::current_dir; +use cargo_test_support::file; +use cargo_test_support::prelude::*; +use cargo_test_support::str; +use cargo_test_support::Project; + +#[cargo_test] +fn case() { + let project = Project::from_template(current_dir!().join("in")); + let project_root = project.root(); + let cwd = &project_root; + + snapbox::cmd::Command::cargo_ui() + .arg("add") + .arg_line("my-package=1.0.0") + .current_dir(cwd) + .assert() + .code(101) + .stdout_eq(str![""]) + .stderr_eq(file!["stderr.term.svg"]); + + assert_ui().subset_matches(current_dir!().join("out"), &project_root); +} diff --git a/tests/testsuite/cargo_add/missing_at_in_crate_spec/out/Cargo.toml b/tests/testsuite/cargo_add/missing_at_in_crate_spec/out/Cargo.toml new file mode 100644 index 00000000000..946b7c86bf0 --- /dev/null +++ b/tests/testsuite/cargo_add/missing_at_in_crate_spec/out/Cargo.toml @@ -0,0 +1,6 @@ +[workspace] + +[package] +name = "cargo-list-test-fixture" +version = "0.0.0" +edition = "2015" diff --git a/tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg b/tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg new file mode 100644 index 00000000000..3cfdede0932 --- /dev/null +++ b/tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg @@ -0,0 +1,27 @@ + + + + + + + error: invalid character `=` in package name: `my-package=1.0.0`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters) + + + + + + diff --git a/tests/testsuite/cargo_add/mod.rs b/tests/testsuite/cargo_add/mod.rs index a2881cfbca2..799d1e01330 100644 --- a/tests/testsuite/cargo_add/mod.rs +++ b/tests/testsuite/cargo_add/mod.rs @@ -65,6 +65,7 @@ mod locked_unchanged; mod lockfile_updated; mod manifest_path_package; mod merge_activated_features; +mod missing_at_in_crate_spec; mod multiple_conflicts_with_features; mod multiple_conflicts_with_rename; mod namever; From 98326ac0f554af5db977147c8d912879edde09e4 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 21 Apr 2025 18:14:40 +0200 Subject: [PATCH 3/4] feat(install): check if given crate argument would be valid with inserted @ symbol, suggest fixed argument --- Cargo.lock | 1 + Cargo.toml | 1 + src/bin/cargo/commands/install.rs | 17 +++++++++++++++++ tests/testsuite/install.rs | 5 +++-- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c63294ade88..e00832c40df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -376,6 +376,7 @@ dependencies = [ "tracing-subscriber", "unicase", "unicode-width", + "unicode-xid", "url", "walkdir", "windows-sys 0.59.0", diff --git a/Cargo.toml b/Cargo.toml index 8348c65bb88..65bb7ff8732 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -216,6 +216,7 @@ tracing = { workspace = true, features = ["attributes"] } tracing-subscriber.workspace = true unicase.workspace = true unicode-width.workspace = true +unicode-xid.workspace = true url.workspace = true walkdir.workspace = true diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index dac5f73940d..5fefb7adfff 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -8,6 +8,7 @@ use cargo::ops; use cargo::util::IntoUrl; use cargo::util::VersionExt; use cargo::CargoResult; +use cargo_util_schemas::manifest::PackageName; use itertools::Itertools; use semver::VersionReq; @@ -133,6 +134,22 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { .collect::>>()?; for (crate_name, _) in krates.iter() { + let package_name = PackageName::new(crate_name); + if !crate_name.contains("@") && package_name.is_err() { + for (idx, ch) in crate_name.char_indices() { + if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') { + let mut suggested_crate_name = crate_name.to_string(); + suggested_crate_name.insert_str(idx, "@"); + if let Ok((_, Some(_))) = parse_crate(&suggested_crate_name.as_str()) { + let err = package_name.unwrap_err(); + return Err( + anyhow::format_err!("{err}\n\n\ + help: if this is meant to be a package name followed by a version, insert an `@` like `{suggested_crate_name}`").into()); + } + } + } + } + if let Some(toolchain) = crate_name.strip_prefix("+") { return Err(anyhow!( "invalid character `+` in package name: `+{toolchain}` diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 123958740d8..ac181b6fc11 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -402,8 +402,9 @@ fn missing_at_symbol_before_version() { cargo_process("install foo=0.2.0") .with_status(101) .with_stderr_data(str![[r#" -[UPDATING] `dummy-registry` index -[ERROR] could not find `foo=0.2.0` in registry `crates-io` with version `*` +[ERROR] invalid character `=` in package name: `foo=0.2.0`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters) + +[HELP] if this is meant to be a package name followed by a version, insert an `@` like `foo@=0.2.0` "#]]) .run(); From 0df8d68cf9e492821b4f006054ff931358f6e846 Mon Sep 17 00:00:00 2001 From: dawe Date: Mon, 21 Apr 2025 18:22:07 +0200 Subject: [PATCH 4/4] feat(add): check if given crate argument would be valid with inserted @ symbol, suggest fixed argument --- src/cargo/ops/cargo_add/crate_spec.rs | 18 +++++++++++++++++- .../missing_at_in_crate_spec/stderr.term.svg | 6 +++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_add/crate_spec.rs b/src/cargo/ops/cargo_add/crate_spec.rs index d8825456c4a..47959b464b0 100644 --- a/src/cargo/ops/cargo_add/crate_spec.rs +++ b/src/cargo/ops/cargo_add/crate_spec.rs @@ -28,7 +28,23 @@ impl CrateSpec { .map(|(n, v)| (n, Some(v))) .unwrap_or((pkg_id, None)); - PackageName::new(name)?; + let package_name = PackageName::new(name); + if !pkg_id.contains("@") && package_name.is_err() { + for (idx, ch) in pkg_id.char_indices() { + if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') { + let mut suggested_pkg_id = pkg_id.to_string(); + suggested_pkg_id.insert_str(idx, "@"); + if let Ok(_) = CrateSpec::resolve(&suggested_pkg_id.as_str()) { + let err = package_name.unwrap_err(); + return Err( + anyhow::format_err!("{err}\n\n\ + help: if this is meant to be a package name followed by a version, insert an `@` like `{suggested_pkg_id}`").into()); + } + } + } + } + + package_name?; if let Some(version) = version { semver::VersionReq::parse(version) diff --git a/tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg b/tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg index 3cfdede0932..b3c52e0c4d6 100644 --- a/tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg +++ b/tests/testsuite/cargo_add/missing_at_in_crate_spec/stderr.term.svg @@ -1,4 +1,4 @@ - +