Auto merge of #43929 - oli-obk:use_placement, r=nrc
Improve placement of `use` suggestions r? @nrc cc @estebank @Mark-Simulacrum fixes #42835 fixes #42548 fixes #43769
This commit is contained in:
bors
commit
06bf94a129
10 changed files with 198 additions and 41 deletions
|
|
@ -217,8 +217,10 @@ impl CodeSuggestion {
|
|||
if !buf.ends_with('\n') {
|
||||
push_trailing(buf, prev_line.as_ref(), &prev_hi, None);
|
||||
}
|
||||
// remove trailing newline
|
||||
buf.pop();
|
||||
// remove trailing newlines
|
||||
while buf.ends_with('\n') {
|
||||
buf.pop();
|
||||
}
|
||||
}
|
||||
bufs
|
||||
}
|
||||
|
|
|
|||
|
|
@ -581,6 +581,55 @@ impl<T> ::std::ops::IndexMut<Namespace> for PerNS<T> {
|
|||
}
|
||||
}
|
||||
|
||||
struct UsePlacementFinder {
|
||||
target_module: NodeId,
|
||||
span: Option<Span>,
|
||||
found_use: bool,
|
||||
}
|
||||
|
||||
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);
|
||||
self.found_use = true;
|
||||
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 +1039,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<ImportSuggestion>,
|
||||
/// 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 +1249,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<PrivacyError<'a>>,
|
||||
/// ambiguity errors are delayed for deduplication
|
||||
ambiguity_errors: Vec<AmbiguityError<'a>>,
|
||||
/// `use` injections are delayed for better placement and deduplication
|
||||
use_injections: Vec<UseError<'a>>,
|
||||
|
||||
gated_errors: FxHashSet<Span>,
|
||||
disallowed_shadowing: Vec<&'a LegacyBinding<'a>>,
|
||||
|
||||
|
|
@ -1401,6 +1465,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 +1530,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 +2479,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 +2532,7 @@ impl<'a> Resolver<'a> {
|
|||
format!("Self::{}", path_str));
|
||||
}
|
||||
}
|
||||
return err;
|
||||
return (err, candidates);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2488,22 +2549,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 +2580,7 @@ impl<'a> Resolver<'a> {
|
|||
}
|
||||
err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?",
|
||||
path_str));
|
||||
return err;
|
||||
return (err, candidates);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
|
@ -2530,10 +2591,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<Def>| {
|
||||
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 +3523,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 +3573,22 @@ 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,
|
||||
found_use: false,
|
||||
};
|
||||
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, finder.found_use);
|
||||
}
|
||||
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);
|
||||
|
|
@ -3697,7 +3779,8 @@ fn import_candidate_to_paths(suggestion: &ImportSuggestion) -> (Span, String, St
|
|||
fn show_candidates(err: &mut DiagnosticBuilder,
|
||||
span: Span,
|
||||
candidates: &[ImportSuggestion],
|
||||
better: bool) {
|
||||
better: bool,
|
||||
found_use: bool) {
|
||||
|
||||
// we want consistent results across executions, but candidates are produced
|
||||
// by iterating through a hash map, so make sure they are ordered:
|
||||
|
|
@ -3713,7 +3796,14 @@ fn show_candidates(err: &mut DiagnosticBuilder,
|
|||
let msg = format!("possible {}candidate{} into scope", better, msg_diff);
|
||||
|
||||
for candidate in &mut path_strings {
|
||||
*candidate = format!("use {};\n", candidate);
|
||||
// produce an additional newline to separate the new use statement
|
||||
// from the directly following item.
|
||||
let additional_newline = if found_use {
|
||||
""
|
||||
} else {
|
||||
"\n"
|
||||
};
|
||||
*candidate = format!("use {};\n{}", candidate, additional_newline);
|
||||
}
|
||||
|
||||
err.span_suggestions(span, &msg, path_strings);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
27
src/test/ui/resolve/use_suggestion_placement.rs
Normal file
27
src/test/ui/resolve/use_suggestion_placement.rs
Normal file
|
|
@ -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 <LICENSE-APACHE or
|
||||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
|
||||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<K, V> = HashMap<K, V>;
|
||||
}
|
||||
38
src/test/ui/resolve/use_suggestion_placement.stderr
Normal file
38
src/test/ui/resolve/use_suggestion_placement.stderr
Normal file
|
|
@ -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<K, V> = HashMap<K, V>;
|
||||
| ^^^^^^^ 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<K, V> = HashMap<K, V>;
|
||||
| ^ unused type parameter
|
||||
|
||||
error[E0091]: type parameter `V` is unused
|
||||
--> $DIR/use_suggestion_placement.rs:26:18
|
||||
|
|
||||
26 | type Dict<K, V> = HashMap<K, V>;
|
||||
| ^ unused type parameter
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
|
||||
|
|
@ -1,11 +1,3 @@
|
|||
error[E0577]: expected module, found struct `S`
|
||||
--> $DIR/visibility-ty-params.rs:16:5
|
||||
|
|
||||
16 | m!{ S<u8> } //~ 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<u8> } //~ ERROR generic arguments in visibility path
|
||||
| -^^^^
|
||||
| |
|
||||
| did you mean `m`?
|
||||
|
||||
error: aborting due to 3 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue