Rollup merge of rust-lang#65566 - estebank:let-expr-as-ty, r=Centril

Use heuristics to suggest assignment

When detecting a possible `=` -> `:` typo in a `let` binding, suggest
assigning instead of setting the type.

Partially address rust-lang#57828.

Author: Centril

src/librustc_resolve/late.rs

@@ -316,22 +316,8 @@ impl<'a> PathSource<'a> {
     }
 }
 
-struct LateResolutionVisitor<'a, 'b> {
-    r: &'b mut Resolver<'a>,
-
-    /// The module that represents the current item scope.
-    parent_scope: ParentScope<'a>,
-
-    /// The current set of local scopes for types and values.
-    /// FIXME #4948: Reuse ribs to avoid allocation.
-    ribs: PerNS<Vec<Rib<'a>>>,
-
-    /// The current set of local scopes, for labels.
-    label_ribs: Vec<Rib<'a, NodeId>>,
-
-    /// The trait that the current context can refer to.
-    current_trait_ref: Option<(Module<'a>, TraitRef)>,
-
+#[derive(Default)]
+struct DiagnosticMetadata {
     /// The current trait's associated types' ident, used for diagnostic suggestions.
     current_trait_assoc_types: Vec<Ident>,
 
@@ -350,6 +336,29 @@ struct LateResolutionVisitor<'a, 'b> {
 
     /// Only used for better errors on `fn(): fn()`.
     current_type_ascription: Vec<Span>,
+
+    /// Only used for better errors on `let <pat>: <expr, not type>;`.
+    current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,
+}
+
+struct LateResolutionVisitor<'a, 'b> {
+    r: &'b mut Resolver<'a>,
+
+    /// The module that represents the current item scope.
+    parent_scope: ParentScope<'a>,
+
+    /// The current set of local scopes for types and values.
+    /// FIXME #4948: Reuse ribs to avoid allocation.
+    ribs: PerNS<Vec<Rib<'a>>>,
+
+    /// The current set of local scopes, for labels.
+    label_ribs: Vec<Rib<'a, NodeId>>,
+
+    /// The trait that the current context can refer to.
+    current_trait_ref: Option<(Module<'a>, TraitRef)>,
+
+    /// Fields used to add information to diagnostic errors.
+    diagnostic_metadata: DiagnosticMetadata,
 }
 
 /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes.
@@ -373,7 +382,18 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
         self.resolve_expr(expr, None);
     }
     fn visit_local(&mut self, local: &'tcx Local) {
+        let local_spans = match local.pat.kind {
+            // We check for this to avoid tuple struct fields.
+            PatKind::Wild => None,
+            _ => Some((
+                local.pat.span,
+                local.ty.as_ref().map(|ty| ty.span),
+                local.init.as_ref().map(|init| init.span),
+            )),
+        };
+        let original = replace(&mut self.diagnostic_metadata.current_let_binding, local_spans);
         self.resolve_local(local);
+        self.diagnostic_metadata.current_let_binding = original;
     }
     fn visit_ty(&mut self, ty: &'tcx Ty) {
         match ty.kind {
@@ -415,7 +435,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
         }
     }
     fn visit_fn(&mut self, fn_kind: FnKind<'tcx>, declaration: &'tcx FnDecl, sp: Span, _: NodeId) {
-        let previous_value = replace(&mut self.current_function, Some(sp));
+        let previous_value = replace(&mut self.diagnostic_metadata.current_function, Some(sp));
         debug!("(resolving function) entering function");
         let rib_kind = match fn_kind {
             FnKind::ItemFn(..) => FnItemRibKind,
@@ -441,7 +461,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
                 debug!("(resolving function) leaving function");
             })
         });
-        self.current_function = previous_value;
+        self.diagnostic_metadata.current_function = previous_value;
     }
 
     fn visit_generics(&mut self, generics: &'tcx Generics) {
@@ -475,7 +495,8 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
         // (We however cannot ban `Self` for defaults on *all* generic
         // lists; e.g. trait generics can usefully refer to `Self`,
         // such as in the case of `trait Add<Rhs = Self>`.)
-        if self.current_self_item.is_some() { // (`Some` if + only if we are in ADT's generics.)
+        if self.diagnostic_metadata.current_self_item.is_some() {
+            // (`Some` if + only if we are in ADT's generics.)
             default_ban_rib.bindings.insert(Ident::with_dummy_span(kw::SelfUpper), Res::Err);
         }
 
@@ -527,12 +548,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
             },
             label_ribs: Vec::new(),
             current_trait_ref: None,
-            current_trait_assoc_types: Vec::new(),
-            current_self_type: None,
-            current_self_item: None,
-            current_function: None,
-            unused_labels: Default::default(),
-            current_type_ascription: Vec::new(),
+            diagnostic_metadata: DiagnosticMetadata::default(),
         }
     }
 
