[redundant_pattern_matching]: include guard in suggestion
#11175
Conversation
|
r? @Manishearth (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
|
This could probably add the guard (like |
| // wow, the HIR for match guards in `PAT if let PAT = expr && expr => ...` is annoying! | ||
| // `guard` here is `Guard::If` with the let expression somewhere deep in the tree of exprs, | ||
| // counter to the intuition that it should be `Guard::IfLet`, so we need another check | ||
| // to see that there aren't any let chains anywhere in the guard, as that would break | ||
| // if we suggest `t.is_none() && (let X = y && z)` for: | ||
| // `match t { None if let X = y && z => true, _ => false }` |
There was a problem hiding this comment.
I feel like this comment might be a bit confusing. If you ( the reviewer(s) ) already understand what this means, feel free to ignore.
Looking at the HIR here: https://godbolt.org/z/8413T1rjP (line 521-522), the guard is not Guard::IfLet, but Guard::If, with the let expression being potentially somewhere deep in the expression tree, so just checking Guard::If is not enough -- it may still be an if let guard despite the guard not being Guard::IfLet
There was a problem hiding this comment.
This is perhaps because it's desugared to something akin a let-chain, or at least something very similar, which is like And<Let, And<Expr, And<Expr, Let>>> or something in the HIR, with Expr being a regular && and Let being && let, so you can probably check the first in Binary::And afaik but perhaps this would have issues on something like if true && let Some(x) = x, I'm not sure. Better safe than sorry though so the current way is fine
Just if let Some(_) = x on its own is IfLet. I suppose it's because IfLet only allows one Let, while it needs a BinaryKind::And for a let chain to be represented
y21
changed the title
redundant_pattern_matching]: don't lint if match guards are presentredundant_pattern_matching]: include guard in suggestion
|
@bors r+ comment seems fine to me. it's confusing but the situation is confusing |
[`redundant_pattern_matching`]: include guard in suggestion Fixes #11174 [`redundant_pattern_matching`]: include guard in suggestion
|
💔 Test failed - checks-action_test |
|
oops, I must have deleted the "changelog:" word from my PR description somehow. I edited it in now. |
|
@bors retry |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes #11174
changelog: [
redundant_pattern_matching]: include guard in suggestion