Skip to content

fix(toml): deprecate hyphen lint names and error on duplicates #17051

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

Open
raushan728 wants to merge 4 commits into
base: master
Choose a base branch
from
+96 −7
Open

fix(toml): deprecate hyphen lint names and error on duplicates #17051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88a8dab
test(toml): add baseline tests for hyphenated and duplicate lint names
raushan728 May 30, 2026
38527bb
fix(toml): warn on hyphen lint names
raushan728 May 31, 2026
4bb4f1b
fix(toml): error on duplicate normalized lint names
raushan728 May 31, 2026
0f44606
refactor(toml): normalize lint names before passing to rustc flags
raushan728 May 30, 2026
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
21 changes: 19 additions & 2 deletions src/cargo/util/toml/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2690,7 +2690,23 @@ supported tools: {}",
if tool == "cargo" && !gctx.cli_unstable().cargo_lints {
warn_for_cargo_lint_feature(gctx, warnings);
}
let mut seen_normalized: HashMap<String, String> = HashMap::new();
for (name, config) in lints {
let normalized = name.replace('-', "_");
if name.contains('-') {
warnings.push(format!(
"`lints.{tool}.{name}` is deprecated in favor of \
`lints.{tool}.{normalized}` and will not work in a \
future edition"
));
}
if let Some(existing) = seen_normalized.get(&normalized) {
anyhow::bail!(

@epage epage May 30, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When a commit has "and" in it, it is a good sign it isn't atomic. This is a separate change.

This is also a breaking change. We either need to start as a future incompat warning or deprecate and error in the next edition.

View changes since the review

@raushan728 raushan728 May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Initially had edition-gated error for both but it broke existing crates on 2024 edition.Changed hyphen names to warnings.push only. For duplicates, kept bail! should this follow the same pattern as hyphen names warnings.push for now, error in future edition?

@epage epage Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If it is edition gated then it shouldn't affect 2024 edition.

Note: if we edition-gate an error, we need a hand-written migration in cargo fix

@raushan728 raushan728 Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, gating against 2024 was the bug since that's already shipped.
Which edition should the error land on, since 2024 is taken?

@epage epage May 30, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we generate an error or push an error?

View changes since the review

"duplicate lint `{existing}` in `[lints.{tool}]`, \
conflicts with `{name}`"
);
}
seen_normalized.insert(normalized.clone(), name.to_string());
if let Some((prefix, suffix)) = name.split_once("::") {
if tool == prefix {
anyhow::bail!(
Expand Down Expand Up @@ -2772,10 +2788,11 @@ fn lints_to_rustflags(lints: &manifest::TomlLints) -> CargoResult<Vec<String>> {
manifest::TomlLintLevel::Allow => "--allow",
};

let normalized_name = name.replace('-', "_");
let option = if tool == "rust" {
format!("{flag}={name}")
format!("{flag}={normalized_name}")
} else {
format!("{flag}={tool}::{name}")
format!("{flag}={tool}::{normalized_name}")
Comment on lines +2791 to +2795

@epage epage Jun 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does this commit exist?

View changes since the review

@raushan728 raushan728 Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rustc already accepts both forms in flags, so this isn't a bug fix, just consistency.

};
(
config.priority(),
Expand Down
3 changes: 2 additions & 1 deletion tests/testsuite/lints/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ im-a-teapot = "warn"
foo.cargo("fetch -Zcargo-lints")
.masquerade_as_nightly_cargo(&["cargo-lints", "test-dummy-unstable"])
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.cargo.im-a-teapot` is deprecated in favor of `lints.cargo.im_a_teapot` and will not work in a future edition
[WARNING] unknown lint: `im-a-teapot`
Comment on lines +50 to 51

@epage epage Jun 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something isn't working right with this

View changes since the review

--> Cargo.toml:12:1
|
Expand All @@ -55,7 +56,7 @@ im-a-teapot = "warn"
|
= [NOTE] `cargo::unknown_lints` is set to `warn` by default
= [HELP] there is a lint with a similar name: `im_a_teapot`
[WARNING] `foo` (manifest) generated 1 warning
[WARNING] `foo` (manifest) generated 2 warnings

"#]])
.run();
Expand Down
11 changes: 8 additions & 3 deletions tests/testsuite/lints/unknown_lints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ this-lint-does-not-exist = "warn"
p.cargo("fetch -Zcargo-lints")
.masquerade_as_nightly_cargo(&["cargo-lints"])
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.cargo.this-lint-does-not-exist` is deprecated in favor of `lints.cargo.this_lint_does_not_exist` and will not work in a future edition
[WARNING] unknown lint: `this-lint-does-not-exist`
Comment on lines +29 to 30

@epage epage Jun 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This lint should likely be changed to this_lint_does_not_exist in a separate commit

View changes since the review

@raushan728 raushan728 Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These use hyphens incidentally to test something unrelated. Will add a prep commit renaming them to underscore first.

--> Cargo.toml:11:1
|
11 | this-lint-does-not-exist = "warn"
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= [NOTE] `cargo::unknown_lints` is set to `warn` in `[lints]`
[WARNING] `foo` (manifest) generated 1 warning
[WARNING] `foo` (manifest) generated 2 warnings

"#]])
.run();
Expand Down Expand Up @@ -73,14 +74,17 @@ workspace = true
p.cargo("fetch -Zcargo-lints")
.masquerade_as_nightly_cargo(&["cargo-lints"])
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.cargo.this-lint-does-not-exist` is deprecated in favor of `lints.cargo.this_lint_does_not_exist` and will not work in a future edition
[WARNING] unknown lint: `this-lint-does-not-exist`
--> Cargo.toml:8:1
|
8 | this-lint-does-not-exist = "warn"
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= [NOTE] `cargo::unknown_lints` is set to `warn` in `[lints]`
[WARNING] workspace (manifest) generated 1 warning
[WARNING] workspace (manifest) generated 2 warnings
[WARNING] foo/Cargo.toml: `lints.cargo.this-lint-does-not-exist` is deprecated in favor of `lints.cargo.this_lint_does_not_exist` and will not work in a future edition
[WARNING] `foo` (manifest) generated 1 warning

"#]])
.run();
Expand Down Expand Up @@ -117,14 +121,15 @@ authors = []
p.cargo("fetch -Zcargo-lints")
.masquerade_as_nightly_cargo(&["cargo-lints"])
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.cargo.this-lint-does-not-exist` is deprecated in favor of `lints.cargo.this_lint_does_not_exist` and will not work in a future edition
[WARNING] unknown lint: `this-lint-does-not-exist`
--> Cargo.toml:8:1
|
8 | this-lint-does-not-exist = "warn"
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= [NOTE] `cargo::unknown_lints` is set to `warn` in `[lints]`
[WARNING] workspace (manifest) generated 1 warning
[WARNING] workspace (manifest) generated 2 warnings
[WARNING] missing `[lints]` to inherit `[workspace.lints]`
--> foo/Cargo.toml
= [NOTE] `cargo::missing_lints_inheritance` is set to `warn` by default
Expand Down
68 changes: 67 additions & 1 deletion tests/testsuite/lints_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,11 @@ fn warn_on_unused_key() {

foo.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.rust.rust-2018-idioms` is deprecated in favor of `lints.rust.rust_2018_idioms` and will not work in a future edition
[WARNING] Cargo.toml: unused manifest key: `lints.rust.rust-2018-idioms.unused`
[WARNING] Cargo.toml: `lints.rust.rust-2018-idioms` is deprecated in favor of `lints.rust.rust_2018_idioms` and will not work in a future edition
[WARNING] Cargo.toml: unused manifest key: `lints.rust.rust-2018-idioms.unused`
[WARNING] `foo` (manifest) generated 2 warnings
[WARNING] `foo` (manifest) generated 4 warnings
[CHECKING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -720,6 +722,8 @@ pub const Ĕ: i32 = 2;

foo.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.rust.confusable-idents` is deprecated in favor of `lints.rust.confusable_idents` and will not work in a future edition

@epage epage Jun 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In a separate commit, we should change this to confusable_idents

View changes since the review

@raushan728 raushan728 Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same here, will fold into the prep commit.

[WARNING] `foo` (manifest) generated 1 warning
[CHECKING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand All @@ -728,6 +732,8 @@ pub const Ĕ: i32 = 2;

foo.cargo("test --doc")
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.rust.confusable-idents` is deprecated in favor of `lints.rust.confusable_idents` and will not work in a future edition
[WARNING] `foo` (manifest) generated 1 warning
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[DOCTEST] foo
Expand Down Expand Up @@ -843,3 +849,63 @@ im_a_teapot = "warn"
"#]])
.run();
}

#[cargo_test]
fn hyphen_in_lint_name() {
let foo = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2015"

[lints.rust]
unexpected-cfgs = "allow"
"#,
)
.file("src/lib.rs", "")
.build();

foo.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] Cargo.toml: `lints.rust.unexpected-cfgs` is deprecated in favor of `lints.rust.unexpected_cfgs` and will not work in a future edition
[WARNING] `foo` (manifest) generated 1 warning
[CHECKING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

"#]])
.run();
}

#[cargo_test]
fn duplicate_lint_name_hyphen_and_underscore() {
let foo = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2015"

[lints.rust]
unexpected_cfgs = "warn"
unexpected-cfgs = "allow"
"#,
)
.file("src/lib.rs", "")
.build();

foo.cargo("check")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`

Caused by:
duplicate lint `unexpected-cfgs` in `[lints.rust]`, conflicts with `unexpected_cfgs`

"#]])
.run();
}
Loading