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

Cleanup: Use declare_lint_pass! and impl_lint_pass! #3917

Closed
Centril opened this issue Apr 3, 2019 · 11 comments
Closed

Cleanup: Use declare_lint_pass! and impl_lint_pass! #3917

Centril opened this issue Apr 3, 2019 · 11 comments
Labels
good-first-issue These issues are a good way to get started with Clippy

Comments

@Centril
Copy link
Contributor

Centril commented Apr 3, 2019

Introduced in rust-lang/rust#59669, macros declare_lint_pass! and impl_lint_pass! are added to librustc/lint/mod.rs. These should help to remove some boilerplate from clippy as well.

cc @oli-obk

@phansch phansch added the good-first-issue These issues are a good way to get started with Clippy label Apr 4, 2019
@kraai
Copy link
Contributor

kraai commented Apr 4, 2019

I'm going to try to do this. I don't think these macros are available on nightly yet, so I'm building a new toolchain that I hope will include them.

@Centril
Copy link
Contributor Author

Centril commented Apr 4, 2019

The PR was merged and is in rust-lang/rust upstream :)

@oli-obk
Copy link
Contributor

oli-obk commented Apr 4, 2019

@kraai there's no need to build a toolchain. You can download even master toolchains via rustup by using our https://github.com/rust-lang/rust-clippy/blob/master/setup-toolchain.sh script

@kraai
Copy link
Contributor

kraai commented Apr 4, 2019

@oli-obk Thanks! I'm trying it now.

@kraai
Copy link
Contributor

kraai commented Apr 4, 2019

I tried using declare_lint_pass! for approx_const but it triggered lint_without_lint_pass. 8386ad2 contains the changes I tried. Is the change wrong or does lint_without_lint_pass need to be updated?

@flip1995
Copy link
Member

flip1995 commented Apr 4, 2019

It could be the case, that the lint_withou_lint_pass lint is unable to detect LintPasses declared by a macro. I will look into this tomorrow evening and get back to you!

@kraai
Copy link
Contributor

kraai commented Apr 4, 2019

@flip1995 Thanks!

@flip1995
Copy link
Member

flip1995 commented Apr 5, 2019

Yeah the lint was broken. You can find the fix here: flip1995/rust-clippy@87ab2e6. Just cherry-pick the commit onto your branch or copy the fix by hand :)

@kraai
Copy link
Contributor

kraai commented Apr 8, 2019

@flip1995 Thanks!

@kraai
Copy link
Contributor

kraai commented Apr 8, 2019

I've started working on this at https://github.com/kraai/rust-clippy/tree/declare_lint_pass.

@flip1995
Copy link
Member

flip1995 commented Apr 8, 2019

The current state of your branch looks good. Multiple PRs got merged yesterday/today and you should rebase on the master branch before making bigger changes.

kraai added a commit to kraai/rust-clippy that referenced this issue Apr 15, 2019
kraai added a commit to kraai/rust-clippy that referenced this issue Apr 16, 2019
@bors bors closed this as completed in 753c396 Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

No branches or pull requests

5 participants