Add small underscore scope lint #3953
Conversation
flip1995
added
the
S-waiting-on-review
| impl EarlyLintPass for Pass { | ||
| fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &Stmt) { | ||
| if_chain! { | ||
| if let StmtKind::Local(ref local) = stmt.node; |
There was a problem hiding this comment.
I think this should apply to all patterns (e.g. in matches, too). You should be able to implement the check_pat method instead of the check_stmt method
| let sugg = if let Some(ref expr) = local.init { | ||
| format!("let _ = {};", snippet(cx, expr.span, "..")) | ||
| } else { | ||
| "let _;".to_string() |
There was a problem hiding this comment.
I don't think this situation can happen. You can just use local.init.expect("let (_, _) should have failed in rustc")
There was a problem hiding this comment.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=99a6a44ab8bb5bd47fc80f8232c50c8d
This seems to be valid Rust syntax, though I never realised terms like let x; were valid Rust at all.
There was a problem hiding this comment.
Though admittedly its entirely pointless to declare a variable _, as there is no way to later initialise it. Perhaps there should be a seperate lint against let _;?
There was a problem hiding this comment.
Matching on all patterns with check_pat means this doesn't matter anyway
There was a problem hiding this comment.
It only works if you specify the types. So let _; is not writable. You're right though, we should lint the situations that can actually occur as you showed in the playground
| fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &Stmt) { | ||
| if_chain! { | ||
| if let StmtKind::Local(ref local) = stmt.node; | ||
| if let PatKind::TupleStruct(_, ref pats, _) | PatKind::Tuple(ref pats, _) = local.pat.node; |
There was a problem hiding this comment.
The same reasoning would apply to let Foo { .. }, let Foo { a: _, b: _ }, [_], [_, _] and similar.
There was a problem hiding this comment.
Makes sense for the struct ones, but I'm unsure about linting the slice patterns. The pattern [_, _] has a different meaning to _, as it will only match on a slice with exactly 2 elements. Admittedly this is a really ugly way of checking the length of a slice, but we wouldn't be able to do a MachineApplicable suggestion, since it might change semantics.
| span_lint_and_sugg( | ||
| cx, | ||
| SMALL_UNDERSCORE_SCOPE, | ||
| local.span, |
There was a problem hiding this comment.
Only lint local.pat.span (or pat.span if you move to check_pat), that way the suggestion simply becomes _ unconditionally
|
☔ The latest upstream changes (presumably #3968) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I'll switch to using the new macros now they are merged |
JamesHinshelwood
force-pushed
the
small-underscore-scope
branch
from
31b848e to
0ae6efe
Compare
| let (_x, (_, _)) = t2; | ||
|
|
||
| let f = Foo { x: 3, y: 7 }; | ||
| let Foo { .. } = f; |
There was a problem hiding this comment.
Could you add a test for println!("foo");? It currently triggers on macro calls like here.
You'll probably need to add an in_external_macro check and return early in your lint code.
| /// | ||
| /// **Example:** | ||
| /// | ||
| /// ```rust,ignore |
There was a problem hiding this comment.
I don't think this has to be ignore, it should work as a doctest already.
phansch
added
S-waiting-on-author
|
It will be a few days till I'm able to work on this again. Apologies for the delay |
|
Unfortunately, I think I won't have time to work on this for at least a month, due to other work. If needed, you can close the PR and I will reopen it again when I have time to work on it. Sorry. |
|
No problem, thanks for the information! I will close this to keep the PR list clean. Feel free to reopen this PR when you're ready to continue working on this. |
flip1995
added
S-inactive-closed
This lint checks for bindings that look like:
let (_, _) = foo;orlet Bar(_) = foo;and suggests making the scope of the_bigger.