Skip to content

Commit

Permalink
Auto merge of #46035 - oli-obk:use_suggestions, r=petrochenkov
Browse files Browse the repository at this point in the history
Add structured suggestions for various "use" suggestions

r? @petrochenkov
  • Loading branch information
bors committed Nov 22, 2017
2 parents 1dc0b57 + 2c2891b commit 45594d5
Show file tree
Hide file tree
Showing 6 changed files with 207 additions and 43 deletions.
25 changes: 18 additions & 7 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,18 @@ struct UsePlacementFinder {
found_use: bool,
}

impl UsePlacementFinder {
fn check(krate: &Crate, target_module: NodeId) -> (Option<Span>, bool) {
let mut finder = UsePlacementFinder {
target_module,
span: None,
found_use: false,
};
visit::walk_crate(&mut finder, krate);
(finder.span, finder.found_use)
}
}

impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
fn visit_mod(
&mut self,
Expand Down Expand Up @@ -1278,6 +1290,8 @@ pub struct Resolver<'a> {
ambiguity_errors: Vec<AmbiguityError<'a>>,
/// `use` injections are delayed for better placement and deduplication
use_injections: Vec<UseError<'a>>,
/// `use` injections for proc macros wrongly imported with #[macro_use]
proc_mac_errors: Vec<macros::ProcMacError>,

gated_errors: FxHashSet<Span>,
disallowed_shadowing: Vec<&'a LegacyBinding<'a>>,
Expand Down Expand Up @@ -1486,6 +1500,7 @@ impl<'a> Resolver<'a> {
privacy_errors: Vec::new(),
ambiguity_errors: Vec::new(),
use_injections: Vec::new(),
proc_mac_errors: Vec::new(),
gated_errors: FxHashSet(),
disallowed_shadowing: Vec::new(),

Expand Down Expand Up @@ -3569,6 +3584,7 @@ impl<'a> Resolver<'a> {
fn report_errors(&mut self, krate: &Crate) {
self.report_shadowing_errors();
self.report_with_use_injections(krate);
self.report_proc_macro_import(krate);
let mut reported_spans = FxHashSet();

for &AmbiguityError { span, name, b1, b2, lexical, legacy } in &self.ambiguity_errors {
Expand Down Expand Up @@ -3618,14 +3634,9 @@ 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);
let (span, found_use) = UsePlacementFinder::check(krate, node_id);
if !candidates.is_empty() {
show_candidates(&mut err, finder.span, &candidates, better, finder.found_use);
show_candidates(&mut err, span, &candidates, better, found_use);
}
err.emit();
}
Expand Down
43 changes: 38 additions & 5 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ pub struct LegacyBinding<'a> {
pub span: Span,
}

pub struct ProcMacError {
crate_name: Symbol,
name: Symbol,
module: ast::NodeId,
use_span: Span,
warn_msg: &'static str,
}

#[derive(Copy, Clone)]
pub enum MacroBinding<'a> {
Legacy(&'a LegacyBinding<'a>),
Expand Down Expand Up @@ -779,12 +787,37 @@ impl<'a> Resolver<'a> {
_ => return,
};

let crate_name = self.cstore.crate_name_untracked(krate);
let def_id = self.current_module.normal_ancestor_id;
let node_id = self.definitions.as_local_node_id(def_id).unwrap();

self.proc_mac_errors.push(ProcMacError {
crate_name: self.cstore.crate_name_untracked(krate),
name,
module: node_id,
use_span,
warn_msg,
});
}

pub fn report_proc_macro_import(&mut self, krate: &ast::Crate) {
for err in self.proc_mac_errors.drain(..) {
let (span, found_use) = ::UsePlacementFinder::check(krate, err.module);

self.session.struct_span_err(use_span, warn_msg)
.help(&format!("instead, import the procedural macro like any other item: \
`use {}::{};`", crate_name, name))
.emit();
if let Some(span) = span {
let found_use = if found_use { "" } else { "\n" };
self.session.struct_span_err(err.use_span, err.warn_msg)
.span_suggestion(
span,
"instead, import the procedural macro like any other item",
format!("use {}::{};{}", err.crate_name, err.name, found_use),
).emit();
} else {
self.session.struct_span_err(err.use_span, err.warn_msg)
.help(&format!("instead, import the procedural macro like any other item: \
`use {}::{};`", err.crate_name, err.name))
.emit();
}
}
}

fn gate_legacy_custom_derive(&mut self, name: Symbol, span: Span) {
Expand Down
117 changes: 108 additions & 9 deletions src/librustc_typeck/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,16 +340,35 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
err: &mut DiagnosticBuilder,
mut msg: String,
candidates: Vec<DefId>) {
let limit = if candidates.len() == 5 { 5 } else { 4 };
for (i, trait_did) in candidates.iter().take(limit).enumerate() {
msg.push_str(&format!("\ncandidate #{}: `use {};`",
i + 1,
self.tcx.item_path_str(*trait_did)));
}
if candidates.len() > limit {
msg.push_str(&format!("\nand {} others", candidates.len() - limit));
let module_did = self.tcx.hir.get_module_parent(self.body_id);
let module_id = self.tcx.hir.as_local_node_id(module_did).unwrap();
let krate = self.tcx.hir.krate();
let (span, found_use) = UsePlacementFinder::check(self.tcx, krate, module_id);
if let Some(span) = span {
let path_strings = candidates.iter().map(|did| {
// produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use {
""
} else {
"\n"
};
format!("use {};\n{}", self.tcx.item_path_str(*did), additional_newline)
}).collect();

err.span_suggestions(span, &msg, path_strings);
} else {
let limit = if candidates.len() == 5 { 5 } else { 4 };
for (i, trait_did) in candidates.iter().take(limit).enumerate() {
msg.push_str(&format!("\ncandidate #{}: `use {};`",
i + 1,
self.tcx.item_path_str(*trait_did)));
}
if candidates.len() > limit {
msg.push_str(&format!("\nand {} others", candidates.len() - limit));
}
err.note(&msg[..]);
}
err.note(&msg[..]);
}

fn suggest_valid_traits(&self,
Expand Down Expand Up @@ -604,3 +623,83 @@ impl<'a> Iterator for AllTraits<'a> {
})
}
}


