-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Suggest using matches
or adding ==
on x == a || b || c
#128159
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3365,4 +3365,113 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
err.span_label(block.span, "this block is missing a tail expression"); | ||
} | ||
} | ||
|
||
pub(crate) fn annotate_incorrect_or_expr( | ||
&self, | ||
diag: &mut Diag<'_>, | ||
expr: &'tcx hir::Expr<'tcx>, | ||
expr_ty: Ty<'tcx>, | ||
expected_ty: Ty<'tcx>, | ||
) { | ||
if expected_ty != self.tcx.types.bool { | ||
return; | ||
} | ||
let hir::Node::Expr(&hir::Expr { | ||
kind: | ||
hir::ExprKind::Binary( | ||
hir::BinOp { node: hir::BinOpKind::Or, span: binop_span }, | ||
lhs, | ||
rhs, | ||
), | ||
hir_id: parent_hir_id, | ||
span: full_span, | ||
.. | ||
}) = self.tcx.parent_hir_node(expr.hir_id) | ||
else { | ||
return; | ||
}; | ||
Comment on lines
+3383
to
+3392
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
|
||
if rhs.hir_id != expr.hir_id { | ||
return; | ||
} | ||
let hir::Expr { | ||
kind: | ||
hir::ExprKind::Binary(hir::BinOp { node: hir::BinOpKind::Eq, span: eq_span }, lhs, _), | ||
.. | ||
} = *lhs | ||
else { | ||
return; | ||
}; | ||
let Some(lhs_ty) = self.typeck_results.borrow().expr_ty_opt(lhs) else { | ||
return; | ||
}; | ||
// Coercion here is not totally right, but w/e. | ||
if !self.can_coerce(expr_ty, lhs_ty) { | ||
Comment on lines
+3407
to
+3408
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
return; | ||
} | ||
|
||
// Track whether all of the exprs to the right, i.e. `|| a || b || c` are all pattern-like. | ||
let mut is_literal = rhs.is_approximately_pattern(); | ||
// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might want to move this comment about walking to before the comment for |
||
let mut expr_hir_id = parent_hir_id; | ||
let mut binop_spans = vec![binop_span]; | ||
while let hir::Node::Expr(&hir::Expr { | ||
kind: | ||
hir::ExprKind::Binary( | ||
hir::BinOp { node: hir::BinOpKind::Or, span: binop_span }, | ||
lhs, | ||
rhs, | ||
), | ||
hir_id: parent_hir_id, | ||
span, | ||
.. | ||
}) = self.tcx.parent_hir_node(expr_hir_id) | ||
&& lhs.hir_id == expr_hir_id | ||
{ | ||
binop_spans.push(binop_span); | ||
expr_hir_id = parent_hir_id; | ||
full_span = span; | ||
is_literal |= rhs.is_approximately_pattern(); | ||
} | ||
|
||
// If the type is structural peq, then suggest `matches!(x, a | b | c)`. | ||
// Otherwise, suggest adding `x == ` to every `||`. | ||
if is_literal | ||
// I know this logic may look a bit sketchy, but int vars don't implement | ||
// `StructuralPeq` b/c they're unconstrained, so just check for those manually. | ||
&& (self.tcx.lang_items().structural_peq_trait().is_some_and(|structural_peq_def_id| { | ||
self.type_implements_trait(structural_peq_def_id, [lhs_ty], self.param_env) | ||
.must_apply_modulo_regions() | ||
}) || lhs_ty.is_integral()) | ||
{ | ||
let eq_span = lhs.span.shrink_to_hi().to(eq_span); | ||
diag.multipart_suggestion_verbose( | ||
"use `matches!()` to match against multiple values", | ||
[ | ||
(full_span.shrink_to_lo(), "matches!(".to_string()), | ||
(full_span.shrink_to_hi(), ")".to_string()), | ||
(eq_span, ",".to_string()), | ||
] | ||
.into_iter() | ||
.chain(binop_spans.into_iter().map(|span| (span, "|".to_string()))) | ||
.collect(), | ||
Applicability::MachineApplicable, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this should be |
||
); | ||
} else if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = lhs.kind | ||
&& let Res::Local(local) = path.res | ||
{ | ||
let local = self.tcx.hir().name(local); | ||
let local_name = format!(" {local} =="); | ||
diag.multipart_suggestion_verbose( | ||
format!("use `{local} == ...` to match against several different values"), | ||
binop_spans | ||
.into_iter() | ||
.map(|span| (span.shrink_to_hi(), local_name.clone())) | ||
.collect(), | ||
Applicability::MachineApplicable, | ||
); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
fn main() { | ||
let x = 1; | ||
if x == 1 || 2 || 3 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a test for |
||
//~^ ERROR mismatched types | ||
//~| ERROR mismatched types | ||
println!("Was 1 or 2 or 3"); | ||
} | ||
|
||
let x = 1.0; | ||
if x == 1.0 || 2.0 || 3.0 { | ||
//~^ ERROR mismatched types | ||
//~| ERROR mismatched types | ||
println!("Was 1.0 or 2.0 or 3.0"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
error[E0308]: mismatched types | ||
--> $DIR/nested-or-of-eq.rs:3:18 | ||
| | ||
LL | if x == 1 || 2 || 3 { | ||
| ------ ^ expected `bool`, found integer | ||
| | | ||
| expected because this is `bool` | ||
| | ||
help: use `matches!()` to match against multiple values | ||
| | ||
LL | if matches!(x, 1 | 2 | 3) { | ||
| +++++++++ ~ ~ ~ + | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/nested-or-of-eq.rs:3:23 | ||
| | ||
LL | if x == 1 || 2 || 3 { | ||
| ----------- ^ expected `bool`, found integer | ||
| | | ||
| expected because this is `bool` | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/nested-or-of-eq.rs:10:20 | ||
| | ||
LL | if x == 1.0 || 2.0 || 3.0 { | ||
| -------- ^^^ expected `bool`, found floating-point number | ||
| | | ||
| expected because this is `bool` | ||
| | ||
help: use `x == ...` to match against several different values | ||
| | ||
LL | if x == 1.0 || x == 2.0 || x == 3.0 { | ||
| ++++ ++++ | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/nested-or-of-eq.rs:10:27 | ||
| | ||
LL | if x == 1.0 || 2.0 || 3.0 { | ||
| --------------- ^^^ expected `bool`, found floating-point number | ||
| | | ||
| expected because this is `bool` | ||
|
||
error: aborting due to 4 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0308`. |
Uh oh!
There was an error while loading. Please reload this page.