@@ -892,16 +908,22 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
 
     fn with_current_self_type<T>(&mut self, self_type: &Ty, f: impl FnOnce(&mut Self) -> T) -> T {
         // Handle nested impls (inside fn bodies)
-        let previous_value = replace(&mut self.current_self_type, Some(self_type.clone()));
+        let previous_value = replace(
+            &mut self.diagnostic_metadata.current_self_type,
+            Some(self_type.clone()),
+        );
         let result = f(self);
-        self.current_self_type = previous_value;
+        self.diagnostic_metadata.current_self_type = previous_value;
         result
     }
 
     fn with_current_self_item<T>(&mut self, self_item: &Item, f: impl FnOnce(&mut Self) -> T) -> T {
-        let previous_value = replace(&mut self.current_self_item, Some(self_item.id));
+        let previous_value = replace(
+            &mut self.diagnostic_metadata.current_self_item,
+            Some(self_item.id),
+        );
         let result = f(self);
-        self.current_self_item = previous_value;
+        self.diagnostic_metadata.current_self_item = previous_value;
         result
     }
 
@@ -912,14 +934,14 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
         f: impl FnOnce(&mut Self) -> T,
     ) -> T {
         let trait_assoc_types = replace(
-            &mut self.current_trait_assoc_types,
+            &mut self.diagnostic_metadata.current_trait_assoc_types,
             trait_items.iter().filter_map(|item| match &item.kind {
                 TraitItemKind::Type(bounds, _) if bounds.len() == 0 => Some(item.ident),
                 _ => None,
             }).collect(),
         );
         let result = f(self);
-        self.current_trait_assoc_types = trait_assoc_types;
+        self.diagnostic_metadata.current_trait_assoc_types = trait_assoc_types;
         result
     }
 
