Skip to content
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

Suggest if let x = y when encountering if x = y #75931

Merged
merged 1 commit into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>>,
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
}

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