Skip to content

Commit d824b23

Browse files
committed
Auto merge of #75931 - estebank:suggest-if-let, r=petrochenkov
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.
2 parents d927e5a + 07112ca commit d824b23

File tree

11 files changed

+268
-102
lines changed

11 files changed

+268
-102
lines changed

compiler/rustc_resolve/src/late.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ struct DiagnosticMetadata<'ast> {
378378

379379
/// Only used for better errors on `let <pat>: <expr, not type>;`.
380380
current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,
381+
382+
/// Used to detect possible `if let` written without `let` and to provide structured suggestion.
383+
in_if_condition: Option<&'ast Expr>,
381384
}
382385

383386
struct LateResolutionVisitor<'a, 'b, 'ast> {
@@ -403,7 +406,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast> {
403406
///
404407
/// In particular, rustdoc uses this to avoid giving errors for `cfg()` items.
405408
/// In most cases this will be `None`, in which case errors will always be reported.
406-
/// If it is `Some(_)`, then it will be updated when entering a nested function or trait body.
409+
/// If it is `true`, then it will be updated when entering a nested function or trait body.
407410
in_func_body: bool,
408411
}
409412

@@ -2199,7 +2202,9 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
21992202

22002203
ExprKind::If(ref cond, ref then, ref opt_else) => {
22012204
self.with_rib(ValueNS, NormalRibKind, |this| {
2205+
let old = this.diagnostic_metadata.in_if_condition.replace(cond);
22022206
this.visit_expr(cond);
2207+
this.diagnostic_metadata.in_if_condition = old;
22032208
this.visit_block(then);
22042209
});
22052210
if let Some(expr) = opt_else {

compiler/rustc_resolve/src/late/diagnostics.rs

+13
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,19 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
176176
let code = source.error_code(res.is_some());
177177
let mut err = self.r.session.struct_span_err_with_code(base_span, &base_msg, code);
178178

179+
match (source, self.diagnostic_metadata.in_if_condition) {
180+
(PathSource::Expr(_), Some(Expr { span, kind: ExprKind::Assign(..), .. })) => {
181+
err.span_suggestion_verbose(
182+
span.shrink_to_lo(),
183+
"you might have meant to use pattern matching",
184+
"let ".to_string(),
185+
Applicability::MaybeIncorrect,
186+
);
187+
self.r.session.if_let_suggestions.borrow_mut().insert(*span);
188+
}
189+
_ => {}
190+
}
191+
179192
let is_assoc_fn = self.self_type_is_available(span);
180193
// Emit help message for fake-self from other languages (e.g., `this` in Javascript).
181194
if ["this", "my"].contains(&&*item_str.as_str()) && is_assoc_fn {

compiler/rustc_session/src/session.rs

+4
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,9 @@ pub struct Session {
213213

214214
known_attrs: Lock<MarkedAttrs>,
215215
used_attrs: Lock<MarkedAttrs>,
216+
217+
/// `Span`s for `if` conditions that we have suggested turning into `if let`.
218+
pub if_let_suggestions: Lock<FxHashSet<Span>>,
216219
}
217220

218221
pub struct PerfStats {
@@ -1354,6 +1357,7 @@ pub fn build_session(
13541357
target_features: FxHashSet::default(),
13551358
known_attrs: Lock::new(MarkedAttrs::new()),
13561359
used_attrs: Lock::new(MarkedAttrs::new()),
1360+
if_let_suggestions: Default::default(),
13571361
};
13581362

13591363
validate_commandline_args_with_session_available(&sess);

compiler/rustc_typeck/src/check/expr.rs

+42-13
Original file line numberDiff line numberDiff line change
@@ -764,30 +764,59 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
764764
rhs: &'tcx hir::Expr<'tcx>,
765765
span: &Span,
766766
) -> Ty<'tcx> {
767-
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
768-
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
769-
770767
let expected_ty = expected.coercion_target_type(self, expr.span);
771768
if expected_ty == self.tcx.types.bool {
772769
// The expected type is `bool` but this will result in `()` so we can reasonably
773770
// say that the user intended to write `lhs == rhs` instead of `lhs = rhs`.
774771
// The likely cause of this is `if foo = bar { .. }`.
775772
let actual_ty = self.tcx.mk_unit();
776773
let mut err = self.demand_suptype_diag(expr.span, expected_ty, actual_ty).unwrap();
777-
let msg = "try comparing for equality";
778-
let left = self.tcx.sess.source_map().span_to_snippet(lhs.span);
779-
let right = self.tcx.sess.source_map().span_to_snippet(rhs.span);
780-
if let (Ok(left), Ok(right)) = (left, right) {
781-
let help = format!("{} == {}", left, right);
782-
err.span_suggestion(expr.span, msg, help, Applicability::MaybeIncorrect);
774+
let lhs_ty = self.check_expr(&lhs);
775+
let rhs_ty = self.check_expr(&rhs);
776+
if self.can_coerce(lhs_ty, rhs_ty) {
777+
if !lhs.is_syntactic_place_expr() {
778+
// Do not suggest `if let x = y` as `==` is way more likely to be the intention.
779+
if let hir::Node::Expr(hir::Expr {
780+
kind: ExprKind::Match(_, _, hir::MatchSource::IfDesugar { .. }),
781+
..
782+
}) = self.tcx.hir().get(
783+
self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(expr.hir_id)),
784+
) {
785+
// Likely `if let` intended.
786+
err.span_suggestion_verbose(
787+
expr.span.shrink_to_lo(),
788+
"you might have meant to use pattern matching",
789+
"let ".to_string(),
790+
Applicability::MaybeIncorrect,
791+
);
792+
}
793+
}
794+
err.span_suggestion_verbose(
795+
*span,
796+
"you might have meant to compare for equality",
797+
"==".to_string(),
798+
Applicability::MaybeIncorrect,
799+
);
783800
} else {
784-
err.help(msg);
801+
// Do this to cause extra errors about the assignment.
802+
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
803+
let _ = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
785804
}
786-
err.emit();
787-
} else {
788-
self.check_lhs_assignable(lhs, "E0070", span);
805+
806+
if self.sess().if_let_suggestions.borrow().get(&expr.span).is_some() {
807+
// We already emitted an `if let` suggestion due to an identifier not found.
808+
err.delay_as_bug();
809+
} else {
810+
err.emit();
811+
}
812+
return self.tcx.ty_error();
789813
}
790814

815+
self.check_lhs_assignable(lhs, "E0070", span);
816+
817+
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
818+
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
819+
791820
self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized);
792821

793822
if lhs_ty.references_error() || rhs_ty.references_error() {

src/test/ui/error-codes/E0070.stderr

+6-6
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@ LL | 1 = 3;
1414
| |
1515
| cannot assign to this expression
1616

17-
error[E0308]: mismatched types
18-
--> $DIR/E0070.rs:8:25
19-
|
20-
LL | some_other_func() = 4;
21-
| ^ expected `()`, found integer
22-
2317
error[E0070]: invalid left-hand side of assignment
2418
--> $DIR/E0070.rs:8:23
2519
|
@@ -28,6 +22,12 @@ LL | some_other_func() = 4;
2822
| |
2923
| cannot assign to this expression
3024

25+
error[E0308]: mismatched types
26+
--> $DIR/E0070.rs:8:25
27+
|
28+
LL | some_other_func() = 4;
29+
| ^ expected `()`, found integer
30+
3131
error: aborting due to 4 previous errors
3232

3333
Some errors have detailed explanations: E0070, E0308.

src/test/ui/issues/issue-13407.stderr

+6-6
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,6 @@ note: the unit struct `C` is defined here
1010
LL | struct C;
1111
| ^^^^^^^^^
1212

13-
error[E0308]: mismatched types
14-
--> $DIR/issue-13407.rs:6:12
15-
|
16-
LL | A::C = 1;
17-
| ^ expected struct `A::C`, found integer
18-
1913
error[E0070]: invalid left-hand side of assignment
2014
--> $DIR/issue-13407.rs:6:10
2115
|
@@ -24,6 +18,12 @@ LL | A::C = 1;
2418
| |
2519
| cannot assign to this expression
2620

21+
error[E0308]: mismatched types
22+
--> $DIR/issue-13407.rs:6:12
23+
|
24+
LL | A::C = 1;
25+
| ^ expected struct `A::C`, found integer
26+
2727
error: aborting due to 3 previous errors
2828

2929
Some errors have detailed explanations: E0070, E0308, E0603.

src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr

+12-8
Original file line numberDiff line numberDiff line change
@@ -568,10 +568,12 @@ error[E0308]: mismatched types
568568
--> $DIR/disallowed-positions.rs:56:8
569569
|
570570
LL | if x = let 0 = 0 {}
571-
| ^^^^^^^^^^^^^
572-
| |
573-
| expected `bool`, found `()`
574-
| help: try comparing for equality: `x == let 0 = 0`
571+
| ^^^^^^^^^^^^^ expected `bool`, found `()`
572+
|
573+
help: you might have meant to compare for equality
574+
|
575+
LL | if x == let 0 = 0 {}
576+
| ^^
575577

576578
error[E0308]: mismatched types
577579
--> $DIR/disallowed-positions.rs:59:8
@@ -754,10 +756,12 @@ error[E0308]: mismatched types
754756
--> $DIR/disallowed-positions.rs:120:11
755757
|
756758
LL | while x = let 0 = 0 {}
757-
| ^^^^^^^^^^^^^
758-
| |
759-
| expected `bool`, found `()`
760-
| help: try comparing for equality: `x == let 0 = 0`
759+
| ^^^^^^^^^^^^^ expected `bool`, found `()`
760+
|
761+
help: you might have meant to compare for equality
762+
|
763+
LL | while x == let 0 = 0 {}
764+
| ^^
761765

762766
error[E0308]: mismatched types
763767
--> $DIR/disallowed-positions.rs:123:11
+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
fn main() {
2+
let foo = Some(0);
3+
let bar = None;
4+
if Some(x) = foo {} //~ ERROR cannot find value `x` in this scope
5+
if Some(foo) = bar {} //~ ERROR mismatched types
6+
if 3 = foo {} //~ ERROR mismatched types
7+
//~^ ERROR mismatched types
8+
if Some(3) = foo {} //~ ERROR mismatched types
9+
}
+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
error[E0425]: cannot find value `x` in this scope
2+
--> $DIR/if-let-typo.rs:4:13
3+
|
4+
LL | if Some(x) = foo {}
5+
| ^ not found in this scope
6+
|
7+
help: you might have meant to use pattern matching
8+
|
9+
LL | if let Some(x) = foo {}
10+
| ^^^
11+
12+
error[E0308]: mismatched types
13+
--> $DIR/if-let-typo.rs:5:8
14+
|
15+
LL | if Some(foo) = bar {}
16+
| ^^^^^^^^^^^^^^^ expected `bool`, found `()`
17+
|
18+
help: you might have meant to use pattern matching
19+
|
20+
LL | if let Some(foo) = bar {}
21+
| ^^^
22+
help: you might have meant to compare for equality
23+
|
24+
LL | if Some(foo) == bar {}
25+
| ^^
26+
27+
error[E0308]: mismatched types
28+
--> $DIR/if-let-typo.rs:6:12
29+
|
30+
LL | if 3 = foo {}
31+
| ^^^ expected integer, found enum `std::option::Option`
32+
|
33+
= note: expected type `{integer}`
34+
found enum `std::option::Option<{integer}>`
35+
36+
error[E0308]: mismatched types
37+
--> $DIR/if-let-typo.rs:6:8
38+
|
39+
LL | if 3 = foo {}
40+
| ^^^^^^^ expected `bool`, found `()`
41+
42+
error[E0308]: mismatched types
43+
--> $DIR/if-let-typo.rs:8:8
44+
|
45+
LL | if Some(3) = foo {}
46+
| ^^^^^^^^^^^^^ expected `bool`, found `()`
47+
|
48+
help: you might have meant to use pattern matching
49+
|
50+
LL | if let Some(3) = foo {}
51+
| ^^^
52+
help: you might have meant to compare for equality
53+
|
54+
LL | if Some(3) == foo {}
55+
| ^^
56+
57+
error: aborting due to 5 previous errors
58+
59+
Some errors have detailed explanations: E0308, E0425.
60+
For more information about an error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)