fix #101880: suggest let for assignment, and some code refactor

chenyukang · chenyukang · commit eb68e27e4c77 · 2022-10-20T22:54:02.000+08:00
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -524,6 +524,9 @@ struct DiagnosticMetadata<'ast> {
     /// Used to detect possible `if let` written without `let` and to provide structured suggestion.
     in_if_condition: Option<&'ast Expr>,
 
+    /// Used to detect possible new binding written without `let` and to provide structured suggestion.
+    in_assignment: Option<&'ast Expr>,
+
     /// If we are currently in a trait object definition. Used to point at the bounds when
     /// encountering a struct or enum.
     current_trait_object: Option<&'ast [ast::GenericBound]>,
@@ -3905,6 +3908,11 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                 self.resolve_expr(elem, Some(expr));
                 self.visit_expr(idx);
             }
+            ExprKind::Assign(..) => {
+                let old = self.diagnostic_metadata.in_assignment.replace(expr);
+                visit::walk_expr(self, expr);
+                self.diagnostic_metadata.in_assignment = old;
+            }
             _ => {
                 visit::walk_expr(self, expr);
             }
diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs
@@ -679,7 +679,9 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
 
             // If the trait has a single item (which wasn't matched by Levenshtein), suggest it
             let suggestion = self.get_single_associated_item(&path, &source, is_expected);
-            self.r.add_typo_suggestion(err, suggestion, ident_span);
+            if !self.r.add_typo_suggestion(err, suggestion, ident_span) {
+                fallback = !self.let_binding_suggestion(err, ident_span);
+            }
         }
         fallback
     }
@@ -1076,41 +1078,14 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
         // where a brace being opened means a block is being started. Look
         // ahead for the next text to see if `span` is followed by a `{`.
         let sm = self.r.session.source_map();
-        let mut sp = span;
-        loop {
-            sp = sm.next_point(sp);
-            match sm.span_to_snippet(sp) {
-                Ok(ref snippet) => {
-                    if snippet.chars().any(|c| !c.is_whitespace()) {
-                        break;
-                    }
-                }
-                _ => break,
-            }
-        }
+        let sp = sm.span_look_ahead(span, None, Some(50));
         let followed_by_brace = matches!(sm.span_to_snippet(sp), Ok(ref snippet) if snippet == "{");
         // In case this could be a struct literal that needs to be surrounded
         // by parentheses, find the appropriate span.
-        let mut i = 0;
-        let mut closing_brace = None;
-        loop {
-            sp = sm.next_point(sp);
-            match sm.span_to_snippet(sp) {
-                Ok(ref snippet) => {
-                    if snippet == "}" {
-                        closing_brace = Some(span.to(sp));
-                        break;
-                    }
-                }
-                _ => break,
-            }
-            i += 1;
-            // The bigger the span, the more likely we're incorrect --
-            // bound it to 100 chars long.
-            if i > 100 {
-                break;
-            }
-        }
+        let closing_span = sm.span_look_ahead(span, Some("}"), Some(50));
+        let closing_brace: Option<Span> = sm
+            .span_to_snippet(closing_span)
+            .map_or(None, |s| if s == "}" { Some(span.to(closing_span)) } else { None });
         (followed_by_brace, closing_brace)
     }
 
@@ -1727,26 +1702,16 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
                             }
                         }
                         if let Ok(base_snippet) = base_snippet {
-                            let mut sp = after_colon_sp;
-                            for _ in 0..100 {
-                                // Try to find an assignment
-                                sp = sm.next_point(sp);
-                                let snippet = sm.span_to_snippet(sp);
-                                match snippet {
-                                    Ok(ref x) if x.as_str() == "=" => {
-                                        err.span_suggestion(
-                                            base_span,
-                                            "maybe you meant to write an assignment here",
-                                            format!("let {}", base_snippet),
-                                            Applicability::MaybeIncorrect,
-                                        );
-                                        show_label = false;
-                                        break;
-                                    }
-                                    Ok(ref x) if x.as_str() == "\n" => break,
-                                    Err(_) => break,
-                                    Ok(_) => {}
-                                }
+                            // Try to find an assignment
+                            let eq_span = sm.span_look_ahead(after_colon_sp, Some("="), Some(50));
+                            if let Ok(ref snippet) = sm.span_to_snippet(eq_span) && snippet == "=" {
+                                err.span_suggestion(
+                                    base_span,
+                                    "maybe you meant to write an assignment here",
+                                    format!("let {}", base_snippet),
+                                    Applicability::MaybeIncorrect,
+                                );
+                                show_label = false;
                             }
                         }
                     }
@@ -1763,6 +1728,31 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
         false
     }
 