struct UsePlacementFinder<'a, 'tcx: 'a, 'gcx: 'tcx> {
target_module: ast::NodeId,
span: Option<Span>,
found_use: bool,
tcx: TyCtxt<'a, 'gcx, 'tcx>
}

impl<'a, 'tcx, 'gcx> UsePlacementFinder<'a, 'tcx, 'gcx> {
fn check(
tcx: TyCtxt<'a, 'gcx, 'tcx>,
krate: &'tcx hir::Crate,
target_module: ast::NodeId,
) -> (Option<Span>, bool) {
let mut finder = UsePlacementFinder {
target_module,
span: None,
found_use: false,
tcx,
};
hir::intravisit::walk_crate(&mut finder, krate);
(finder.span, finder.found_use)
}
}

impl<'a, 'tcx, 'gcx> hir::intravisit::Visitor<'tcx> for UsePlacementFinder<'a, 'tcx, 'gcx> {
fn visit_mod(
&mut self,
module: &'tcx hir::Mod,
_: Span,
node_id: ast::NodeId,
) {
if self.span.is_some() {
return;
}
if node_id != self.target_module {
hir::intravisit::walk_mod(self, module, node_id);
return;
}
// find a use statement
for item_id in &module.item_ids {
let item = self.tcx.hir.expect_item(item_id.id);
match item.node {
hir::ItemUse(..) => {
// don't suggest placing a use before the prelude
// import or other generated ones
if item.span.ctxt().outer().expn_info().is_none() {
self.span = Some(item.span.with_hi(item.span.lo()));
self.found_use = true;
return;
}
},
// don't place use before extern crate
hir::ItemExternCrate(_) => {}
// but place them before the first other item
_ => if self.span.map_or(true, |span| item.span < span ) {
if item.span.ctxt().outer().expn_info().is_none() {
// don't insert between attributes and an item
if item.attrs.is_empty() {
self.span = Some(item.span.with_hi(item.span.lo()));
} else {
// find the first attribute on the item
for attr in &item.attrs {
if self.span.map_or(true, |span| attr.span < span) {
self.span = Some(attr.span.with_hi(attr.span.lo()));
}
}
}
}
},
}
}
}
fn nested_visit_map<'this>(
&'this mut self
) -> hir::intravisit::NestedVisitorMap<'this, 'tcx> {
hir::intravisit::NestedVisitorMap::None
}
}
54 changes: 36 additions & 18 deletions src/test/ui/impl-trait/no-method-suggested-traits.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@ error[E0599]: no method named `method` found for type `u32` in the current scope
| ^^^^^^
|
= help: items from traits can only be used if the trait is in scope
= note: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
candidate #1: `use foo::Bar;`
candidate #2: `use no_method_suggested_traits::foo::PubPub;`
candidate #3: `use no_method_suggested_traits::qux::PrivPub;`
candidate #4: `use no_method_suggested_traits::Reexported;`
help: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
|
14 | use foo::Bar;
|
14 | use no_method_suggested_traits::foo::PubPub;
|
14 | use no_method_suggested_traits::qux::PrivPub;
|
14 | use no_method_suggested_traits::Reexported;
|

