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.

Sign up for GitHub

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

Jump to bottom

fix(lints): Remove ability to specify - in lint name #13837

Merged
merged 2 commits into from
May 1, 2024
Merged

fix(lints): Remove ability to specify - in lint name #13837

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9159ebb
fix(cargo-lints): Don't rewrite dash to underscore in lint name
Muscraft May 1, 2024
6c08e58
feat(cargo-lints): Add a lint for unknown_lints
Muscraft May 1, 2024
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
13 changes: 4 additions & 9 deletions src/cargo/core/workspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,6 @@ impl<'gctx> Workspace<'gctx> {
.cloned()
.unwrap_or_default()
.into_iter()
.map(|(k, v)| (k.replace('-', "_"), v))
.collect();

for (path, maybe_pkg) in &self.packages.packages {
Expand Down Expand Up @@ -1214,10 +1213,6 @@ impl<'gctx> Workspace<'gctx> {
.get("cargo")
.cloned()
.unwrap_or(manifest::TomlToolLints::default());
let normalized_lints = cargo_lints
.into_iter()
.map(|(name, lint)| (name.replace('-', "_"), lint))
.collect();

// We should only be using workspace lints if the `[lints]` table is
// present in the manifest, and `workspace` is set to `true`
Expand All @@ -1242,7 +1237,7 @@ impl<'gctx> Workspace<'gctx> {
analyze_cargo_lints_table(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
ws_contents,
ws_document,
Expand All @@ -1252,23 +1247,23 @@ impl<'gctx> Workspace<'gctx> {
check_im_a_teapot(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_implicit_features(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
unused_dependencies(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
Expand Down
25 changes: 5 additions & 20 deletions src/cargo/util/lints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,31 +117,21 @@ fn verify_feature_enabled(
gctx: &GlobalContext,
) -> CargoResult<()> {
if !manifest.unstable_features().is_enabled(feature_gate) {
let dash_name = lint_name.replace("_", "-");
let dash_feature_name = feature_gate.name().replace("_", "-");
let title = format!("use of unstable lint `{}`", dash_name);
let title = format!("use of unstable lint `{}`", lint_name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: implicit arg capture (should we force that in clippy?)

Suggested change
let title = format!("use of unstable lint `{}`", lint_name);
let title = format!("use of unstable lint `{lint_name}`");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If its something we bring up in code review but could be automated, then yes

with the caveat on how often we don't want it which I believe is pretty rare for this one.

let label = format!(
"this is behind `{}`, which is not enabled",
dash_feature_name
);
let second_title = format!("`cargo::{}` was inherited", dash_name);
let second_title = format!("`cargo::{}` was inherited", lint_name);
let help = format!(
"consider adding `cargo-features = [\"{}\"]` to the top of the manifest",
dash_feature_name
);
let message = match reason {
LintLevelReason::Package => {
let span = get_span(
manifest.document(),
&["lints", "cargo", dash_name.as_str()],
false,
)
.or(get_span(
manifest.document(),
&["lints", "cargo", lint_name],
false,
))
.unwrap();
let span =
get_span(manifest.document(), &["lints", "cargo", lint_name], false).unwrap();

Level::Error
.title(&title)
Expand All @@ -155,15 +145,10 @@ fn verify_feature_enabled(
}
LintLevelReason::Workspace => {
let lint_span = get_span(
ws_document,
&["workspace", "lints", "cargo", dash_name.as_str()],
false,
)
.or(get_span(
ws_document,
&["workspace", "lints", "cargo", lint_name],
false,
))
)
.unwrap();
let inherit_span_key =
get_span(manifest.document(), &["lints", "workspace"], false).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/lints/implicit_features/edition_2021_warn/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ baz = { version = "0.1.0", optional = true }
target-dep = { version = "0.1.0", optional = true }

[lints.cargo]
implicit-features = "warn"
implicit_features = "warn"
"#,
)
.file("src/lib.rs", "")
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/lints/implicit_features/edition_2024/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ baz = { version = "0.1.0", optional = true }
baz = ["dep:baz"]

[lints.cargo]
unused-optional-dependency = "allow"
unused_optional_dependency = "allow"
"#,
)
.file("src/lib.rs", "")
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/lints/unused_optional_dependencies/edition_2021/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ edition = "2021"
bar = { version = "0.1.0", optional = true }

[lints.cargo]
implicit-features = "allow"
implicit_features = "allow"
"#,
)
.file("src/lib.rs", "")
Expand Down
51 changes: 21 additions & 30 deletions tests/testsuite/lints_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,6 @@ edition = "2015"
authors = []

[lints.cargo]
im-a-teapot = "warn"
"#,
)
.file("src/lib.rs", "")
Expand Down Expand Up @@ -799,7 +798,6 @@ authors = []
im-a-teapot = true

[lints.cargo]
im-a-teapot = "warn"
"#,
)
.file("src/lib.rs", "")
Expand Down Expand Up @@ -835,7 +833,7 @@ authors = []
im-a-teapot = true

[lints.cargo]
im-a-teapot = "warn"
im_a_teapot = "warn"
"#,
)
.file("src/lib.rs", "")
Expand All @@ -860,7 +858,7 @@ warning: `im_a_teapot` is specified
}

#[cargo_test]
fn cargo_lints_underscore_supported() {
fn cargo_lints_dashes_dont_get_rewritten() {
let foo = project()
.file(
"Cargo.toml",
Expand All @@ -875,7 +873,7 @@ authors = []
im-a-teapot = true

[lints.cargo]
im_a_teapot = "warn"
im-a-teapot = "warn"
"#,
)
.file("src/lib.rs", "")
Expand All @@ -885,13 +883,6 @@ im_a_teapot = "warn"
.masquerade_as_nightly_cargo(&["cargo-lints", "test-dummy-unstable"])
.with_stderr(
"\
warning: `im_a_teapot` is specified
--> Cargo.toml:9:1
|
9 | im-a-teapot = true
| ------------------
|
= note: `cargo::im_a_teapot` is set to `warn` in `[lints]`
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] [..]
",
Expand All @@ -915,8 +906,8 @@ authors = []
im-a-teapot = true

[lints.cargo]
im-a-teapot = { level = "warn", priority = 10 }
test-dummy-unstable = { level = "forbid", priority = -1 }
im_a_teapot = { level = "warn", priority = 10 }
test_dummy_unstable = { level = "forbid", priority = -1 }
"#,
)
.file("src/lib.rs", "")
Expand Down Expand Up @@ -948,8 +939,8 @@ fn workspace_cargo_lints() {
cargo-features = ["test-dummy-unstable"]

[workspace.lints.cargo]
im-a-teapot = { level = "warn", priority = 10 }
test-dummy-unstable = { level = "forbid", priority = -1 }
im_a_teapot = { level = "warn", priority = 10 }
test_dummy_unstable = { level = "forbid", priority = -1 }

[package]
name = "foo"
Expand Down Expand Up @@ -992,7 +983,7 @@ fn dont_always_inherit_workspace_lints() {
members = ["foo"]

[workspace.lints.cargo]
im-a-teapot = "warn"
im_a_teapot = "warn"
"#,
)
.file(
Expand Down Expand Up @@ -1038,7 +1029,7 @@ edition = "2021"
baz = { version = "0.1.0", optional = true }

[lints.cargo]
implicit-features = "warn"
implicit_features = "warn"
"#,
)
.file("src/lib.rs", "")
Expand All @@ -1056,7 +1047,7 @@ edition = "2021"
bar = "0.1.0"

[lints.cargo]
implicit-features = "warn"
implicit_features = "warn"
"#,
)
.file("src/lib.rs", "")
Expand Down Expand Up @@ -1091,7 +1082,7 @@ edition = "2015"
authors = []

[lints.cargo]
im-a-teapot = "warn"
im_a_teapot = "warn"
"#,
)
.file("src/lib.rs", "")
Expand All @@ -1102,10 +1093,10 @@ im-a-teapot = "warn"
.with_status(101)
.with_stderr(
"\
error: use of unstable lint `im-a-teapot`
error: use of unstable lint `im_a_teapot`
--> Cargo.toml:9:1
|
9 | im-a-teapot = \"warn\"
9 | im_a_teapot = \"warn\"
| ^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled
|
= help: consider adding `cargo-features = [\"test-dummy-unstable\"]` to the top of the manifest
Expand All @@ -1125,8 +1116,8 @@ fn check_feature_gated_workspace() {
members = ["foo"]

[workspace.lints.cargo]
im-a-teapot = { level = "warn", priority = 10 }
test-dummy-unstable = { level = "forbid", priority = -1 }
im_a_teapot = { level = "warn", priority = 10 }
test_dummy_unstable = { level = "forbid", priority = -1 }
"#,
)
.file(
Expand All @@ -1150,26 +1141,26 @@ workspace = true
.with_status(101)
.with_stderr(
"\
error: use of unstable lint `im-a-teapot`
error: use of unstable lint `im_a_teapot`
--> Cargo.toml:6:1
|
6 | im-a-teapot = { level = \"warn\", priority = 10 }
6 | im_a_teapot = { level = \"warn\", priority = 10 }
| ^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled
|
note: `cargo::im-a-teapot` was inherited
note: `cargo::im_a_teapot` was inherited
--> foo/Cargo.toml:9:1
|
9 | workspace = true
| ----------------
|
= help: consider adding `cargo-features = [\"test-dummy-unstable\"]` to the top of the manifest
error: use of unstable lint `test-dummy-unstable`
error: use of unstable lint `test_dummy_unstable`
--> Cargo.toml:7:1
|
7 | test-dummy-unstable = { level = \"forbid\", priority = -1 }
7 | test_dummy_unstable = { level = \"forbid\", priority = -1 }
| ^^^^^^^^^^^^^^^^^^^ this is behind `test-dummy-unstable`, which is not enabled
|
note: `cargo::test-dummy-unstable` was inherited
note: `cargo::test_dummy_unstable` was inherited
--> foo/Cargo.toml:9:1
|
9 | workspace = true
Expand Down