Skip to content

Commit 538c163

Browse files
authored
feat(add/install): check if given crate argument would be valid with inserted @ symbol (#15441)
Suggest to user to use a crate name with an inserted @ before the first invalid package name character Fixes #15318 <!-- Thanks for submitting a pull request 🎉! Here are some tips for you: * If this is your first contribution, read "Cargo Contribution Guide" first: https://doc.crates.io/contrib/ * Run `cargo fmt --all` to format your code changes. * Small commits and pull requests are always preferable and easy to review. * If your idea is large and needs feedback from the community, read how: https://doc.crates.io/contrib/process/#working-on-large-features * Cargo takes care of compatibility. Read our design principles: https://doc.crates.io/contrib/design.html * When changing help text of cargo commands, follow the steps to generate docs: https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages * If your PR is not finished, set it as "draft" PR or add "WIP" in its title. * It's ok to use the CI resources to test your PR, but please don't abuse them. ### What does this PR try to resolve? Explain the motivation behind this change. A clear overview along with an in-depth explanation are helpful. You can use `Fixes #<issue number>` to associate this PR to an existing issue. ### How should we test and review this PR? Demonstrate how you test this change and guide reviewers through your PR. With a smooth review process, a pull request usually gets reviewed quicker. If you don't know how to write and run your tests, please read the guide: https://doc.crates.io/contrib/tests ### Additional information Other information you want to mention in this PR, such as prior arts, future extensions, an unresolved problem, or a TODO list. -->
2 parents a98aec6 + 0df8d68 commit 538c163

File tree

10 files changed

+113
-1
lines changed

10 files changed

+113
-1
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ tracing = { workspace = true, features = ["attributes"] }
216216
tracing-subscriber.workspace = true
217217
unicase.workspace = true
218218
unicode-width.workspace = true
219+
unicode-xid.workspace = true
219220
url.workspace = true
220221
walkdir.workspace = true
221222

src/bin/cargo/commands/install.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use cargo::ops;
88
use cargo::util::IntoUrl;
99
use cargo::util::VersionExt;
1010
use cargo::CargoResult;
11+
use cargo_util_schemas::manifest::PackageName;
1112
use itertools::Itertools;
1213
use semver::VersionReq;
1314

@@ -133,6 +134,22 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
133134
.collect::<crate::CargoResult<Vec<_>>>()?;
134135

135136
for (crate_name, _) in krates.iter() {
137+
let package_name = PackageName::new(crate_name);
138+
if !crate_name.contains("@") && package_name.is_err() {
139+
for (idx, ch) in crate_name.char_indices() {
140+
if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') {
141+
let mut suggested_crate_name = crate_name.to_string();
142+
suggested_crate_name.insert_str(idx, "@");
143+
if let Ok((_, Some(_))) = parse_crate(&suggested_crate_name.as_str()) {
144+
let err = package_name.unwrap_err();
145+
return Err(
146+
anyhow::format_err!("{err}\n\n\
147+
help: if this is meant to be a package name followed by a version, insert an `@` like `{suggested_crate_name}`").into());
148+
}
149+
}
150+
}
151+
}
152+
136153
if let Some(toolchain) = crate_name.strip_prefix("+") {
137154
return Err(anyhow!(
138155
"invalid character `+` in package name: `+{toolchain}`

src/cargo/ops/cargo_add/crate_spec.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,23 @@ impl CrateSpec {
2828
.map(|(n, v)| (n, Some(v)))
2929
.unwrap_or((pkg_id, None));
3030

31-
PackageName::new(name)?;
31+
let package_name = PackageName::new(name);
32+
if !pkg_id.contains("@") && package_name.is_err() {
33+
for (idx, ch) in pkg_id.char_indices() {
34+
if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') {
35+
let mut suggested_pkg_id = pkg_id.to_string();
36+
suggested_pkg_id.insert_str(idx, "@");
37+
if let Ok(_) = CrateSpec::resolve(&suggested_pkg_id.as_str()) {
38+
let err = package_name.unwrap_err();
39+
return Err(
40+
anyhow::format_err!("{err}\n\n\
41+
help: if this is meant to be a package name followed by a version, insert an `@` like `{suggested_pkg_id}`").into());
42+
}
43+
}
44+
}
45+
}
46+
47+
package_name?;
3248

3349
if let Some(version) = version {
3450
semver::VersionReq::parse(version)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../add-basic.in/
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::current_dir;
3+
use cargo_test_support::file;
4+
use cargo_test_support::prelude::*;
5+
use cargo_test_support::str;
6+
use cargo_test_support::Project;
7+
8+
#[cargo_test]
9+
fn case() {
10+
let project = Project::from_template(current_dir!().join("in"));
11+
let project_root = project.root();
12+
let cwd = &project_root;
13+
14+
snapbox::cmd::Command::cargo_ui()
15+
.arg("add")
16+
.arg_line("my-package=1.0.0")
17+
.current_dir(cwd)
18+
.assert()
19+
.code(101)
20+
.stdout_eq(str![""])
21+
.stderr_eq(file!["stderr.term.svg"]);
22+
23+
assert_ui().subset_matches(current_dir!().join("out"), &project_root);
24+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[workspace]
2+
3+
[package]
4+
name = "cargo-list-test-fixture"
5+
version = "0.0.0"
6+
edition = "2015"
Lines changed: 31 additions & 0 deletions
Loading

tests/testsuite/cargo_add/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ mod locked_unchanged;
6565
mod lockfile_updated;
6666
mod manifest_path_package;
6767
mod merge_activated_features;
68+
mod missing_at_in_crate_spec;
6869
mod multiple_conflicts_with_features;
6970
mod multiple_conflicts_with_rename;
7071
mod namever;

tests/testsuite/install.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,20 @@ fn bad_version() {
396396
.run();
397397
}
398398

399+
#[cargo_test]
400+
fn missing_at_symbol_before_version() {
401+
pkg("foo", "0.0.1");
402+
cargo_process("install foo=0.2.0")
403+
.with_status(101)
404+
.with_stderr_data(str![[r#"
405+
[ERROR] invalid character `=` in package name: `foo=0.2.0`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)
406+
407+
[HELP] if this is meant to be a package name followed by a version, insert an `@` like `foo@=0.2.0`
408+
409+
"#]])
410+
.run();
411+
}
412+
399413
#[cargo_test]
400414
fn bad_paths() {
401415
cargo_process("install")

0 commit comments

Comments
 (0)