rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.44k stars 1.54k forks source link

used_unused_variable lint #4239

Open yaahc opened 5 years ago

yaahc commented 5 years ago

The idea is to lint for variables that have had a _ prepended on them to avoid unused variable lints and then subsequently been used.

The motivation for this change is that I want to use cargo fix --clippy as my default command to run to check my working copy for compiler errors, but it puts _ on the front of variables when I'm initially writing them and then doesn't clean up after itself once I start using said variables. This bothers me and I want to fix it to make my workflow smoother.

We would want to restrict this lint by default, a its entirely reasonable to start variables with an underscore normally, so we don't necessarily want to force people to treat a leading _ as a reserved that they can't use except to indicate a variable is unused.

For this to be useful for my workflow the lint should be marked as MachineApplicable.

yaahc commented 5 years ago

The plan here is likely to implement this in rustc rather than as part of clippy, and I believe that is because the infrastructure to do this check already exists in rustc but would have to be reimplemented in clippy.

The one main blocker that I know of is the fact that rustc doesn't have an equivalent for "restricted lints" which are things that are not universally considered bad but that some people may wish to selectively enable. The idea is that we will add this class of lints to rustc and implement used_unused_variable as a restricted rustc lint.

@Manishearth feel free to elaborate on or correct any of the above.

yaahc commented 5 years ago

I'm going to leave this issue open for now until we've opened an issue against rustc and the compiler folks have agreed to the plan as outlined above.

flip1995 commented 5 years ago

rustc doesn't have an equivalent for "restricted lints"

In rustc these would be the allow-by-default lints.

But I think the main problem is, that rustc has the policy to not include new lints, unless they're really necessary (in combination with a new lang feature / edition / ...). I don't think this lint would make it in rustc. For lints like this Clippy exists.

You can "map" lint levels from Clippy to rustc (if a lint would be uplifted) like this:

Clippy rustc
deny-by-default warn-by-default
warn-by-default allow-by-default
allow-by-default not a lint in rustc

Maybe we could expose the rustc interface to Clippy and use this to implement the lint?

oli-obk commented 4 years ago

I thought we had such a lint, but it had bugs.