@@ -1746,7 +1768,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
 
     fn with_resolved_label(&mut self, label: Option<Label>, id: NodeId, f: impl FnOnce(&mut Self)) {
         if let Some(label) = label {
-            self.unused_labels.insert(id, label.ident.span);
+            self.diagnostic_metadata.unused_labels.insert(id, label.ident.span);
             self.with_label_rib(NormalRibKind, |this| {
                 let ident = label.ident.modern_and_legacy();
                 this.label_ribs.last_mut().unwrap().bindings.insert(ident, id);
@@ -1850,7 +1872,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
                     Some(node_id) => {
                         // Since this res is a label, it is never read.
                         self.r.label_res_map.insert(expr.id, node_id);
-                        self.unused_labels.remove(&node_id);
+                        self.diagnostic_metadata.unused_labels.remove(&node_id);
                     }
                 }
 
@@ -1912,9 +1934,9 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
                 }
             }
             ExprKind::Type(ref type_expr, _) => {
-                self.current_type_ascription.push(type_expr.span);
+                self.diagnostic_metadata.current_type_ascription.push(type_expr.span);
                 visit::walk_expr(self, expr);
-                self.current_type_ascription.pop();
+                self.diagnostic_metadata.current_type_ascription.pop();
             }
             // `async |x| ...` gets desugared to `|x| future_from_generator(|| ...)`, so we need to
             // resolve the arguments within the proper scopes so that usages of them inside the
@@ -2073,7 +2095,7 @@ impl<'a> Resolver<'a> {
     pub(crate) fn late_resolve_crate(&mut self, krate: &Crate) {
         let mut late_resolution_visitor = LateResolutionVisitor::new(self);
         visit::walk_crate(&mut late_resolution_visitor, krate);
-        for (id, span) in late_resolution_visitor.unused_labels.iter() {
+        for (id, span) in late_resolution_visitor.diagnostic_metadata.unused_labels.iter() {
             self.session.buffer_lint(lint::builtin::UNUSED_LABELS, *id, *span, "unused label");
         }
     }

src/librustc_resolve/late/diagnostics.rs

@@ -72,10 +72,26 @@ impl<'a> LateResolutionVisitor<'a, '_> {
         let path_str = Segment::names_to_string(path);
         let item_str = path.last().unwrap().ident;
         let code = source.error_code(res.is_some());
-        let (base_msg, fallback_label, base_span) = if let Some(res) = res {
+        let (base_msg, fallback_label, base_span, could_be_expr) = if let Some(res) = res {
             (format!("expected {}, found {} `{}`", expected, res.descr(), path_str),
                 format!("not a {}", expected),
-                span)
+                span,
+                match res {
+                    Res::Def(DefKind::Fn, _) => {
+                        // Verify whether this is a fn call or an Fn used as a type.
+                        self.r.session.source_map().span_to_snippet(span).map(|snippet| {
+                            snippet.ends_with(')')
+                        }).unwrap_or(false)
+                    }
+                    Res::Def(DefKind::Ctor(..), _) |
+                    Res::Def(DefKind::Method, _) |
+                    Res::Def(DefKind::Const, _) |
+                    Res::Def(DefKind::AssocConst, _) |
+                    Res::SelfCtor(_) |
+                    Res::PrimTy(_) |
+                    Res::Local(_) => true,
+                    _ => false,
+                })
         } else {
             let item_span = path.last().unwrap().ident.span;
             let (mod_prefix, mod_str) = if path.len() == 1 {
@@ -94,7 +110,8 @@ impl<'a> LateResolutionVisitor<'a, '_> {
             };
             (format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str),
                 format!("not found in {}", mod_str),
-                item_span)
+                item_span,
+                false)
         };
 
         let code = DiagnosticId::Error(code.into());
@@ -134,7 +151,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
                     "`self` value is a keyword only available in methods with a `self` parameter",
                 ),
             });
-            if let Some(span) = &self.current_function {
+            if let Some(span) = &self.diagnostic_metadata.current_function {
                 err.span_label(*span, "this function doesn't have a `self` parameter");
             }
             return (err, Vec::new());
@@ -257,6 +274,18 @@ impl<'a> LateResolutionVisitor<'a, '_> {
         if !levenshtein_worked {
             err.span_label(base_span, fallback_label);
             self.type_ascription_suggestion(&mut err, base_span);
+            match self.diagnostic_metadata.current_let_binding {
+                Some((pat_sp, Some(ty_sp), None))
+                if ty_sp.contains(base_span) && could_be_expr => {
+                    err.span_suggestion_short(
+                        pat_sp.between(ty_sp),
+                        "use `=` if you meant to assign",
+                        " = ".to_string(),
+                        Applicability::MaybeIncorrect,
+                    );
+                }
+                _ => {}
+            }
         }
         (err, candidates)
     }
