diff options
| author | Mazdak Farrokhzad <twingoow@gmail.com> | 2019-10-27 16:46:51 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2019-10-27 16:46:51 +0100 |
| commit | a466f014b50a49fc380e5c9d8878c937732b0fb2 (patch) | |
| tree | 7893326dddb6402a6ae31255f550e222bc207149 | |
| parent | b7176b44a203322c834302f3b515f8c10a54f2c1 (diff) | |
| parent | b579c5a2d6948e9ff2efa81db85f7bca64f574e4 (diff) | |
| download | rust-a466f014b50a49fc380e5c9d8878c937732b0fb2.tar.gz rust-a466f014b50a49fc380e5c9d8878c937732b0fb2.zip | |
Rollup merge of #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 #57828.
| -rw-r--r-- | src/librustc_resolve/late.rs | 94 | ||||
| -rw-r--r-- | src/librustc_resolve/late/diagnostics.rs | 47 | ||||
| -rw-r--r-- | src/libsyntax/parse/parser/stmt.rs | 2 | ||||
| -rw-r--r-- | src/test/ui/privacy/privacy-ns2.rs | 1 | ||||
| -rw-r--r-- | src/test/ui/privacy/privacy-ns2.stderr | 29 | ||||
| -rw-r--r-- | src/test/ui/suggestions/let-binding-init-expr-as-ty.rs | 12 | ||||
| -rw-r--r-- | src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr | 28 |
7 files changed, 162 insertions, 51 deletions
diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 136ab1f0444..9b254ab7ec1 100644 --- a/src/librustc_resolve/late.rs +++ b/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"); } } diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 2721df4c687..07f44e0742e 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/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. diff --git a/src/libsyntax/parse/parser/stmt.rs b/src/libsyntax/parse/parser/stmt.rs index d54d9c4b8e9..ea7e4c05ea1 100644 --- a/src/libsyntax/parse/parser/stmt.rs +++ b/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(); diff --git a/src/test/ui/privacy/privacy-ns2.rs b/src/test/ui/privacy/privacy-ns2.rs index c4e400f5adb..61fcebd787e 100644 --- a/src/test/ui/privacy/privacy-ns2.rs +++ b/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() { diff --git a/src/test/ui/privacy/privacy-ns2.stderr b/src/test/ui/privacy/privacy-ns2.stderr index f0d5da68685..58671addecd 100644 --- a/src/test/ui/privacy/privacy-ns2.stderr +++ b/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`. diff --git a/src/test/ui/suggestions/let-binding-init-expr-as-ty.rs b/src/test/ui/suggestions/let-binding-init-expr-as-ty.rs new file mode 100644 index 00000000000..beea951a18a --- /dev/null +++ b/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); +} diff --git a/src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr b/src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr new file mode 100644 index 00000000000..a7c784fe827 --- /dev/null +++ b/src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr @@ -0,0 +1,28 @@ +error[E0573]: expected type, found local variable `num` + --> $DIR/let-binding-init-expr-as-ty.rs:2:27 + | +LL | let foo: i32::from_be(num); + | -- ^^^ not a type + | | + | help: use `=` if you meant to assign + +error: parenthesized type parameters may only be used with a `Fn` trait + --> $DIR/let-binding-init-expr-as-ty.rs:2:19 + | +LL | let foo: i32::from_be(num); + | ^^^^^^^^^^^^ + | + = note: `#[deny(parenthesized_params_in_types_and_modules)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #42238 <https://github.com/rust-lang/rust/issues/42238> + +error[E0223]: ambiguous associated type + --> $DIR/let-binding-init-expr-as-ty.rs:2:14 + | +LL | let foo: i32::from_be(num); + | ^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<i32 as Trait>::from_be` + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0223, E0573. +For more information about an error, try `rustc --explain E0223`. |
