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

Autogenerate new lints #4994

Merged
merged 3 commits into from
Jan 16, 2020
Merged

Autogenerate new lints #4994

merged 3 commits into from
Jan 16, 2020

Conversation

bradsherman
Copy link
Contributor

@bradsherman bradsherman commented Jan 4, 2020

Add option in clippy_dev to automatically generate boilerplate code for adding new lints

example:

./util/dev new_lint --name=iter_nth_zero --type=late

Fixes #4942

changelog: none

@bradsherman
Copy link
Contributor Author

I will add another commit to update docs once the code is set in stone, but I wanted to push this up before assuming too much.

@bradsherman bradsherman force-pushed the new_lint_gen branch 2 times, most recently from 6fb9158 to 1a3660f Compare January 4, 2020 21:58
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this!

Initial implementation LGTM overall.

clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Only one thing left 🎉

clippy_dev/src/new_lint.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Jan 8, 2020
@bradsherman
Copy link
Contributor Author

Since we seem to be at a good spot with the auto-generated code, I'm going to start working on two more commits for this PR:

  1. Update documentation to reflect new process for adding lints
  2. Add a new lint using said process to make sure no lints have a description of "default lint description"

@flip1995
Copy link
Member

Great! If you have any questions, don't hesitate to ask!

@bradsherman
Copy link
Contributor Author

I've started updating the docs and adding the new lint, but when trying to build I'm getting a ton of errors like this:

error[E0432]: unresolved import `rustc::declare_lint_pass`
 --> clippy_lints/src/utils/author.rs:5:5
  |
5 | use rustc::declare_lint_pass;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^ no `declare_lint_pass` in the root

error[E0432]: unresolved import `syntax::errors`
 --> clippy_lints/src/utils/diagnostics.rs:8:13
  |
8 | use syntax::errors::DiagnosticBuilder;
  |             ^^^^^^ could not find `errors` in `syntax`

error[E0432]: unresolved import `rustc::declare_lint_pass`
 --> clippy_lints/src/utils/inspector.rs:4:5
  |
4 | use rustc::declare_lint_pass;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^ no `declare_lint_pass` in the root

I've installed the latest Rust from master but still no success. Any ideas?

@krishna-veerareddy
Copy link
Contributor

krishna-veerareddy commented Jan 11, 2020

Since the rustc API isn't guaranteed to be stable we often have breaking changes like the ones you see above. For example rustc::declare_lint_pass would now be rustc_session::declare_lint_pass. There is a rustup fix in the process of being merged into clippy that will fix most of the errors.

Here's the rustup PR: #5040. Once this gets merged you would only need to update changes in this PR to be compatible with the latest rustc API. Hope that helps.

@bradsherman
Copy link
Contributor Author

okay cool that makes sense, thank you! I will keep an eye on that PR and proceed once it is merged.

@bors
Copy link
Contributor

bors commented Jan 11, 2020

☔ The latest upstream changes (presumably #5040) made this pull request unmergeable. Please resolve the merge conflicts.

@bradsherman
Copy link
Contributor Author

I'm not sure how to set up the tests for creating an internal lint ie checking to make sure no lints have default lint description. Could someone point me in the right direction please?

@flip1995
Copy link
Member

flip1995 commented Jan 13, 2020

https://github.com/rust-lang/rust-clippy/blob/master/tests/ui/lint_without_lint_pass.rs

Is an example on how to write tests for lint declarations.

Just create a test file with similar imports and declare a lint with the description "default lint description" and one with another description.

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Documentation update looks great!

doc/adding_lints.md Show resolved Hide resolved
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM overrall

clippy_lints/src/utils/internal_lints.rs Outdated Show resolved Hide resolved
clippy_lints/src/utils/internal_lints.rs Outdated Show resolved Hide resolved
- Add option in clippy_dev to automatically generate boilerplate
  code for adding new lints
- Add instructions for adding new lints with
  the new automation
- Lint for any new lints that have the default lint description
  from the automation
@flip1995
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 16, 2020

📌 Commit 32337a9 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Jan 16, 2020

⌛ Testing commit 32337a9 with merge d613fd1...

bors added a commit that referenced this pull request Jan 16, 2020
Autogenerate new lints

Add option in clippy_dev to automatically generate boilerplate code for adding new lints

example:

`./util/dev new_lint --name=iter_nth_zero --type=late`

Fixes #4942
@bors
Copy link
Contributor

bors commented Jan 16, 2020

💔 Test failed - checks-travis

@flip1995 flip1995 added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jan 16, 2020
@flip1995
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Jan 16, 2020

⌛ Testing commit 32337a9 with merge a5750d2...

bors added a commit that referenced this pull request Jan 16, 2020
Autogenerate new lints

Add option in clippy_dev to automatically generate boilerplate code for adding new lints

example:

`./util/dev new_lint --name=iter_nth_zero --type=late`

Fixes #4942

changelog: none
@bors
Copy link
Contributor

bors commented Jan 16, 2020

💔 Test failed - checks-travis

@flip1995
Copy link
Member

@bors retry (spurious)

@bors
Copy link
Contributor

bors commented Jan 16, 2020

⌛ Testing commit 32337a9 with merge 8b72b72...

bors added a commit that referenced this pull request Jan 16, 2020
Autogenerate new lints

Add option in clippy_dev to automatically generate boilerplate code for adding new lints

example:

`./util/dev new_lint --name=iter_nth_zero --type=late`

Fixes #4942

changelog: none
@bors
Copy link
Contributor

bors commented Jan 16, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 8b72b72 to master...

@bors bors merged commit 32337a9 into rust-lang:master Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic new lint boilerplate generation
5 participants