@@ -491,7 +520,9 @@ impl<'a> LateResolutionVisitor<'a, '_> {
 
         // Fields are generally expected in the same contexts as locals.
         if filter_fn(Res::Local(ast::DUMMY_NODE_ID)) {
-            if let Some(node_id) = self.current_self_type.as_ref().and_then(extract_node_id) {
+            if let Some(node_id) = self.diagnostic_metadata.current_self_type.as_ref()
+                .and_then(extract_node_id)
+            {
                 // Look for a field with the same name in the current self_type.
                 if let Some(resolution) = self.r.partial_res_map.get(&node_id) {
                     match resolution.base_res() {
@@ -510,7 +541,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
             }
         }
 
-        for assoc_type_ident in &self.current_trait_assoc_types {
+        for assoc_type_ident in &self.diagnostic_metadata.current_trait_assoc_types {
             if *assoc_type_ident == ident {
                 return Some(AssocSuggestion::AssocItem);
             }
@@ -644,11 +675,9 @@ impl<'a> LateResolutionVisitor<'a, '_> {
         err: &mut DiagnosticBuilder<'_>,
         base_span: Span,
     ) {
-        debug!("type_ascription_suggetion {:?}", base_span);
         let cm = self.r.session.source_map();
         let base_snippet = cm.span_to_snippet(base_span);
-        debug!("self.current_type_ascription {:?}", self.current_type_ascription);
-        if let Some(sp) = self.current_type_ascription.last() {
+        if let Some(sp) = self.diagnostic_metadata.current_type_ascription.last() {
             let mut sp = *sp;
             loop {
                 // Try to find the `:`; bail on first non-':' / non-whitespace.

src/libsyntax/parse/parser/stmt.rs

@@ -239,7 +239,7 @@ impl<'a> Parser<'a> {
                 err.span_suggestion_short(
                     colon_sp,
                     "use `=` if you meant to assign",
-                    "=".to_string(),
+                    " =".to_string(),
                     Applicability::MachineApplicable
                 );
                 err.emit();

src/test/ui/privacy/privacy-ns2.rs

@@ -39,6 +39,7 @@ fn test_single2() {
     use foo2::Bar;
 
     let _x : Box<Bar>; //~ ERROR expected type, found function `Bar`
+    let _x : Bar(); //~ ERROR expected type, found function `Bar`
 }
 
 fn test_list2() {

src/test/ui/privacy/privacy-ns2.stderr

@@ -48,7 +48,26 @@ LL | use foo3::Bar;
    |
 
 error[E0573]: expected type, found function `Bar`
-  --> $DIR/privacy-ns2.rs:47:17
+  --> $DIR/privacy-ns2.rs:42:14
+   |
+LL |     let _x : Bar();
+   |              ^^^^^ not a type
+   |
+help: use `=` if you meant to assign
+   |
+LL |     let _x = Bar();
+   |            ^
+help: possible better candidates are found in other modules, you can import them into scope
+   |
+LL | use foo1::Bar;
+   |
+LL | use foo2::Bar;
+   |
+LL | use foo3::Bar;
+   |
+
+error[E0573]: expected type, found function `Bar`
+  --> $DIR/privacy-ns2.rs:48:17
    |
 LL |     let _x: Box<Bar>;
    |                 ^^^
@@ -67,24 +86,24 @@ LL | use foo3::Bar;
    |
 
 error[E0603]: trait `Bar` is private
-  --> $DIR/privacy-ns2.rs:60:15
+  --> $DIR/privacy-ns2.rs:61:15
    |
 LL |     use foo3::Bar;
    |               ^^^
 
 error[E0603]: trait `Bar` is private
-  --> $DIR/privacy-ns2.rs:64:15
+  --> $DIR/privacy-ns2.rs:65:15
    |
 LL |     use foo3::Bar;
    |               ^^^
 
 error[E0603]: trait `Bar` is private
-  --> $DIR/privacy-ns2.rs:71:16
+  --> $DIR/privacy-ns2.rs:72:16
    |
 LL |     use foo3::{Bar,Baz};
    |                ^^^
 
-error: aborting due to 7 previous errors
+error: aborting due to 8 previous errors
 
 Some errors have detailed explanations: E0423, E0573, E0603.
 For more information about an error, try `rustc --explain E0423`.

src/test/ui/suggestions/let-binding-init-expr-as-ty.rs

@@ -0,0 +1,12 @@
+pub fn foo(num: i32) -> i32 {
+    let foo: i32::from_be(num);
+    //~^ ERROR expected type, found local variable `num`
+    //~| ERROR parenthesized type parameters may only be used with a `Fn` trait
+    //~| ERROR ambiguous associated type
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+    foo
+}
+
+fn main() {
+    let _ = foo(42);
+}