error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::boxed::Box<&u32>>` in the current scope
--> $DIR/no-method-suggested-traits.rs:38:44
Expand All @@ -18,11 +23,16 @@ error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::box
| ^^^^^^
|
= help: items from traits can only be used if the trait is in scope
= note: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
candidate #1: `use foo::Bar;`
candidate #2: `use no_method_suggested_traits::foo::PubPub;`
candidate #3: `use no_method_suggested_traits::qux::PrivPub;`
candidate #4: `use no_method_suggested_traits::Reexported;`
help: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
|
14 | use foo::Bar;
|
14 | use no_method_suggested_traits::foo::PubPub;
|
14 | use no_method_suggested_traits::qux::PrivPub;
|
14 | use no_method_suggested_traits::Reexported;
|

error[E0599]: no method named `method` found for type `char` in the current scope
--> $DIR/no-method-suggested-traits.rs:44:9
Expand All @@ -31,8 +41,10 @@ error[E0599]: no method named `method` found for type `char` in the current scop
| ^^^^^^
|
= help: items from traits can only be used if the trait is in scope
= note: the following trait is implemented but not in scope, perhaps add a `use` for it:
candidate #1: `use foo::Bar;`
help: the following trait is implemented but not in scope, perhaps add a `use` for it:
|
14 | use foo::Bar;
|

error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::boxed::Box<&char>>` in the current scope
--> $DIR/no-method-suggested-traits.rs:48:43
Expand All @@ -41,8 +53,10 @@ error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::box
| ^^^^^^
|
= help: items from traits can only be used if the trait is in scope
= note: the following trait is implemented but not in scope, perhaps add a `use` for it:
candidate #1: `use foo::Bar;`
help: the following trait is implemented but not in scope, perhaps add a `use` for it:
|
14 | use foo::Bar;
|

error[E0599]: no method named `method` found for type `i32` in the current scope
--> $DIR/no-method-suggested-traits.rs:53:10
Expand All @@ -51,8 +65,10 @@ error[E0599]: no method named `method` found for type `i32` in the current scope
| ^^^^^^
|
= help: items from traits can only be used if the trait is in scope
= note: the following trait is implemented but not in scope, perhaps add a `use` for it:
candidate #1: `use no_method_suggested_traits::foo::PubPub;`
help: the following trait is implemented but not in scope, perhaps add a `use` for it:
|
14 | use no_method_suggested_traits::foo::PubPub;
|

error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::boxed::Box<&i32>>` in the current scope
--> $DIR/no-method-suggested-traits.rs:57:44
Expand All @@ -61,8 +77,10 @@ error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::box
| ^^^^^^
|
= help: items from traits can only be used if the trait is in scope
= note: the following trait is implemented but not in scope, perhaps add a `use` for it:
candidate #1: `use no_method_suggested_traits::foo::PubPub;`
help: the following trait is implemented but not in scope, perhaps add a `use` for it:
|
14 | use no_method_suggested_traits::foo::PubPub;
|

error[E0599]: no method named `method` found for type `Foo` in the current scope
--> $DIR/no-method-suggested-traits.rs:62:9
Expand Down
5 changes: 3 additions & 2 deletions src/test/ui/issue-35976.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ error: the `wait` method cannot be invoked on a trait object
|
24 | arg.wait();
| ^^^^
help: another candidate was found in the following trait, perhaps add a `use` for it:
|
11 | use private::Future;
|
= note: another candidate was found in the following trait, perhaps add a `use` for it:
candidate #1: `use private::Future;`

error: aborting due to previous error

6 changes: 4 additions & 2 deletions src/test/ui/trait-method-private.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ error[E0624]: method `method` is private
| ^^^^^^
|
= help: items from traits can only be used if the trait is in scope
= note: the following trait is implemented but not in scope, perhaps add a `use` for it:
candidate #1: `use inner::Bar;`
help: the following trait is implemented but not in scope, perhaps add a `use` for it:
|
11 | use inner::Bar;
|

error: aborting due to previous error

0 comments on commit 45594d5

Please sign in to comment.