Skip to content

Commit

Permalink
Suggest if let x = y when encountering if x = y
Browse files Browse the repository at this point in the history
Detect potential cases where `if let` was meant but `let` was left out.

Fix #44990.
  • Loading branch information
estebank committed Aug 30, 2020
1 parent 85fbf49 commit 07112ca
Show file tree
Hide file tree
Showing 11 changed files with 268 additions and 102 deletions.
7 changes: 6 additions & 1 deletion compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,9 @@ struct DiagnosticMetadata<'ast> {

/// Only used for better errors on `let <pat>: <expr, not type>;`.
current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,

/// 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> {
Expand All @@ -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,
}

Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ pub struct Session {

known_attrs: Lock<MarkedAttrs>,
used_attrs: Lock<MarkedAttrs>,

/// `Span`s for `if` conditions that we have suggested turning into `if let`.
pub if_let_suggestions: Lock<FxHashSet<Span>>,
}

pub struct PerfStats {
Expand Down Expand Up @@ -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);
Expand Down
55 changes: 42 additions & 13 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,30 +764,59 @@ 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
// say that the user intended to write `lhs == rhs` instead of `lhs = rhs`.
// 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() {
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/error-codes/E0070.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/issues/issue-13407.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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.
Expand Down
20 changes: 12 additions & 8 deletions src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/suggestions/if-let-typo.rs
Original file line number Diff line number Diff line change
@@ -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
}
60 changes: 60 additions & 0 deletions src/test/ui/suggestions/if-let-typo.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading

0 comments on commit 07112ca

Please sign in to comment.