Skip to content

Add structured suggestions for various "use" suggestions #46035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -3539,6 +3554,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 @@ -3588,14 +3604,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