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

Fix wrong config option being suggested for deprecated wrong_pub_self_convention lint #7382

Merged
merged 1 commit into from
Jun 21, 2021

Conversation

matthiaskrgr
Copy link
Member

Problem:
for code like

#![warn(clippy::wrong_pub_self_convention)]
fn main() {
    println!("Hello, world!");
}

clippy will issue a warning to use a clippy.toml option instead:

warning: lint `clippy::wrong_pub_self_convention` has been removed: set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items
 --> src/main.rs:2:9
  |
2 | #![warn(clippy::wrong_pub_self_convention)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(renamed_and_removed_lints)]` on by default

But using the lint name as seen in the warning message
echo "avoid_breaking_exported_api = true\n" > clippy.toml

Will cause an error:

error: error reading Clippy's configuration file `/tmp/clippytest/clippy.toml`: unknown field `avoid_breaking_exported_api`, expected one of `avoid-breaking-exported-api`, ...

Replace the underscores with dashes in the deprecation message.

changelog: avoid_breaking_exported_api: suggest correct clippy config toml option in the deprecation message

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 20, 2021
@matthiaskrgr
Copy link
Member Author

Hmm, apparently cargo dev update_lints will revert my fix 😆

@matthiaskrgr
Copy link
Member Author

Seems that

    file_change |= replace_region_in_file(
        Path::new("clippy_lints/src/lib.rs"),
        "begin deprecated lints",
        "end deprecated lints",
        false,
        update_mode == UpdateMode::Change,
        || gen_deprecated(deprecated_lints.iter()),
    )
    .changed;

is causing this (update_lints.rs)

…_convention lint

Problem:
for code like
````
fn main() {
    println!("Hello, world!");
}
````
clippy will issue a warning to use a clippy.toml option instead:

````
warning: lint `clippy::wrong_pub_self_convention` has been removed: set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items
 --> src/main.rs:2:9
  |
2 | #![warn(clippy::wrong_pub_self_convention)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(renamed_and_removed_lints)]` on by default
````

But using the lint name as seen in the warning message
echo "avoid_breaking_exported_api = true\n" > clippy.toml

Will cause an error:
````
error: error reading Clippy's configuration file `/tmp/clippytest/clippy.toml`: unknown field `avoid_breaking_exported_api`, expected one of `avoid-breaking-exported-api`, ...
````

Replace the underscores with dashes in the deprecation message.

changelog: avoid_breaking_exported_api: suggest correct clippy config toml option in the deprecation message
@matthiaskrgr
Copy link
Member Author

I think i fixed it

@flip1995
Copy link
Member

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Jun 21, 2021

📌 Commit 8276f26 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Jun 21, 2021

⌛ Testing commit 8276f26 with merge 404bd1a...

@bors
Copy link
Collaborator

bors commented Jun 21, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 404bd1a to master...

@bors bors merged commit 404bd1a into rust-lang:master Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants