From e0ba29c41396da831cd3d431f4484516eb4434ab Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Thu, 17 Aug 2017 11:03:59 +0200 Subject: [PATCH] Improve placement of `use` suggestions --- src/librustc_resolve/lib.rs | 119 +++++++++++++++--- .../ui/resolve/enums-are-namespaced-xc.stderr | 6 +- src/test/ui/resolve/issue-21221-3.stderr | 2 +- src/test/ui/resolve/issue-21221-4.stderr | 2 +- src/test/ui/resolve/issue-3907.stderr | 2 +- .../ui/resolve/privacy-struct-ctor.stderr | 6 +- .../ui/resolve/use_suggestion_placement.rs | 27 ++++ .../resolve/use_suggestion_placement.stderr | 38 ++++++ src/test/ui/span/visibility-ty-params.stderr | 16 +-- 9 files changed, 181 insertions(+), 37 deletions(-) create mode 100644 src/test/ui/resolve/use_suggestion_placement.rs create mode 100644 src/test/ui/resolve/use_suggestion_placement.stderr diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 2502f04ee6aef..500639dfde699 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -581,6 +581,53 @@ impl ::std::ops::IndexMut for PerNS { } } +struct UsePlacementFinder { + target_module: NodeId, + span: Option, +} + +impl<'tcx> Visitor<'tcx> for UsePlacementFinder { + fn visit_mod( + &mut self, + module: &'tcx ast::Mod, + _: Span, + _: &[ast::Attribute], + node_id: NodeId, + ) { + if self.span.is_some() { + return; + } + if node_id != self.target_module { + visit::walk_mod(self, module); + return; + } + // find a use statement + for item in &module.items { + match item.node { + ItemKind::Use(..) => { + // don't suggest placing a use before the prelude + // import or other generated ones + if item.span == DUMMY_SP { + let mut span = item.span; + span.hi = span.lo; + self.span = Some(span); + return; + } + }, + // don't place use before extern crate + ItemKind::ExternCrate(_) => {} + // but place them before the first other item + _ => if self.span.map_or(true, |span| item.span < span ) { + let mut span = item.span; + span.hi = span.lo; + self.span = Some(span); + }, + } + } + assert!(self.span.is_some(), "a file can't have no items and emit suggestions"); + } +} + impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { fn visit_item(&mut self, item: &'tcx Item) { self.resolve_item(item); @@ -990,6 +1037,16 @@ enum NameBindingKind<'a> { struct PrivacyError<'a>(Span, Name, &'a NameBinding<'a>); +struct UseError<'a> { + err: DiagnosticBuilder<'a>, + /// Attach `use` statements for these candidates + candidates: Vec, + /// The node id of the module to place the use statements in + node_id: NodeId, + /// Whether the diagnostic should state that it's "better" + better: bool, +} + struct AmbiguityError<'a> { span: Span, name: Name, @@ -1190,15 +1247,20 @@ pub struct Resolver<'a> { extern_module_map: FxHashMap<(DefId, bool /* MacrosOnly? */), Module<'a>>, pub make_glob_map: bool, - // Maps imports to the names of items actually imported (this actually maps - // all imports, but only glob imports are actually interesting). + /// Maps imports to the names of items actually imported (this actually maps + /// all imports, but only glob imports are actually interesting). pub glob_map: GlobMap, used_imports: FxHashSet<(NodeId, Namespace)>, pub maybe_unused_trait_imports: NodeSet, + /// privacy errors are delayed until the end in order to deduplicate them privacy_errors: Vec>, + /// ambiguity errors are delayed for deduplication ambiguity_errors: Vec>, + /// `use` injections are delayed for better placement and deduplication + use_injections: Vec>, + gated_errors: FxHashSet, disallowed_shadowing: Vec<&'a LegacyBinding<'a>>, @@ -1401,6 +1463,7 @@ impl<'a> Resolver<'a> { privacy_errors: Vec::new(), ambiguity_errors: Vec::new(), + use_injections: Vec::new(), gated_errors: FxHashSet(), disallowed_shadowing: Vec::new(), @@ -1465,10 +1528,11 @@ impl<'a> Resolver<'a> { ImportResolver { resolver: self }.finalize_imports(); self.current_module = self.graph_root; self.finalize_current_module_macro_resolutions(); + visit::walk_crate(self, krate); check_unused::check_crate(self, krate); - self.report_errors(); + self.report_errors(krate); self.crate_loader.postprocess(krate); } @@ -2413,25 +2477,20 @@ impl<'a> Resolver<'a> { __diagnostic_used!(E0411); err.code("E0411".into()); err.span_label(span, "`Self` is only available in traits and impls"); - return err; + return (err, Vec::new()); } if is_self_value(path, ns) { __diagnostic_used!(E0424); err.code("E0424".into()); err.span_label(span, format!("`self` value is only available in \ methods with `self` parameter")); - return err; + return (err, Vec::new()); } // Try to lookup the name in more relaxed fashion for better error reporting. let ident = *path.last().unwrap(); let candidates = this.lookup_import_candidates(ident.node.name, ns, is_expected); - if !candidates.is_empty() { - let mut module_span = this.current_module.span; - module_span.hi = module_span.lo; - // Report import candidates as help and proceed searching for labels. - show_candidates(&mut err, module_span, &candidates, def.is_some()); - } else if is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) { + if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) { let enum_candidates = this.lookup_import_candidates(ident.node.name, ns, is_enum_variant); let mut enum_candidates = enum_candidates.iter() @@ -2471,7 +2530,7 @@ impl<'a> Resolver<'a> { format!("Self::{}", path_str)); } } - return err; + return (err, candidates); } } @@ -2488,22 +2547,22 @@ impl<'a> Resolver<'a> { match (def, source) { (Def::Macro(..), _) => { err.span_label(span, format!("did you mean `{}!(...)`?", path_str)); - return err; + return (err, candidates); } (Def::TyAlias(..), PathSource::Trait) => { err.span_label(span, "type aliases cannot be used for traits"); - return err; + return (err, candidates); } (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node { ExprKind::Field(_, ident) => { err.span_label(parent.span, format!("did you mean `{}::{}`?", path_str, ident.node)); - return err; + return (err, candidates); } ExprKind::MethodCall(ref segment, ..) => { err.span_label(parent.span, format!("did you mean `{}::{}(...)`?", path_str, segment.identifier)); - return err; + return (err, candidates); } _ => {} }, @@ -2519,7 +2578,7 @@ impl<'a> Resolver<'a> { } err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", path_str)); - return err; + return (err, candidates); } _ => {} } @@ -2530,10 +2589,14 @@ impl<'a> Resolver<'a> { err.span_label(base_span, fallback_label); this.type_ascription_suggestion(&mut err, base_span); } - err + (err, candidates) }; let report_errors = |this: &mut Self, def: Option| { - report_errors(this, def).emit(); + let (err, candidates) = report_errors(this, def); + let def_id = this.current_module.normal_ancestor_id; + let node_id = this.definitions.as_local_node_id(def_id).unwrap(); + let better = def.is_some(); + this.use_injections.push(UseError { err, candidates, node_id, better }); err_path_resolution() }; @@ -3458,8 +3521,9 @@ impl<'a> Resolver<'a> { vis.is_accessible_from(module.normal_ancestor_id, self) } - fn report_errors(&mut self) { + fn report_errors(&mut self, krate: &Crate) { self.report_shadowing_errors(); + self.report_with_use_injections(krate); let mut reported_spans = FxHashSet(); for &AmbiguityError { span, name, b1, b2, lexical, legacy } in &self.ambiguity_errors { @@ -3507,6 +3571,21 @@ impl<'a> Resolver<'a> { } } + fn report_with_use_injections(&mut self, krate: &Crate) { + for UseError { mut err, candidates, node_id, better } in self.use_injections.drain(..) { + let mut finder = UsePlacementFinder { + target_module: node_id, + span: None, + }; + visit::walk_crate(&mut finder, krate); + if !candidates.is_empty() { + let span = finder.span.expect("did not find module"); + show_candidates(&mut err, span, &candidates, better); + } + err.emit(); + } + } + fn report_shadowing_errors(&mut self) { for (ident, scope) in replace(&mut self.lexical_macro_resolutions, Vec::new()) { self.resolve_legacy_scope(scope, ident, true); diff --git a/src/test/ui/resolve/enums-are-namespaced-xc.stderr b/src/test/ui/resolve/enums-are-namespaced-xc.stderr index 4b32ecff2fbdf..a401861274deb 100644 --- a/src/test/ui/resolve/enums-are-namespaced-xc.stderr +++ b/src/test/ui/resolve/enums-are-namespaced-xc.stderr @@ -6,7 +6,7 @@ error[E0425]: cannot find value `A` in module `namespaced_enums` | help: possible candidate is found in another module, you can import it into scope | -12 | use namespaced_enums::Foo::A; +14 | use namespaced_enums::Foo::A; | error[E0425]: cannot find function `B` in module `namespaced_enums` @@ -17,7 +17,7 @@ error[E0425]: cannot find function `B` in module `namespaced_enums` | help: possible candidate is found in another module, you can import it into scope | -12 | use namespaced_enums::Foo::B; +14 | use namespaced_enums::Foo::B; | error[E0422]: cannot find struct, variant or union type `C` in module `namespaced_enums` @@ -28,7 +28,7 @@ error[E0422]: cannot find struct, variant or union type `C` in module `namespace | help: possible candidate is found in another module, you can import it into scope | -12 | use namespaced_enums::Foo::C; +14 | use namespaced_enums::Foo::C; | error: aborting due to 3 previous errors diff --git a/src/test/ui/resolve/issue-21221-3.stderr b/src/test/ui/resolve/issue-21221-3.stderr index f4d183192b6cb..da849ecc71ab4 100644 --- a/src/test/ui/resolve/issue-21221-3.stderr +++ b/src/test/ui/resolve/issue-21221-3.stderr @@ -6,7 +6,7 @@ error[E0405]: cannot find trait `OuterTrait` in this scope | help: possible candidate is found in another module, you can import it into scope | -16 | use issue_21221_3::outer::OuterTrait; +18 | use issue_21221_3::outer::OuterTrait; | error: cannot continue compilation due to previous error diff --git a/src/test/ui/resolve/issue-21221-4.stderr b/src/test/ui/resolve/issue-21221-4.stderr index eb71ee893ce71..78059ed37bee8 100644 --- a/src/test/ui/resolve/issue-21221-4.stderr +++ b/src/test/ui/resolve/issue-21221-4.stderr @@ -6,7 +6,7 @@ error[E0405]: cannot find trait `T` in this scope | help: possible candidate is found in another module, you can import it into scope | -16 | use issue_21221_4::T; +18 | use issue_21221_4::T; | error: cannot continue compilation due to previous error diff --git a/src/test/ui/resolve/issue-3907.stderr b/src/test/ui/resolve/issue-3907.stderr index 70b4599dbf8cb..7a4d0ca698e6d 100644 --- a/src/test/ui/resolve/issue-3907.stderr +++ b/src/test/ui/resolve/issue-3907.stderr @@ -6,7 +6,7 @@ error[E0404]: expected trait, found type alias `Foo` | help: possible better candidate is found in another module, you can import it into scope | -12 | use issue_3907::Foo; +14 | use issue_3907::Foo; | error: cannot continue compilation due to previous error diff --git a/src/test/ui/resolve/privacy-struct-ctor.stderr b/src/test/ui/resolve/privacy-struct-ctor.stderr index 47141eda33c55..ee1481ec6f2b0 100644 --- a/src/test/ui/resolve/privacy-struct-ctor.stderr +++ b/src/test/ui/resolve/privacy-struct-ctor.stderr @@ -10,7 +10,7 @@ error[E0423]: expected value, found struct `Z` | help: possible better candidate is found in another module, you can import it into scope | -15 | use m::n::Z; +16 | use m::n::Z; | error[E0423]: expected value, found struct `S` @@ -24,7 +24,7 @@ error[E0423]: expected value, found struct `S` | help: possible better candidate is found in another module, you can import it into scope | -13 | use m::S; +15 | use m::S; | error[E0423]: expected value, found struct `xcrate::S` @@ -38,7 +38,7 @@ error[E0423]: expected value, found struct `xcrate::S` | help: possible better candidate is found in another module, you can import it into scope | -13 | use m::S; +15 | use m::S; | error[E0603]: tuple struct `Z` is private diff --git a/src/test/ui/resolve/use_suggestion_placement.rs b/src/test/ui/resolve/use_suggestion_placement.rs new file mode 100644 index 0000000000000..e0027fed4d6f2 --- /dev/null +++ b/src/test/ui/resolve/use_suggestion_placement.rs @@ -0,0 +1,27 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +macro_rules! y { + () => {} +} + +mod m { + pub const A: i32 = 0; +} + +fn main() { + y!(); + let _ = A; + foo(); +} + +fn foo() { + type Dict = HashMap; +} diff --git a/src/test/ui/resolve/use_suggestion_placement.stderr b/src/test/ui/resolve/use_suggestion_placement.stderr new file mode 100644 index 0000000000000..5c74d8bed6665 --- /dev/null +++ b/src/test/ui/resolve/use_suggestion_placement.stderr @@ -0,0 +1,38 @@ +error[E0425]: cannot find value `A` in this scope + --> $DIR/use_suggestion_placement.rs:21:13 + | +21 | let _ = A; + | ^ not found in this scope + | +help: possible candidate is found in another module, you can import it into scope + | +11 | use m::A; + | + +error[E0412]: cannot find type `HashMap` in this scope + --> $DIR/use_suggestion_placement.rs:26:23 + | +26 | type Dict = HashMap; + | ^^^^^^^ not found in this scope + | +help: possible candidates are found in other modules, you can import them into scope + | +11 | use std::collections::HashMap; + | +11 | use std::collections::hash_map::HashMap; + | + +error[E0091]: type parameter `K` is unused + --> $DIR/use_suggestion_placement.rs:26:15 + | +26 | type Dict = HashMap; + | ^ unused type parameter + +error[E0091]: type parameter `V` is unused + --> $DIR/use_suggestion_placement.rs:26:18 + | +26 | type Dict = HashMap; + | ^ unused type parameter + +error: aborting due to 4 previous errors + diff --git a/src/test/ui/span/visibility-ty-params.stderr b/src/test/ui/span/visibility-ty-params.stderr index 0460b7ca025a6..344cf69748ecc 100644 --- a/src/test/ui/span/visibility-ty-params.stderr +++ b/src/test/ui/span/visibility-ty-params.stderr @@ -1,11 +1,3 @@ -error[E0577]: expected module, found struct `S` - --> $DIR/visibility-ty-params.rs:16:5 - | -16 | m!{ S } //~ ERROR generic arguments in visibility path - | -^^^^ - | | - | did you mean `m`? - error: generic arguments in visibility path --> $DIR/visibility-ty-params.rs:16:6 | @@ -18,5 +10,13 @@ error: generic arguments in visibility path 20 | m!{ m<> } //~ ERROR generic arguments in visibility path | ^^ +error[E0577]: expected module, found struct `S` + --> $DIR/visibility-ty-params.rs:16:5 + | +16 | m!{ S } //~ ERROR generic arguments in visibility path + | -^^^^ + | | + | did you mean `m`? + error: aborting due to 3 previous errors