Skip to content

Commit

Permalink
Only suggest imports if not imported.
Browse files Browse the repository at this point in the history
This commit modifies name resolution error reporting so that if a name
is in scope and has been imported then we do not suggest importing it.

This can occur when we add a label about constructors not being visible
due to private fields. In these cases, we know that the struct/variant
has been imported and we should silence any suggestions to import the
struct/variant.
  • Loading branch information
davidtwco committed Feb 11, 2019
1 parent de111e6 commit 48b0c9d
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 17 deletions.
10 changes: 9 additions & 1 deletion src/librustc_resolve/error_reporting.rs
Expand Up @@ -106,7 +106,15 @@ impl<'a> Resolver<'a> {

// Try to lookup name in more relaxed fashion for better error reporting.
let ident = path.last().unwrap().ident;
let candidates = self.lookup_import_candidates(ident, ns, is_expected);
let candidates = self.lookup_import_candidates(ident, ns, is_expected)
.drain(..)
.filter(|ImportSuggestion { did, .. }| {
match (did, def.and_then(|def| def.opt_def_id())) {
(Some(suggestion_did), Some(actual_did)) => *suggestion_did != actual_did,
_ => true,
}
})
.collect::<Vec<_>>();
if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) {
let enum_candidates =
self.lookup_import_candidates(ident, ns, is_enum_variant);
Expand Down
16 changes: 12 additions & 4 deletions src/librustc_resolve/lib.rs
Expand Up @@ -25,7 +25,7 @@ use rustc::hir::def::*;
use rustc::hir::def::Namespace::*;
use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId};
use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
use rustc::ty;
use rustc::ty::{self, DefIdTree};
use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap};
use rustc::{bug, span_bug};

Expand Down Expand Up @@ -93,6 +93,7 @@ enum ScopeSet {

/// A free importable items suggested in case of resolution failure.
struct ImportSuggestion {
did: Option<DefId>,
path: Path,
}

Expand Down Expand Up @@ -4392,7 +4393,8 @@ impl<'a> Resolver<'a> {

// collect results based on the filter function
if ident.name == lookup_ident.name && ns == namespace {
if filter_fn(name_binding.def()) {
let def = name_binding.def();
if filter_fn(def) {
// create the path
let mut segms = path_segments.clone();
if lookup_ident.span.rust_2018() {
Expand All @@ -4416,7 +4418,12 @@ impl<'a> Resolver<'a> {
// declared as public (due to pruning, we don't explore
// outside crate private modules => no need to check this)
if !in_module_is_extern || name_binding.vis == ty::Visibility::Public {
candidates.push(ImportSuggestion { path });
let did = match def {
Def::StructCtor(did, _) | Def::VariantCtor(did, _) =>
self.parent(did),
_ => def.opt_def_id(),
};
candidates.push(ImportSuggestion { did, path });
}
}
}
Expand Down Expand Up @@ -4513,7 +4520,8 @@ impl<'a> Resolver<'a> {
span: name_binding.span,
segments: path_segments,
};
result = Some((module, ImportSuggestion { path }));
let did = module.def().and_then(|def| def.opt_def_id());
result = Some((module, ImportSuggestion { did, path }));
} else {
// add the module to the lookup
if seen_modules.insert(module.def_id().unwrap()) {
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/issue-42944.rs
@@ -0,0 +1,19 @@
mod foo {
pub struct B(());
}

mod bar {
use foo::B;

fn foo() {
B(()); //~ ERROR expected function, found struct `B` [E0423]
}
}

mod baz {
fn foo() {
B(()); //~ ERROR cannot find function `B` in this scope [E0425]
}
}

fn main() {}
20 changes: 20 additions & 0 deletions src/test/ui/issue-42944.stderr
@@ -0,0 +1,20 @@
error[E0423]: expected function, found struct `B`
--> $DIR/issue-42944.rs:9:9
|
LL | B(()); //~ ERROR expected function, found struct `B` [E0423]
| ^ constructor is not visible here due to private fields

error[E0425]: cannot find function `B` in this scope
--> $DIR/issue-42944.rs:15:9
|
LL | B(()); //~ ERROR cannot find function `B` in this scope [E0425]
| ^ not found in this scope
help: possible candidate is found in another module, you can import it into scope
|
LL | use foo::B;
|

error: aborting due to 2 previous errors

Some errors occurred: E0423, E0425.
For more information about an error, try `rustc --explain E0423`.
15 changes: 3 additions & 12 deletions src/test/ui/resolve/privacy-struct-ctor.stderr
Expand Up @@ -2,25 +2,16 @@ error[E0423]: expected value, found struct `Z`
--> $DIR/privacy-struct-ctor.rs:20:9
|
LL | Z;
| ^ constructor is not visible here due to private fields
help: a tuple struct with a similar name exists
|
LL | S;
| ^
help: possible better candidate is found in another module, you can import it into scope
|
LL | use m::n::Z;
|
| |
| constructor is not visible here due to private fields
| help: a tuple struct with a similar name exists: `S`

error[E0423]: expected value, found struct `S`
--> $DIR/privacy-struct-ctor.rs:33:5
|
LL | S;
| ^ constructor is not visible here due to private fields
help: possible better candidate is found in another module, you can import it into scope
|
LL | use m::S;
|

error[E0423]: expected value, found struct `S2`
--> $DIR/privacy-struct-ctor.rs:38:5
Expand Down

0 comments on commit 48b0c9d

Please sign in to comment.