+    fn let_binding_suggestion(&self, err: &mut Diagnostic, ident_span: Span) -> bool {
+        // try to give a suggestion for this pattern: `name = 1`, which is common in other languages
+        let mut added_suggestion = false;
+        if let Some(Expr { kind: ExprKind::Assign(lhs, _rhs, _), .. }) = self.diagnostic_metadata.in_assignment &&
+            let ast::ExprKind::Path(None, _) = lhs.kind {
+                let sm = self.r.session.source_map();
+                let line_span = sm.span_extend_to_line(ident_span);
+                let ident_name = sm.span_to_snippet(ident_span).unwrap();
+                // HACK(chenyukang): make sure ident_name is at the starting of the line to protect against macros
+                if sm
+                    .span_to_snippet(line_span)
+                    .map_or(false, |s| s.trim().starts_with(&ident_name))
+                {
+                    err.span_suggestion_verbose(
+                        ident_span.shrink_to_lo(),
+                        "you might have meant to introduce a new binding",
+                        "let ".to_string(),
+                        Applicability::MaybeIncorrect,
+                    );
+                    added_suggestion = true;
+                }
+            }
+        added_suggestion
+    }
+
     fn find_module(&mut self, def_id: DefId) -> Option<(Module<'a>, ImportSuggestion)> {
         let mut result = None;
         let mut seen_modules = FxHashSet::default();
diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs
@@ -877,6 +877,26 @@ impl SourceMap {
         Span::new(BytePos(start_of_next_point), end_of_next_point, sp.ctxt(), None)
     }
 
+    /// Returns a new span to check next none-whitespace character or some specified expected character
+    /// If `expect` is none, the first span of non-whitespace character is returned.
+    /// If `expect` presented, the first span of the character `expect` is returned
+    /// Otherwise, the span reached to limit is returned.
+    pub fn span_look_ahead(&self, span: Span, expect: Option<&str>, limit: Option<usize>) -> Span {
+        let mut sp = span;
+        for _ in 0..limit.unwrap_or(100 as usize) {
+            sp = self.next_point(sp);
+            if let Ok(ref snippet) = self.span_to_snippet(sp) {
+                if expect.map_or(false, |es| snippet == es) {
+                    break;
+                }
+                if expect.is_none() && snippet.chars().any(|c| !c.is_whitespace()) {
+                    break;
+                }
+            }
+        }
+        sp
+    }
+
     /// Finds the width of the character, either before or after the end of provided span,
     /// depending on the `forwards` parameter.
     fn find_width_of_character_at_span(&self, sp: Span, forwards: bool) -> u32 {
diff --git a/src/test/ui/suggestions/suggest-let-for-assignment.fixed b/src/test/ui/suggestions/suggest-let-for-assignment.fixed
@@ -0,0 +1,17 @@
+// run-rustfix
+
+fn main() {
+    let demo = 1; //~ ERROR cannot find value `demo` in this scope
+    dbg!(demo); //~ ERROR cannot find value `demo` in this scope
+
+    let x = "x"; //~ ERROR cannot find value `x` in this scope
+    println!("x: {}", x); //~ ERROR cannot find value `x` in this scope
+
+    if x == "x" {
+        //~^ ERROR cannot find value `x` in this scope
+        println!("x is 1");
+    }
+
+    let y = 1 + 2; //~ ERROR cannot find value `y` in this scope
+    println!("y: {}", y); //~ ERROR cannot find value `y` in this scope
+}
diff --git a/src/test/ui/suggestions/suggest-let-for-assignment.rs b/src/test/ui/suggestions/suggest-let-for-assignment.rs
@@ -0,0 +1,17 @@
+// run-rustfix
+
+fn main() {
+    demo = 1; //~ ERROR cannot find value `demo` in this scope
+    dbg!(demo); //~ ERROR cannot find value `demo` in this scope
+
+    x = "x"; //~ ERROR cannot find value `x` in this scope
+    println!("x: {}", x); //~ ERROR cannot find value `x` in this scope
+
+    if x == "x" {
+        //~^ ERROR cannot find value `x` in this scope
+        println!("x is 1");
+    }
+
+    y = 1 + 2; //~ ERROR cannot find value `y` in this scope
+    println!("y: {}", y); //~ ERROR cannot find value `y` in this scope
+}
diff --git a/src/test/ui/suggestions/suggest-let-for-assignment.stderr b/src/test/ui/suggestions/suggest-let-for-assignment.stderr
@@ -0,0 +1,60 @@
+error[E0425]: cannot find value `demo` in this scope
+  --> $DIR/suggest-let-for-assignment.rs:4:5
+   |
+LL |     demo = 1;
+   |     ^^^^
+   |
+help: you might have meant to introduce a new binding
+   |
+LL |     let demo = 1;
+   |     +++
+
+error[E0425]: cannot find value `demo` in this scope
+  --> $DIR/suggest-let-for-assignment.rs:5:10
+   |
+LL |     dbg!(demo);
+   |          ^^^^ not found in this scope
+
+error[E0425]: cannot find value `x` in this scope
+  --> $DIR/suggest-let-for-assignment.rs:7:5
+   |
+LL |     x = "x";
+   |     ^
+   |
+help: you might have meant to introduce a new binding
+   |
+LL |     let x = "x";
+   |     +++
+
+error[E0425]: cannot find value `x` in this scope
+  --> $DIR/suggest-let-for-assignment.rs:8:23
+   |
+LL |     println!("x: {}", x);
+   |                       ^ not found in this scope
+
+error[E0425]: cannot find value `x` in this scope
+  --> $DIR/suggest-let-for-assignment.rs:10:8
+   |
+LL |     if x == "x" {
+   |        ^ not found in this scope
+
+error[E0425]: cannot find value `y` in this scope
+  --> $DIR/suggest-let-for-assignment.rs:15:5
+   |
+LL |     y = 1 + 2;
+   |     ^
+   |
+help: you might have meant to introduce a new binding
+   |
+LL |     let y = 1 + 2;
+   |     +++
+
+error[E0425]: cannot find value `y` in this scope
+  --> $DIR/suggest-let-for-assignment.rs:16:23
+   |
+LL |     println!("y: {}", y);
+   |                       ^ not found in this scope
+
+error: aborting due to 7 previous errors
+
+For more information about this error, try `rustc --explain E0425`.
