Skip to content

Commit

Permalink
Auto merge of #11172 - ehuss:namespaced-feature-err-help, r=weihanglo
Browse files Browse the repository at this point in the history
Provide a better error message when mixing dep: with /

Features of the form `dep:foo/feature` aren't accepted as valid syntax. This generated a somewhat confusing error message of:

```
feature `f1` includes `dep:bar/bar-feat`, but `dep:bar` is not a dependency
```

This PR adds a more targeted error message that provides some suggestions on how to fix it.

There is more context in #9574 as to why the syntax is the way it is.
  • Loading branch information
bors committed Oct 3, 2022
2 parents 5181f99 + fc1f5a4 commit 0b84a35
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 0 deletions.
30 changes: 30 additions & 0 deletions src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,36 @@ fn build_feature_map(
feature
);
}

// dep: cannot be combined with /
if let Some(stripped_dep) = dep_name.strip_prefix("dep:") {
let has_other_dep = explicitly_listed.contains(stripped_dep);
let is_optional = dep_map
.get(stripped_dep)
.iter()
.flat_map(|d| d.iter())
.any(|d| d.is_optional());
let extra_help = if *weak || has_other_dep || !is_optional {
// In this case, the user should just remove dep:.
// Note that "hiding" an optional dependency
// wouldn't work with just a single `dep:foo?/bar`
// because there would not be any way to enable
// `foo`.
String::new()
} else {
format!(
"\nIf the intent is to avoid creating an implicit feature \
`{stripped_dep}` for an optional dependency, \
then consider replacing this with two values:\n \
\"dep:{stripped_dep}\", \"{stripped_dep}/{dep_feature}\""
)
};
bail!(
"feature `{feature}` includes `{fv}` with both `dep:` and `/`\n\
To fix this, remove the `dep:` prefix.{extra_help}"
)
}

// Validation of the feature name will be performed in the resolver.
if !is_any_dep {
bail!(
Expand Down
126 changes: 126 additions & 0 deletions tests/testsuite/features_namespaced.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1073,3 +1073,129 @@ feat3 = ["feat2"]
)],
);
}

#[cargo_test]
fn namespaced_feature_together() {
// Check for an error when `dep:` is used with `/`
Package::new("bar", "1.0.0")
.feature("bar-feat", &[])
.publish();

// Non-optional shouldn't have extra err.
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = "1.0"
[features]
f1 = ["dep:bar/bar-feat"]
"#,
)
.file("src/lib.rs", "")
.build();
p.cargo("check")
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `f1` includes `dep:bar/bar-feat` with both `dep:` and `/`
To fix this, remove the `dep:` prefix.
",
)
.run();

// Weak dependency shouldn't have extra err.
p.change_file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = {version = "1.0", optional = true }
[features]
f1 = ["dep:bar?/bar-feat"]
"#,
);
p.cargo("check")
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `f1` includes `dep:bar?/bar-feat` with both `dep:` and `/`
To fix this, remove the `dep:` prefix.
",
)
.run();

// If dep: is already specified, shouldn't have extra err.
p.change_file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = {version = "1.0", optional = true }
[features]
f1 = ["dep:bar", "dep:bar/bar-feat"]
"#,
);
p.cargo("check")
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `f1` includes `dep:bar/bar-feat` with both `dep:` and `/`
To fix this, remove the `dep:` prefix.
",
)
.run();

// Only when the other 3 cases aren't true should it give some extra help.
p.change_file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = {version = "1.0", optional = true }
[features]
f1 = ["dep:bar/bar-feat"]
"#,
);
p.cargo("check")
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `f1` includes `dep:bar/bar-feat` with both `dep:` and `/`
To fix this, remove the `dep:` prefix.
If the intent is to avoid creating an implicit feature `bar` for an optional \
dependency, then consider replacing this with two values:
\"dep:bar\", \"bar/bar-feat\"
",
)
.run();
}

0 comments on commit 0b84a35

Please sign in to comment.