Suggest using matches or adding == on x == a || b || c
#128159
Conversation
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
rustbot
added
S-waiting-on-review
compiler-errors
force-pushed
the
nested-ors
branch
from
cf3926d
to
5832db3
Compare
compiler-errors
force-pushed
the
nested-ors
branch
from
5832db3
to
f8a7a6a
Compare
| @@ -0,0 +1,15 @@ | |||
| fn main() { | |||
| let x = 1; | |||
| if x == 1 || 2 || 3 { | |||
There was a problem hiding this comment.
Add a test for x == 1 || 2 || a where let a = 3;
| lhs, | ||
| rhs, | ||
| ), | ||
| hir_id: parent_hir_id, | ||
| span: full_span, | ||
| .. | ||
| }) = self.tcx.parent_hir_node(expr.hir_id) | ||
| else { | ||
| return; | ||
| }; |
There was a problem hiding this comment.
I think it would be best if we documented this code. It took some time for me to fully realize what we are doing here. Consider documenting that:
- We start to see if a suggestion is applicable when
binx == a || bisn't abool. (type mismatch) - We don't try to suggest when
bcannot be coerced tox.
| // Track the span of the outermost `||` expr. | ||
| let mut full_span = full_span; | ||
|
|
||
| // Walk up the expr tree gathering up the binop spans of any subsequent `|| a || b || c`. |
There was a problem hiding this comment.
You might want to move this comment about walking to before the comment for is_literal so the reader knows what we would be doing better
| // Coercion here is not totally right, but w/e. | ||
| if !self.can_coerce(expr_ty, lhs_ty) { |
There was a problem hiding this comment.
This only checks one of the types of the exprs can be coerced. I can think of a weird way where the user wanted to write x == a | b | c in place of x == a || b || c where c has a different type to a or b or x but its BitOr::Output is the same type so the equality would work. I think you might want to bullet-proof this by checking that everything in the binops have the same types.
| .into_iter() | ||
| .chain(binop_spans.into_iter().map(|span| (span, "|".to_string()))) | ||
| .collect(), | ||
| Applicability::MachineApplicable, |
There was a problem hiding this comment.
I don't think this should be MachineApplicable. We've only approximated that the exprs might be valid patterns but we aren't sure.
rustbot
added
S-waiting-on-author
lolbinarycat
added
the
A-diagnostics
Fixes #128132.
This has two approaches. Given some
expr == a || b || ca,b,c, ... are approximately "pattern-like" i.e. only contain expr kinds that should show up in patterns, and the type isStructuralPeq, then we suggest changing the expression to amatches!().expr) is a local, then we suggest changing the expression toexpr == a || expr == b || expr == c.