From f293eb43d672aeb6a5e5c7301fde07c852e95f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 18 Oct 2019 10:33:25 -0700 Subject: [PATCH 01/13] Use heuristics to suggest assignment When detecting a possible `=` -> `:` typo in a `let` binding, suggest assigning instead of setting the type. --- src/librustc_resolve/late.rs | 15 +++++++ src/librustc_resolve/late/diagnostics.rs | 25 +++++++++-- .../let-binding-init-expr-as-ty.rs | 11 +++++ .../let-binding-init-expr-as-ty.stderr | 41 +++++++++++++++++++ 4 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/suggestions/let-binding-init-expr-as-ty.rs create mode 100644 src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 73a282b1a0ec1..11fcab0c3f4e6 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -354,6 +354,9 @@ struct LateResolutionVisitor<'a, 'b> { /// Only used for better errors on `fn(): fn()`. current_type_ascription: Vec, + + /// Only used for better errors on `let : ;`. + current_let_binding: Option<(Span, Span)>, } /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes. @@ -377,7 +380,18 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> { self.resolve_expr(expr, None); } fn visit_local(&mut self, local: &'tcx Local) { + debug!("visit_local {:?} {:?} {:?}", local, local.pat, local.pat.kind); + let val = match local { + Local { pat, ty: Some(ty), init: None, .. } => match pat.kind { + // We check for this to avoid tuple struct fields. + PatKind::Wild => None, + _ => Some((pat.span, ty.span)), + }, + _ => None, + }; + let original = replace(&mut self.current_let_binding, val); self.resolve_local(local); + self.current_let_binding = original; } fn visit_ty(&mut self, ty: &'tcx Ty) { match ty.kind { @@ -554,6 +568,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { current_function: None, unused_labels: Default::default(), current_type_ascription: Vec::new(), + current_let_binding: None, } } diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 2721df4c68763..1912649f8b57c 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -72,10 +72,14 @@ 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, is_local) = if let Some(res) = res { (format!("expected {}, found {} `{}`", expected, res.descr(), path_str), format!("not a {}", expected), - span) + span, + match res { + Res::Local(_) => true, + _ => false, + }) } else { let item_span = path.last().unwrap().ident.span; let (mod_prefix, mod_str) = if path.len() == 1 { @@ -94,7 +98,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()); @@ -257,6 +262,20 @@ impl<'a> LateResolutionVisitor<'a, '_> { if !levenshtein_worked { err.span_label(base_span, fallback_label); self.type_ascription_suggestion(&mut err, base_span); + if let Some(span) = self.current_let_binding.and_then(|(pat_sp, ty_sp)| { + if ty_sp.contains(base_span) && is_local { + Some(pat_sp.between(ty_sp)) + } else { + None + } + }) { + err.span_suggestion( + span, + "maybe you meant to assign a value instead of defining this let binding's type", + " = ".to_string(), + Applicability::MaybeIncorrect, + ); + } } (err, candidates) } 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 0000000000000..4b572d6255bc6 --- /dev/null +++ b/src/test/ui/suggestions/let-binding-init-expr-as-ty.rs @@ -0,0 +1,11 @@ +pub fn foo(num: i32) -> i32 { //~ ERROR mismatched types + 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 +} + +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 0000000000000..b472c267987a9 --- /dev/null +++ b/src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr @@ -0,0 +1,41 @@ +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: maybe you meant to assign a value instead of defining this let binding's type + | +LL | let foo = i32::from_be(num); + | ^ + +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 + +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: `::from_be` + +error[E0308]: mismatched types + --> $DIR/let-binding-init-expr-as-ty.rs:1:25 + | +LL | pub fn foo(num: i32) -> i32 { + | --- ^^^ expected i32, found () + | | + | implicitly returns `()` as its body has no tail or `return` expression + | + = note: expected type `i32` + found type `()` + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0223, E0308, E0573. +For more information about an error, try `rustc --explain E0223`. From fa0b721d64184af30e9d655a468d0ea084fff4ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 18 Oct 2019 10:37:28 -0700 Subject: [PATCH 02/13] Remove unnecessary error in test --- .../ui/suggestions/let-binding-init-expr-as-ty.rs | 3 ++- .../let-binding-init-expr-as-ty.stderr | 15 ++------------- 2 files changed, 4 insertions(+), 14 deletions(-) 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 index 4b572d6255bc6..beea951a18a29 100644 --- a/src/test/ui/suggestions/let-binding-init-expr-as-ty.rs +++ b/src/test/ui/suggestions/let-binding-init-expr-as-ty.rs @@ -1,9 +1,10 @@ -pub fn foo(num: i32) -> i32 { //~ ERROR mismatched types +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() { 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 index b472c267987a9..f4b0a38a105be 100644 --- a/src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr +++ b/src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr @@ -24,18 +24,7 @@ error[E0223]: ambiguous associated type LL | let foo: i32::from_be(num); | ^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `::from_be` -error[E0308]: mismatched types - --> $DIR/let-binding-init-expr-as-ty.rs:1:25 - | -LL | pub fn foo(num: i32) -> i32 { - | --- ^^^ expected i32, found () - | | - | implicitly returns `()` as its body has no tail or `return` expression - | - = note: expected type `i32` - found type `()` - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0223, E0308, E0573. +Some errors have detailed explanations: E0223, E0573. For more information about an error, try `rustc --explain E0223`. From cdc5400178b71163f3966037e0fae6ce65ca583e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 19 Oct 2019 10:13:56 -0700 Subject: [PATCH 03/13] review comments and tweaks --- src/librustc_resolve/late.rs | 110 ++++++++++-------- src/librustc_resolve/late/diagnostics.rs | 48 +++++--- src/libsyntax/parse/parser/stmt.rs | 2 +- src/test/ui/privacy/privacy-ns2.rs | 1 + src/test/ui/privacy/privacy-ns2.stderr | 28 ++++- .../let-binding-init-expr-as-ty.stderr | 8 +- 6 files changed, 119 insertions(+), 78 deletions(-) diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 11fcab0c3f4e6..93f14b760c26f 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -320,22 +320,7 @@ 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>>, - - /// The current set of local scopes, for labels. - label_ribs: Vec>, - - /// The trait that the current context can refer to. - current_trait_ref: Option<(Module<'a>, TraitRef)>, - +struct DiagnosticMetadata { /// The current trait's associated types' ident, used for diagnostic suggestions. current_trait_assoc_types: Vec, @@ -356,7 +341,27 @@ struct LateResolutionVisitor<'a, 'b> { current_type_ascription: Vec, /// Only used for better errors on `let : ;`. - current_let_binding: Option<(Span, Span)>, + current_let_binding: Option<(Span, Option, Option)>, +} + +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>>, + + /// The current set of local scopes, for labels. + label_ribs: Vec>, + + /// 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. @@ -380,18 +385,18 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> { self.resolve_expr(expr, None); } fn visit_local(&mut self, local: &'tcx Local) { - debug!("visit_local {:?} {:?} {:?}", local, local.pat, local.pat.kind); - let val = match local { - Local { pat, ty: Some(ty), init: None, .. } => match pat.kind { - // We check for this to avoid tuple struct fields. - PatKind::Wild => None, - _ => Some((pat.span, ty.span)), - }, - _ => None, + 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.current_let_binding, val); + let original = replace(&mut self.diagnostic_metadata.current_let_binding, local_spans); self.resolve_local(local); - self.current_let_binding = original; + self.diagnostic_metadata.current_let_binding = original; } fn visit_ty(&mut self, ty: &'tcx Ty) { match ty.kind { @@ -433,7 +438,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, @@ -459,7 +464,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) { @@ -493,7 +498,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`.) - 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); } @@ -562,13 +568,15 @@ 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(), - current_let_binding: None, + diagnostic_metadata: DiagnosticMetadata { + 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(), + current_let_binding: None, + } } } @@ -928,16 +936,22 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { fn with_current_self_type(&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(&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 } @@ -948,14 +962,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 } @@ -1782,7 +1796,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { fn with_resolved_label(&mut self, label: Option