Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a better error message when mixing dep: with / #11172

Merged
merged 1 commit into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}