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

New lint: Unknown clippy lints #3161

Merged
merged 9 commits into from
Nov 2, 2018
Merged

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented Sep 10, 2018

This is the Clippy version of the rustc lint unknown_lints. The behavior of this lint is pretty much the same.

Before this is merged a small change in the compiler needs to be done: CheckLintNameResult needs to be public. See rust-lang/rust#54106

if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
&name.as_str(),
Some(tool_name.as_str()),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

weird indent

cx,
UNKNOWN_CLIPPY_LINTS,
lint.span,
&format!("unknwon clippy lint: clippy::{}", name),
Copy link
Contributor

Choose a reason for hiding this comment

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

unknwon

|db| {
if name.as_str().chars().any(|c| c.is_uppercase()) {
let name_lower = name.as_str().to_lowercase().to_string();
match lint_store.check_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.

I'm guessing there's currently no way to manually iterate the list and do a levensthein search for close matches?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. This is the behavior of the unknown_lints lint of the compiler. This could be an enhancement for this lint AND the compiler lint. (I'm not sure if the performance of this would be bad though)

Copy link
Contributor

Choose a reason for hiding this comment

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

We do have some tricks like checking whether a lint is allowed before doing the heavy work. But yea, we should fix this in the compiler first

Copy link
Member Author

Choose a reason for hiding this comment

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

There is lev_distance in the compiler.

Copy link
Member

Choose a reason for hiding this comment

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

I opened rust-lang/rust#54737 for improving the rustc unknown_lints suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure when I get around to work on Rust's unknown_lints, I'm fine with merging this and creating an issue to improve the suggestions later on.

kennytm added a commit to kennytm/rust that referenced this pull request Sep 12, 2018
…=Manishearth

Reexport CheckLintNameResult

Make the enum `CheckLintNameResult` public, so that lint tools (aka Clippy) can use it together with [`LintStore::check_lint_name`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/struct.LintStore.html#method.check_lint_name), to handle the case that a scoped `tool_lint` doesn't exist in the tool.

This is currently not handled by the compiler:
https://github.com/rust-lang/rust/blob/595345419d12c3ea860151df52f78744a31bafff/src/librustc/lint/levels.rs#L309-L314

Needed for rust-lang/rust-clippy#3161

r? @Manishearth
@flip1995 flip1995 closed this Sep 12, 2018
@flip1995 flip1995 reopened this Sep 12, 2018
clippy_lints/src/attrs.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 force-pushed the unknown_clippy_lints branch from ea1116f to 146ff14 Compare September 15, 2018 09:05
@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 15, 2018
@flip1995 flip1995 force-pushed the unknown_clippy_lints branch 2 times, most recently from 6f00e1c to 1c4221c Compare September 29, 2018 12:58
@flip1995 flip1995 force-pushed the unknown_clippy_lints branch 2 times, most recently from 7d45441 to 167436a Compare October 15, 2018 13:20
clippy_lints/src/attrs.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,8 @@

Copy link
Member

Choose a reason for hiding this comment

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

Should this test also the old cfg_attr attributes?

Copy link
Member Author

@flip1995 flip1995 Nov 1, 2018

Choose a reason for hiding this comment

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

I would recommend to explicitly write a lint for cfg_attr. I actually already did: flip1995@1f9c0a9

The problem is, that it's impossible to lint crate-level cfg_attrs, which makes this lint only semi valuable.

@flip1995 flip1995 force-pushed the unknown_clippy_lints branch from 167436a to 6f584f3 Compare November 1, 2018 19:37
@phansch
Copy link
Member

phansch commented Nov 1, 2018

r=me once the build is working properly again

@phansch
Copy link
Member

phansch commented Nov 2, 2018

bors r+

bors bot added a commit that referenced this pull request Nov 2, 2018
3161: New lint: Unknown clippy lints r=phansch a=flip1995

This is the Clippy version of the `rustc` lint `unknown_lints`. The behavior of this lint is pretty much the same.

Before this is merged a small change in the compiler needs to be done: `CheckLintNameResult` needs to be public. See rust-lang/rust#54106

3387: Replace big if/else expression with match r=flip1995 a=mikerite



3388: RIIR update lints: Generate deprecated lints r=phansch a=phansch

The update script now also generates the 'register_removed' section in
`clippy_lints/src/lib.rs`.

Also, instead of using `let mut store ...`, I added a new identifier
line so that the replacement will continue to work in case `let mut
store ...` ever changes.

cc #2882

Co-authored-by: flip1995 <9744647+flip1995@users.noreply.github.com>
Co-authored-by: flip1995 <hello@philkrones.com>
Co-authored-by: Michael Wright <mikerite@lavabit.com>
Co-authored-by: Philipp Hansch <dev@phansch.net>
@bors
Copy link
Contributor

bors bot commented Nov 2, 2018

Build failed (retrying...)

@phansch
Copy link
Member

phansch commented Nov 2, 2018

This needs an updated stderr flie after adding the copyright notice ©️

bors bot added a commit that referenced this pull request Nov 2, 2018
3161: New lint: Unknown clippy lints r=phansch a=flip1995

This is the Clippy version of the `rustc` lint `unknown_lints`. The behavior of this lint is pretty much the same.

Before this is merged a small change in the compiler needs to be done: `CheckLintNameResult` needs to be public. See rust-lang/rust#54106

3387: Replace big if/else expression with match r=flip1995 a=mikerite



Co-authored-by: flip1995 <9744647+flip1995@users.noreply.github.com>
Co-authored-by: flip1995 <hello@philkrones.com>
Co-authored-by: Michael Wright <mikerite@lavabit.com>
@phansch
Copy link
Member

phansch commented Nov 2, 2018

bors r- (because I think otherwise it gets included in these 'rollup' commits?)

@bors
Copy link
Contributor

bors bot commented Nov 2, 2018

Canceled

@flip1995 flip1995 force-pushed the unknown_clippy_lints branch from 196addb to 32396f6 Compare November 2, 2018 13:01
@flip1995
Copy link
Member Author

flip1995 commented Nov 2, 2018

bors r=phansch

bors bot added a commit that referenced this pull request Nov 2, 2018
3161: New lint: Unknown clippy lints r=phansch a=flip1995

This is the Clippy version of the `rustc` lint `unknown_lints`. The behavior of this lint is pretty much the same.

Before this is merged a small change in the compiler needs to be done: `CheckLintNameResult` needs to be public. See rust-lang/rust#54106

Co-authored-by: flip1995 <9744647+flip1995@users.noreply.github.com>
Co-authored-by: flip1995 <hello@philkrones.com>
@bors
Copy link
Contributor

bors bot commented Nov 2, 2018

@bors bors bot merged commit 32396f6 into rust-lang:master Nov 2, 2018
@flip1995 flip1995 deleted the unknown_clippy_lints branch June 18, 2019 10:07
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.

3 participants