From 07112ca62d378c90d107448b9040faddeea9a445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 25 Aug 2020 20:28:25 -0700 Subject: [PATCH] Suggest `if let x = y` when encountering `if x = y` Detect potential cases where `if let` was meant but `let` was left out. Fix #44990. --- compiler/rustc_resolve/src/late.rs | 7 +- .../rustc_resolve/src/late/diagnostics.rs | 13 ++ compiler/rustc_session/src/session.rs | 4 + compiler/rustc_typeck/src/check/expr.rs | 55 +++++++-- src/test/ui/error-codes/E0070.stderr | 12 +- src/test/ui/issues/issue-13407.stderr | 12 +- .../disallowed-positions.stderr | 20 +-- src/test/ui/suggestions/if-let-typo.rs | 9 ++ src/test/ui/suggestions/if-let-typo.stderr | 60 +++++++++ .../assignment-expected-bool.stderr | 114 +++++++++++------- .../type/type-check/assignment-in-if.stderr | 64 ++++++---- 11 files changed, 268 insertions(+), 102 deletions(-) create mode 100644 src/test/ui/suggestions/if-let-typo.rs create mode 100644 src/test/ui/suggestions/if-let-typo.stderr diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index d113eb22abadc..2b2123e295d09 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -378,6 +378,9 @@ struct DiagnosticMetadata<'ast> { /// Only used for better errors on `let : ;`. current_let_binding: Option<(Span, Option, Option)>, + + /// Used to detect possible `if let` written without `let` and to provide structured suggestion. + in_if_condition: Option<&'ast Expr>, } struct LateResolutionVisitor<'a, 'b, 'ast> { @@ -403,7 +406,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast> { /// /// In particular, rustdoc uses this to avoid giving errors for `cfg()` items. /// In most cases this will be `None`, in which case errors will always be reported. - /// If it is `Some(_)`, then it will be updated when entering a nested function or trait body. + /// If it is `true`, then it will be updated when entering a nested function or trait body. in_func_body: bool, } @@ -2199,7 +2202,9 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { ExprKind::If(ref cond, ref then, ref opt_else) => { self.with_rib(ValueNS, NormalRibKind, |this| { + let old = this.diagnostic_metadata.in_if_condition.replace(cond); this.visit_expr(cond); + this.diagnostic_metadata.in_if_condition = old; this.visit_block(then); }); if let Some(expr) = opt_else { diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index d392967af3856..8cb6b6553ffe0 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -176,6 +176,19 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { let code = source.error_code(res.is_some()); let mut err = self.r.session.struct_span_err_with_code(base_span, &base_msg, code); + match (source, self.diagnostic_metadata.in_if_condition) { + (PathSource::Expr(_), Some(Expr { span, kind: ExprKind::Assign(..), .. })) => { + err.span_suggestion_verbose( + span.shrink_to_lo(), + "you might have meant to use pattern matching", + "let ".to_string(), + Applicability::MaybeIncorrect, + ); + self.r.session.if_let_suggestions.borrow_mut().insert(*span); + } + _ => {} + } + let is_assoc_fn = self.self_type_is_available(span); // Emit help message for fake-self from other languages (e.g., `this` in Javascript). if ["this", "my"].contains(&&*item_str.as_str()) && is_assoc_fn { diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index c006e593e4755..53c3e4df9761e 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -213,6 +213,9 @@ pub struct Session { known_attrs: Lock, used_attrs: Lock, + + /// `Span`s for `if` conditions that we have suggested turning into `if let`. + pub if_let_suggestions: Lock>, } pub struct PerfStats { @@ -1354,6 +1357,7 @@ pub fn build_session( target_features: FxHashSet::default(), known_attrs: Lock::new(MarkedAttrs::new()), used_attrs: Lock::new(MarkedAttrs::new()), + if_let_suggestions: Default::default(), }; validate_commandline_args_with_session_available(&sess); diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 1573fb967910b..cc8a6953f1397 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -764,9 +764,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { rhs: &'tcx hir::Expr<'tcx>, span: &Span, ) -> Ty<'tcx> { - let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace); - let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs)); - let expected_ty = expected.coercion_target_type(self, expr.span); if expected_ty == self.tcx.types.bool { // The expected type is `bool` but this will result in `()` so we can reasonably @@ -774,20 +771,52 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // The likely cause of this is `if foo = bar { .. }`. let actual_ty = self.tcx.mk_unit(); let mut err = self.demand_suptype_diag(expr.span, expected_ty, actual_ty).unwrap(); - let msg = "try comparing for equality"; - let left = self.tcx.sess.source_map().span_to_snippet(lhs.span); - let right = self.tcx.sess.source_map().span_to_snippet(rhs.span); - if let (Ok(left), Ok(right)) = (left, right) { - let help = format!("{} == {}", left, right); - err.span_suggestion(expr.span, msg, help, Applicability::MaybeIncorrect); + let lhs_ty = self.check_expr(&lhs); + let rhs_ty = self.check_expr(&rhs); + if self.can_coerce(lhs_ty, rhs_ty) { + if !lhs.is_syntactic_place_expr() { + // Do not suggest `if let x = y` as `==` is way more likely to be the intention. + if let hir::Node::Expr(hir::Expr { + kind: ExprKind::Match(_, _, hir::MatchSource::IfDesugar { .. }), + .. + }) = self.tcx.hir().get( + self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(expr.hir_id)), + ) { + // Likely `if let` intended. + err.span_suggestion_verbose( + expr.span.shrink_to_lo(), + "you might have meant to use pattern matching", + "let ".to_string(), + Applicability::MaybeIncorrect, + ); + } + } + err.span_suggestion_verbose( + *span, + "you might have meant to compare for equality", + "==".to_string(), + Applicability::MaybeIncorrect, + ); } else { - err.help(msg); + // Do this to cause extra errors about the assignment. + let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace); + let _ = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs)); } - err.emit(); - } else { - self.check_lhs_assignable(lhs, "E0070", span); + + if self.sess().if_let_suggestions.borrow().get(&expr.span).is_some() { + // We already emitted an `if let` suggestion due to an identifier not found. + err.delay_as_bug(); + } else { + err.emit(); + } + return self.tcx.ty_error(); } + self.check_lhs_assignable(lhs, "E0070", span); + + let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace); + let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs)); + self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized); if lhs_ty.references_error() || rhs_ty.references_error() { diff --git a/src/test/ui/error-codes/E0070.stderr b/src/test/ui/error-codes/E0070.stderr index d809bb18dee16..e24d498e3520c 100644 --- a/src/test/ui/error-codes/E0070.stderr +++ b/src/test/ui/error-codes/E0070.stderr @@ -14,12 +14,6 @@ LL | 1 = 3; | | | cannot assign to this expression -error[E0308]: mismatched types - --> $DIR/E0070.rs:8:25 - | -LL | some_other_func() = 4; - | ^ expected `()`, found integer - error[E0070]: invalid left-hand side of assignment --> $DIR/E0070.rs:8:23 | @@ -28,6 +22,12 @@ LL | some_other_func() = 4; | | | cannot assign to this expression +error[E0308]: mismatched types + --> $DIR/E0070.rs:8:25 + | +LL | some_other_func() = 4; + | ^ expected `()`, found integer + error: aborting due to 4 previous errors Some errors have detailed explanations: E0070, E0308. diff --git a/src/test/ui/issues/issue-13407.stderr b/src/test/ui/issues/issue-13407.stderr index f30b6cdeaf073..5352066bf771e 100644 --- a/src/test/ui/issues/issue-13407.stderr +++ b/src/test/ui/issues/issue-13407.stderr @@ -10,12 +10,6 @@ note: the unit struct `C` is defined here LL | struct C; | ^^^^^^^^^ -error[E0308]: mismatched types - --> $DIR/issue-13407.rs:6:12 - | -LL | A::C = 1; - | ^ expected struct `A::C`, found integer - error[E0070]: invalid left-hand side of assignment --> $DIR/issue-13407.rs:6:10 | @@ -24,6 +18,12 @@ LL | A::C = 1; | | | cannot assign to this expression +error[E0308]: mismatched types + --> $DIR/issue-13407.rs:6:12 + | +LL | A::C = 1; + | ^ expected struct `A::C`, found integer + error: aborting due to 3 previous errors Some errors have detailed explanations: E0070, E0308, E0603. diff --git a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr index 7f343d1a853ac..3372495d0feeb 100644 --- a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr +++ b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr @@ -568,10 +568,12 @@ error[E0308]: mismatched types --> $DIR/disallowed-positions.rs:56:8 | LL | if x = let 0 = 0 {} - | ^^^^^^^^^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `x == let 0 = 0` + | ^^^^^^^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | if x == let 0 = 0 {} + | ^^ error[E0308]: mismatched types --> $DIR/disallowed-positions.rs:59:8 @@ -754,10 +756,12 @@ error[E0308]: mismatched types --> $DIR/disallowed-positions.rs:120:11 | LL | while x = let 0 = 0 {} - | ^^^^^^^^^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `x == let 0 = 0` + | ^^^^^^^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | while x == let 0 = 0 {} + | ^^ error[E0308]: mismatched types --> $DIR/disallowed-positions.rs:123:11 diff --git a/src/test/ui/suggestions/if-let-typo.rs b/src/test/ui/suggestions/if-let-typo.rs new file mode 100644 index 0000000000000..c1e417b97f619 --- /dev/null +++ b/src/test/ui/suggestions/if-let-typo.rs @@ -0,0 +1,9 @@ +fn main() { + let foo = Some(0); + let bar = None; + if Some(x) = foo {} //~ ERROR cannot find value `x` in this scope + if Some(foo) = bar {} //~ ERROR mismatched types + if 3 = foo {} //~ ERROR mismatched types + //~^ ERROR mismatched types + if Some(3) = foo {} //~ ERROR mismatched types +} diff --git a/src/test/ui/suggestions/if-let-typo.stderr b/src/test/ui/suggestions/if-let-typo.stderr new file mode 100644 index 0000000000000..09db8e0146970 --- /dev/null +++ b/src/test/ui/suggestions/if-let-typo.stderr @@ -0,0 +1,60 @@ +error[E0425]: cannot find value `x` in this scope + --> $DIR/if-let-typo.rs:4:13 + | +LL | if Some(x) = foo {} + | ^ not found in this scope + | +help: you might have meant to use pattern matching + | +LL | if let Some(x) = foo {} + | ^^^ + +error[E0308]: mismatched types + --> $DIR/if-let-typo.rs:5:8 + | +LL | if Some(foo) = bar {} + | ^^^^^^^^^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to use pattern matching + | +LL | if let Some(foo) = bar {} + | ^^^ +help: you might have meant to compare for equality + | +LL | if Some(foo) == bar {} + | ^^ + +error[E0308]: mismatched types + --> $DIR/if-let-typo.rs:6:12 + | +LL | if 3 = foo {} + | ^^^ expected integer, found enum `std::option::Option` + | + = note: expected type `{integer}` + found enum `std::option::Option<{integer}>` + +error[E0308]: mismatched types + --> $DIR/if-let-typo.rs:6:8 + | +LL | if 3 = foo {} + | ^^^^^^^ expected `bool`, found `()` + +error[E0308]: mismatched types + --> $DIR/if-let-typo.rs:8:8 + | +LL | if Some(3) = foo {} + | ^^^^^^^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to use pattern matching + | +LL | if let Some(3) = foo {} + | ^^^ +help: you might have meant to compare for equality + | +LL | if Some(3) == foo {} + | ^^ + +error: aborting due to 5 previous errors + +Some errors have detailed explanations: E0308, E0425. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/type/type-check/assignment-expected-bool.stderr b/src/test/ui/type/type-check/assignment-expected-bool.stderr index 3f1caddf728cb..d1c13a33f7f4b 100644 --- a/src/test/ui/type/type-check/assignment-expected-bool.stderr +++ b/src/test/ui/type/type-check/assignment-expected-bool.stderr @@ -2,100 +2,126 @@ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:6:19 | LL | let _: bool = 0 = 0; - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | let _: bool = 0 == 0; + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:9:14 | LL | 0 => 0 = 0, - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | 0 => 0 == 0, + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:10:14 | LL | _ => 0 = 0, - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | _ => 0 == 0, + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:14:17 | LL | true => 0 = 0, - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | true => 0 == 0, + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:18:8 | LL | if 0 = 0 {} - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to use pattern matching + | +LL | if let 0 = 0 {} + | ^^^ +help: you might have meant to compare for equality + | +LL | if 0 == 0 {} + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:20:24 | LL | let _: bool = if { 0 = 0 } { - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | let _: bool = if { 0 == 0 } { + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:21:9 | LL | 0 = 0 - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | 0 == 0 + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:23:9 | LL | 0 = 0 - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | 0 == 0 + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:26:13 | LL | let _ = (0 = 0) - | ^^^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | let _ = (0 == 0) + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:27:14 | LL | && { 0 = 0 } - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | && { 0 == 0 } + | ^^ error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:28:12 | LL | || (0 = 0); - | ^^^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `0 == 0` + | ^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | || (0 == 0); + | ^^ error[E0070]: invalid left-hand side of assignment --> $DIR/assignment-expected-bool.rs:31:22 diff --git a/src/test/ui/type/type-check/assignment-in-if.stderr b/src/test/ui/type/type-check/assignment-in-if.stderr index 0957dcb986b51..f5306a1226416 100644 --- a/src/test/ui/type/type-check/assignment-in-if.stderr +++ b/src/test/ui/type/type-check/assignment-in-if.stderr @@ -2,55 +2,71 @@ error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:15:8 | LL | if x = x { - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `x == x` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | if x == x { + | ^^ error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:20:8 | LL | if (x = x) { - | ^^^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `x == x` + | ^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | if (x == x) { + | ^^ error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:25:8 | LL | if y = (Foo { foo: x }) { - | ^^^^^^^^^^^^^^^^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `y == (Foo { foo: x })` + | ^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | if y == (Foo { foo: x }) { + | ^^ error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:30:8 | LL | if 3 = x { - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `3 == x` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to use pattern matching + | +LL | if let 3 = x { + | ^^^ +help: you might have meant to compare for equality + | +LL | if 3 == x { + | ^^ error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:36:13 | LL | x = 4 - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `x == 4` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | x == 4 + | ^^ error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:38:13 | LL | x = 5 - | ^^^^^ - | | - | expected `bool`, found `()` - | help: try comparing for equality: `x == 5` + | ^^^^^ expected `bool`, found `()` + | +help: you might have meant to compare for equality + | +LL | x == 5 + | ^^ error: aborting due to 6 previous errors