From f8a7a6aad5a9277d05175cb171f621a75eb046f9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 24 Jul 2024 16:42:13 -0400 Subject: [PATCH] Suggest using `matches` or adding `==` on `x == a || b || c` --- compiler/rustc_hir/src/hir.rs | 17 ++- compiler/rustc_hir_typeck/src/demand.rs | 3 +- .../src/fn_ctxt/suggestions.rs | 109 ++++++++++++++++++ tests/ui/binop/nested-or-of-eq.rs | 15 +++ tests/ui/binop/nested-or-of-eq.stderr | 45 ++++++++ 5 files changed, 182 insertions(+), 7 deletions(-) create mode 100644 tests/ui/binop/nested-or-of-eq.rs create mode 100644 tests/ui/binop/nested-or-of-eq.stderr diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index bf773f2d48793..d49531736b97f 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1882,12 +1882,17 @@ impl Expr<'_> { /// To a first-order approximation, is this a pattern? pub fn is_approximately_pattern(&self) -> bool { match &self.kind { - ExprKind::Array(_) - | ExprKind::Call(..) - | ExprKind::Tup(_) - | ExprKind::Lit(_) - | ExprKind::Path(_) - | ExprKind::Struct(..) => true, + ExprKind::Array(exprs) | ExprKind::Tup(exprs) => { + exprs.iter().all(|expr| expr.is_approximately_pattern()) + } + ExprKind::Struct(_, fields, None) => { + fields.iter().all(|field| field.expr.is_approximately_pattern()) + } + ExprKind::Call(callee, exprs) => { + callee.is_approximately_pattern() + && exprs.iter().all(|expr| expr.is_approximately_pattern()) + } + ExprKind::Lit(_) | ExprKind::Path(_) => true, _ => false, } } diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 4f1c2fdd92260..60a189df583e4 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -74,7 +74,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn emit_coerce_suggestions( &self, err: &mut Diag<'_>, - expr: &hir::Expr<'tcx>, + expr: &'tcx hir::Expr<'tcx>, expr_ty: Ty<'tcx>, expected: Ty<'tcx>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, @@ -86,6 +86,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.annotate_expected_due_to_let_ty(err, expr, error); self.annotate_loop_expected_due_to_inference(err, expr, error); + self.annotate_incorrect_or_expr(err, expr, expr_ty, expected); // FIXME(#73154): For now, we do leak check when coercing function // pointers in typeck, instead of only during borrowck. This can lead diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index fe7495deb2bf3..21b85e331c8cc 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -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; + }; + 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) { + 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`. + 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, + ); + } 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, + ); + } + } } diff --git a/tests/ui/binop/nested-or-of-eq.rs b/tests/ui/binop/nested-or-of-eq.rs new file mode 100644 index 0000000000000..3e65f7d770705 --- /dev/null +++ b/tests/ui/binop/nested-or-of-eq.rs @@ -0,0 +1,15 @@ +fn main() { + let x = 1; + if x == 1 || 2 || 3 { + //~^ 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"); + } +} diff --git a/tests/ui/binop/nested-or-of-eq.stderr b/tests/ui/binop/nested-or-of-eq.stderr new file mode 100644 index 0000000000000..30eabec8aaf04 --- /dev/null +++ b/tests/ui/binop/nested-or-of-eq.stderr @@ -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`.