Skip to content

Commit ddb37b0

Browse files
authored
fix(resolver): Better match rustc in error styling (#16795)
### What does this PR try to resolve? The original motivation was to find ways to make some of the suggestions stand out more but we generally had a wording and organization mismatch with rustc See [#t-cargo > wrong missing feature reported with `target`-gated deps @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/wrong.20missing.20feature.20reported.20with.20.60target.60-gated.20deps/near/581695948) ### How to test and review this PR?
2 parents 99bf958 + df9bd97 commit ddb37b0

File tree

16 files changed

+124
-104
lines changed

16 files changed

+124
-104
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.

crates/resolver-tests/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ cargo-platform.workspace = true
1010
cargo-util-schemas.workspace = true
1111
cargo-util.workspace = true
1212
proptest.workspace = true
13+
snapbox.workspace = true
1314
varisat.workspace = true
1415

1516
[lints]

crates/resolver-tests/tests/resolve.rs

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use cargo::core::Dependency;
22
use cargo::core::dependency::DepKind;
33
use cargo::util::GlobalContext;
4+
use snapbox::assert_data_eq;
5+
use snapbox::str;
46

57
use resolver_tests::{
68
helpers::{
@@ -991,12 +993,15 @@ fn cyclic_good_error_message() {
991993
let reg = registry(input);
992994
let error = resolve(vec![dep("A"), dep("B")], &reg).unwrap_err();
993995
println!("{}", error);
994-
assert_eq!("\
996+
assert_data_eq!(
997+
error.to_string(),
998+
str![[r#"
995999
cyclic package dependency: package `A v0.0.0 (registry `https://example.com/`)` depends on itself. Cycle:
9961000
package `A v0.0.0 (registry `https://example.com/`)`
997-
... which satisfies dependency `A = \"*\"` of package `C v0.0.0 (registry `https://example.com/`)`
998-
... which satisfies dependency `C = \"*\"` of package `A v0.0.0 (registry `https://example.com/`)`\
999-
", error.to_string());
1001+
... which satisfies dependency `A = "*"` of package `C v0.0.0 (registry `https://example.com/`)`
1002+
... which satisfies dependency `C = "*"` of package `A v0.0.0 (registry `https://example.com/`)`
1003+
"#]]
1004+
);
10001005
}
10011006

10021007
#[test]
@@ -1012,22 +1017,22 @@ fn shortest_path_in_error_message() {
10121017
];
10131018
let error = resolve(vec![dep("A")], &registry(input)).unwrap_err();
10141019
println!("{}", error);
1015-
assert_eq!(
1016-
"\
1020+
assert_data_eq!(
1021+
error.to_string(),
1022+
str![[r#"
10171023
failed to select a version for `F`.
10181024
... required by package `A v1.0.0 (registry `https://example.com/`)`
1019-
... which satisfies dependency `A = \"*\"` of package `root v1.0.0 (registry `https://example.com/`)`
1025+
... which satisfies dependency `A = "*"` of package `root v1.0.0 (registry `https://example.com/`)`
10201026
versions that meet the requirements `<=0.1.1` are: 0.1.1, 0.1.0
10211027
1022-
all possible versions conflict with previously selected packages.
1028+
all possible versions conflict with previously selected packages
10231029
10241030
previously selected package `F v0.1.2 (registry `https://example.com/`)`
1025-
... which satisfies dependency `F = \"^0.1.2\"` of package `E v1.0.0 (registry `https://example.com/`)`
1026-
... which satisfies dependency `E = \"*\"` of package `A v1.0.0 (registry `https://example.com/`)`
1027-
... which satisfies dependency `A = \"*\"` of package `root v1.0.0 (registry `https://example.com/`)`
1031+
... which satisfies dependency `F = "^0.1.2"` of package `E v1.0.0 (registry `https://example.com/`)`
1032+
... which satisfies dependency `E = "*"` of package `A v1.0.0 (registry `https://example.com/`)`
1033+
... which satisfies dependency `A = "*"` of package `root v1.0.0 (registry `https://example.com/`)`
10281034
1029-
failed to select a version for `F` which could resolve this conflict\
1030-
",
1031-
error.to_string()
1035+
failed to select a version for `F` which could resolve this conflict
1036+
"#]]
10321037
);
10331038
}

src/cargo/core/resolver/errors.rs

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,11 @@ pub(super) fn activation_error(
145145
msg.push_str(link);
146146
msg.push_str("` as well:\n");
147147
msg.push_str(&describe_path_in_context(resolver_ctx, p));
148-
msg.push_str("\nOnly one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. ");
149-
msg.push_str("Try to adjust your dependencies so that only one package uses the `links = \"");
148+
msg.push_str("\nnote: only one package in the dependency graph may specify the same links value to ensure that only one copy of a native library is linked in the final binary");
149+
msg.push_str("\nfor more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links");
150+
msg.push_str("\nhelp: try to adjust your dependencies so that only one package uses the `links = \"");
150151
msg.push_str(link);
151-
msg.push_str("\"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.");
152+
msg.push_str("\"` value");
152153
}
153154
ConflictReason::MissingFeature(feature) => {
154155
msg.push_str("\n\npackage `");
@@ -162,16 +163,14 @@ pub(super) fn activation_error(
162163
msg.push_str("` does not have that feature.\n");
163164
let latest = candidates.last().expect("in the non-empty branch");
164165
if let Some(closest) = closest(feature, latest.features().keys(), |k| k) {
165-
msg.push_str(" package `");
166-
msg.push_str(&*dep.package_name());
167-
msg.push_str("` does have feature `");
166+
msg.push_str("help: there is a feature `");
168167
msg.push_str(closest);
169-
msg.push_str("`\n");
168+
msg.push_str("` with a similar name\n");
170169
} else if !latest.features().is_empty() {
171170
let mut features: Vec<_> =
172171
latest.features().keys().map(|f| f.as_str()).collect();
173172
features.sort();
174-
msg.push_str(" available features: ");
173+
msg.push_str("help: available features: ");
175174
msg.push_str(&features.join(", "));
176175
msg.push_str("\n");
177176
}
@@ -188,7 +187,7 @@ pub(super) fn activation_error(
188187
msg.push_str(&*dep.package_name());
189188
msg.push_str("` does not have that feature.\n");
190189
msg.push_str(
191-
" A required dependency with that name exists, \
190+
"note: a required dependency with that name exists, \
192191
but only optional dependencies can be used as features.\n",
193192
);
194193
// p == parent so the full path is redundant.
@@ -204,7 +203,7 @@ pub(super) fn activation_error(
204203
msg.push_str(&*dep.package_name());
205204
msg.push_str("` does not have that feature.\n");
206205
msg.push_str(
207-
" An optional dependency with that name exists, \
206+
"note: an optional dependency with that name exists, \
208207
but that dependency uses the \"dep:\" \
209208
syntax in the features table, so it does not have an \
210209
implicit feature with that name.\n",
@@ -216,7 +215,7 @@ pub(super) fn activation_error(
216215

217216
if has_semver {
218217
// Group these errors together.
219-
msg.push_str("\n\nall possible versions conflict with previously selected packages.");
218+
msg.push_str("\n\nall possible versions conflict with previously selected packages");
220219
for (p, r) in &conflicting_activations {
221220
if let ConflictReason::Semver = r {
222221
msg.push_str("\n\n previously selected ");
@@ -336,7 +335,7 @@ pub(super) fn activation_error(
336335
if let Some(pre) = candidates.iter().find(|c| c.version().is_prerelease()) {
337336
let _ = write!(
338337
&mut hints,
339-
"\nif you are looking for the prerelease package it needs to be specified explicitly"
338+
"\nhelp: if you are looking for the prerelease package it needs to be specified explicitly"
340339
);
341340
let _ = write!(
342341
&mut hints,
@@ -352,35 +351,35 @@ pub(super) fn activation_error(
352351
if dep.source_id().is_path() && dep.version_req().is_locked() {
353352
let _ = write!(
354353
&mut hints,
355-
"\nconsider running `cargo update` to update \
356-
a path dependency's locked version",
354+
"\nhelp: to update a path dependency's locked version, run `cargo update`",
357355
);
358356
}
359357

360358
if registry.is_replaced(dep.source_id()) {
361359
let _ = write!(
362360
&mut hints,
363-
"\nperhaps a crate was updated and forgotten to be re-vendored?"
361+
"\nnote: perhaps a crate was updated and forgotten to be re-vendored?"
364362
);
365363
}
366364
} else if let Some(name_candidates) = alt_names(registry, dep) {
367365
let name_candidates = match name_candidates {
368366
Ok(c) => c,
369367
Err(e) => return to_resolve_err(e),
370368
};
371-
let _ = writeln!(&mut msg, "no matching package found",);
372-
let _ = writeln!(&mut msg, "searched package name: `{}`", dep.package_name());
369+
let _ = writeln!(
370+
&mut msg,
371+
"no matching package named `{}` found",
372+
dep.package_name()
373+
);
374+
373375
let mut names = name_candidates
374376
.iter()
375377
.take(3)
376378
.map(|c| c.1.name().as_str())
377379
.collect::<Vec<_>>();
378-
379380
if name_candidates.len() > 3 {
380381
names.push("...");
381382
}
382-
// Vertically align first suggestion with missing crate name
383-
// so a typo jumps out at you.
384383
let suggestions =
385384
names
386385
.iter()
@@ -390,7 +389,10 @@ pub(super) fn activation_error(
390389
i if names.len() - 1 == i && name_candidates.len() <= 3 => acc + " or " + el,
391390
_ => acc + ", " + el,
392391
});
393-
let _ = writeln!(&mut msg, "perhaps you meant: {suggestions}");
392+
let _ = writeln!(
393+
&mut hints,
394+
"\nhelp: packages with similar names: {suggestions}"
395+
);
394396
} else {
395397
let _ = writeln!(
396398
&mut msg,
@@ -414,10 +416,10 @@ pub(super) fn activation_error(
414416
if let Some(offline_flag) = gctx.offline_flag() {
415417
let _ = write!(
416418
&mut hints,
417-
"\nAs a reminder, you're using offline mode ({offline_flag}) \
418-
which can sometimes cause surprising resolution failures, \
419-
if this error is too confusing you may wish to retry \
420-
without `{offline_flag}`.",
419+
"\nnote: offline mode (via `{offline_flag}`) \
420+
can sometimes cause surprising resolution failures\
421+
\nhelp: if this error is too confusing you may wish to retry \
422+
without `{offline_flag}`",
421423
);
422424
}
423425
}

tests/testsuite/build.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,7 +1412,7 @@ fn incompatible_dependencies() {
14121412
... which satisfies dependency `qux = "^0.1.0"` of package `foo v0.0.1 ([ROOT]/foo)`
14131413
versions that meet the requirements `>=1.0.1` are: 1.0.2, 1.0.1
14141414
1415-
all possible versions conflict with previously selected packages.
1415+
all possible versions conflict with previously selected packages
14161416
14171417
previously selected package `bad v1.0.0`
14181418
... which satisfies dependency `bad = "=1.0.0"` of package `baz v0.1.0`
@@ -1459,7 +1459,7 @@ fn incompatible_dependencies_with_multi_semver() {
14591459
... required by package `foo v0.0.1 ([ROOT]/foo)`
14601460
versions that meet the requirements `>=1.0.1, <=2.0.0` are: 2.0.0, 1.0.1
14611461
1462-
all possible versions conflict with previously selected packages.
1462+
all possible versions conflict with previously selected packages
14631463
14641464
previously selected package `bad v2.0.1`
14651465
... which satisfies dependency `bad = ">=2.0.1"` of package `baz v0.1.0`

tests/testsuite/build_script.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,7 +1264,9 @@ versions that meet the requirements `*` are: 0.5.0
12641264
12651265
package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
12661266
package `foo v0.5.0 ([ROOT]/foo)`
1267-
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the `links = "a"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.
1267+
[NOTE] only one package in the dependency graph may specify the same links value to ensure that only one copy of a native library is linked in the final binary
1268+
for more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links
1269+
[HELP] try to adjust your dependencies so that only one package uses the `links = "a"` value
12681270
12691271
failed to select a version for `a-sys` which could resolve this conflict
12701272
@@ -1391,7 +1393,9 @@ versions that meet the requirements `*` are: 0.5.0
13911393
13921394
package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
13931395
package `foo v0.5.0 ([ROOT]/foo)`
1394-
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the `links = "a"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.
1396+
[NOTE] only one package in the dependency graph may specify the same links value to ensure that only one copy of a native library is linked in the final binary
1397+
for more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links
1398+
[HELP] try to adjust your dependencies so that only one package uses the `links = "a"` value
13951399
13961400
failed to select a version for `a-sys` which could resolve this conflict
13971401
@@ -5419,7 +5423,9 @@ versions that meet the requirements `*` are: 0.5.0
54195423
54205424
package `a` links to the native library `a`, but it conflicts with a previous package which links to `a` as well:
54215425
package `foo v0.5.0 ([ROOT]/foo)`
5422-
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust your dependencies so that only one package uses the `links = "a"` value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.
5426+
[NOTE] only one package in the dependency graph may specify the same links value to ensure that only one copy of a native library is linked in the final binary
5427+
for more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links
5428+
[HELP] try to adjust your dependencies so that only one package uses the `links = "a"` value
54235429
54245430
failed to select a version for `a` which could resolve this conflict
54255431

tests/testsuite/direct_minimal_versions.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ fn mixed_dependencies() {
8484
... required by package `foo v0.0.1 ([ROOT]/foo)`
8585
versions that meet the requirements `^1.1` are: 1.1.0
8686
87-
all possible versions conflict with previously selected packages.
87+
all possible versions conflict with previously selected packages
8888
8989
previously selected package `dep v1.0.0`
9090
... which satisfies dependency `dep = "^1.0"` of package `foo v0.0.1 ([ROOT]/foo)`
@@ -244,7 +244,7 @@ fn indirect_conflict() {
244244
... which satisfies dependency `direct = "^1.0"` of package `foo v0.0.1 ([ROOT]/foo)`
245245
versions that meet the requirements `^2.1` are: 2.2.0, 2.1.0
246246
247-
all possible versions conflict with previously selected packages.
247+
all possible versions conflict with previously selected packages
248248
249249
previously selected package `indirect v2.0.0`
250250
... which satisfies dependency `indirect = "^2.0"` of package `foo v0.0.1 ([ROOT]/foo)`

tests/testsuite/directory.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,10 @@ fn simple_install_fail() {
196196
To reuse those artifacts with a future compilation, set the environment variable `CARGO_BUILD_BUILD_DIR` to that path.
197197
198198
Caused by:
199-
no matching package found
200-
searched package name: `baz`
201-
perhaps you meant: bar or foo
199+
no matching package named `baz` found
202200
location searched: directory source `[ROOT]/index` (which is replacing registry `crates-io`)
203201
required by package `bar v0.1.0`
202+
[HELP] packages with similar names: bar or foo
204203
205204
"#]])
206205
.run();
@@ -778,7 +777,7 @@ Caused by:
778777
candidate versions found which didn't match: 0.0.1
779778
location searched: directory source `[..] (which is replacing registry `[..]`)
780779
required by package `bar v0.1.0`
781-
perhaps a crate was updated and forgotten to be re-vendored?
780+
[NOTE] perhaps a crate was updated and forgotten to be re-vendored?
782781
783782
"#]])
784783
.with_status(101)

tests/testsuite/features.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ fn dependency_activates_typoed_feature() {
279279
versions that meet the requirements `*` are: 0.0.1
280280
281281
package `foo` depends on `bar` with feature `bar` but `bar` does not have that feature.
282-
package `bar` does have feature `baz`
282+
[HELP] there is a feature `baz` with a similar name
283283
284284
285285
failed to select a version for `bar` which could resolve this conflict
@@ -330,7 +330,7 @@ fn dependency_activates_feature_with_no_close_match() {
330330
versions that meet the requirements `*` are: 0.0.1
331331
332332
package `foo` depends on `bar` with feature `serde` but `bar` does not have that feature.
333-
available features: cookies, json, tls
333+
[HELP] available features: cookies, json, tls
334334
335335
336336
failed to select a version for `bar` which could resolve this conflict
@@ -549,7 +549,7 @@ fn dependency_activates_required_dependency() {
549549
versions that meet the requirements `*` are: 0.0.1
550550
551551
package `foo` depends on `bar` with feature `baz` but `bar` does not have that feature.
552-
A required dependency with that name exists, but only optional dependencies can be used as features.
552+
[NOTE] a required dependency with that name exists, but only optional dependencies can be used as features.
553553
554554
555555
failed to select a version for `bar` which could resolve this conflict

tests/testsuite/lockfile_compat.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ dependencies = [
891891
candidate versions found which didn't match: 0.0.1
892892
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
893893
required by package `test v0.0.0 ([ROOT]/foo)`
894-
perhaps a crate was updated and forgotten to be re-vendored?
894+
[NOTE] perhaps a crate was updated and forgotten to be re-vendored?
895895
896896
"#]])
897897
.run();

0 commit comments

Comments
 (0)