Skip to content
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

Do not suggest importing inaccessible items #88838

Merged
merged 2 commits into from
Oct 1, 2021
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
140 changes: 109 additions & 31 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,15 @@ impl<'a> Resolver<'a> {

let import_suggestions =
self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
show_candidates(err, None, &import_suggestions, false, true);
show_candidates(
&self.definitions,
self.session,
err,
None,
&import_suggestions,
false,
true,
);

if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {
let msg = format!("unsafe traits like `{}` should be implemented explicitly", ident);
Expand Down Expand Up @@ -1689,6 +1697,8 @@ fn find_span_immediately_after_crate_name(
/// entities with that name in all crates. This method allows outputting the
/// results of this search in a programmer-friendly way
crate fn show_candidates(
definitions: &rustc_hir::definitions::Definitions,
session: &Session,
err: &mut DiagnosticBuilder<'_>,
// This is `None` if all placement locations are inside expansions
use_placement_span: Option<Span>,
Expand All @@ -1700,43 +1710,111 @@ crate fn show_candidates(
return;
}

let mut accessible_path_strings: Vec<(String, &str, Option<DefId>)> = Vec::new();
let mut inaccessible_path_strings: Vec<(String, &str, Option<DefId>)> = Vec::new();

candidates.iter().for_each(|c| {
(if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
.push((path_names_to_string(&c.path), c.descr, c.did))
});

// we want consistent results across executions, but candidates are produced
// by iterating through a hash map, so make sure they are ordered:
let mut path_strings: Vec<_> =
candidates.iter().map(|c| path_names_to_string(&c.path)).collect();
for path_strings in [&mut accessible_path_strings, &mut inaccessible_path_strings] {
path_strings.sort_by(|a, b| a.0.cmp(&b.0));
let core_path_strings =
path_strings.drain_filter(|p| p.0.starts_with("core::")).collect::<Vec<_>>();
path_strings.extend(core_path_strings);
path_strings.dedup_by(|a, b| a.0 == b.0);
}

if !accessible_path_strings.is_empty() {
let (determiner, kind) = if accessible_path_strings.len() == 1 {
("this", accessible_path_strings[0].1)
} else {
("one of these", "items")
};

path_strings.sort();
let core_path_strings =
path_strings.drain_filter(|p| p.starts_with("core::")).collect::<Vec<String>>();
path_strings.extend(core_path_strings);
path_strings.dedup();
let instead = if instead { " instead" } else { "" };
let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);

let (determiner, kind) = if candidates.len() == 1 {
("this", candidates[0].descr)
} else {
("one of these", "items")
};

let instead = if instead { " instead" } else { "" };
let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);

if let Some(span) = use_placement_span {
for candidate in &mut path_strings {
// 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);
}
if let Some(span) = use_placement_span {
for candidate in &mut accessible_path_strings {
// produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use { "" } else { "\n" };
candidate.0 = format!("use {};\n{}", &candidate.0, additional_newline);
}

err.span_suggestions(span, &msg, path_strings.into_iter(), Applicability::Unspecified);
} else {
msg.push(':');
err.span_suggestions(
span,
&msg,
accessible_path_strings.into_iter().map(|a| a.0),
Applicability::Unspecified,
);
} else {
msg.push(':');

for candidate in path_strings {
msg.push('\n');
msg.push_str(&candidate);
for candidate in accessible_path_strings {
msg.push('\n');
msg.push_str(&candidate.0);
}

err.note(&msg);
}
} else {
assert!(!inaccessible_path_strings.is_empty());

if inaccessible_path_strings.len() == 1 {
let (name, descr, def_id) = &inaccessible_path_strings[0];
let msg = format!("{} `{}` exists but is inaccessible", descr, name);

if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
let span = definitions.def_span(local_def_id);
let span = session.source_map().guess_head_span(span);
let mut multi_span = MultiSpan::from_span(span);
multi_span.push_span_label(span, "not accessible".to_string());
err.span_note(multi_span, &msg);
} else {
err.note(&msg);
}
} else {
let (_, descr_first, _) = &inaccessible_path_strings[0];
let descr = if inaccessible_path_strings
.iter()
.skip(1)
.all(|(_, descr, _)| descr == descr_first)
{
format!("{}", descr_first)
} else {
"item".to_string()
};

let mut msg = format!("these {}s exist but are inaccessible", descr);
let mut has_colon = false;

err.note(&msg);
let mut spans = Vec::new();
for (name, _, def_id) in &inaccessible_path_strings {
if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
let span = definitions.def_span(local_def_id);
let span = session.source_map().guess_head_span(span);
spans.push((name, span));
} else {
if !has_colon {
msg.push(':');
has_colon = true;
}
msg.push('\n');
msg.push_str(name);
}
}

let mut multi_span = MultiSpan::from_spans(spans.iter().map(|(_, sp)| *sp).collect());
for (name, span) in spans {
multi_span.push_span_label(span, format!("`{}`: not accessible", name));
}

err.span_note(multi_span, &msg);
}
}
}
10 changes: 9 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2966,7 +2966,15 @@ impl<'a> Resolver<'a> {
(None, false)
};
if !candidates.is_empty() {
diagnostics::show_candidates(&mut err, span, &candidates, instead, found_use);
diagnostics::show_candidates(
&self.definitions,
self.session,
&mut err,
span,
&candidates,
instead,
found_use,
);
} else if let Some((span, msg, sugg, appl)) = suggestion {
err.span_suggestion(span, msg, sugg, appl);
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/hygiene/globs.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0425]: cannot find function `f` in this scope
LL | f();
| ^ not found in this scope
|
help: consider importing one of these items
help: consider importing this function
|
LL | use foo::f;
|
Expand Down Expand Up @@ -37,7 +37,7 @@ LL | n!(f);
LL | n!(f);
| ^ not found in this scope
|
= note: consider importing one of these items:
= note: consider importing this function:
foo::f
= note: this error originates in the macro `n` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand All @@ -50,7 +50,7 @@ LL | n!(f);
LL | f
| ^ not found in this scope
|
= note: consider importing one of these items:
= note: consider importing this function:
foo::f
= note: this error originates in the macro `n` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand Down
60 changes: 27 additions & 33 deletions src/test/ui/imports/glob-resolve1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,23 @@ error[E0425]: cannot find function `fpriv` in this scope
LL | fpriv();
| ^^^^^ not found in this scope
|
help: consider importing this function
|
LL | use bar::fpriv;
note: function `bar::fpriv` exists but is inaccessible
--> $DIR/glob-resolve1.rs:7:5
|
LL | fn fpriv() {}
| ^^^^^^^^^^ not accessible

error[E0425]: cannot find function `epriv` in this scope
--> $DIR/glob-resolve1.rs:27:5
|
LL | epriv();
| ^^^^^ not found in this scope
|
help: consider importing this function
|
LL | use bar::epriv;
note: function `bar::epriv` exists but is inaccessible
--> $DIR/glob-resolve1.rs:9:9
|
LL | fn epriv();
| ^^^^^^^^^^^ not accessible

error[E0423]: expected value, found enum `B`
--> $DIR/glob-resolve1.rs:28:5
Expand All @@ -44,10 +46,11 @@ error[E0425]: cannot find value `C` in this scope
LL | C;
| ^ not found in this scope
|
help: consider importing this unit struct
|
LL | use bar::C;
note: unit struct `bar::C` exists but is inaccessible
--> $DIR/glob-resolve1.rs:18:5
|
LL | struct C;
| ^^^^^^^^^ not accessible

error[E0425]: cannot find function `import` in this scope
--> $DIR/glob-resolve1.rs:30:5
Expand All @@ -67,16 +70,13 @@ LL | pub enum B {
| ---------- similarly named enum `B` defined here
...
LL | foo::<A>();
| ^
|
help: an enum with a similar name exists
| ^ help: an enum with a similar name exists: `B`
|
LL | foo::<B>();
| ~
help: consider importing this enum
|
LL | use bar::A;
note: enum `bar::A` exists but is inaccessible
--> $DIR/glob-resolve1.rs:11:5
|
LL | enum A {
| ^^^^^^ not accessible

error[E0412]: cannot find type `C` in this scope
--> $DIR/glob-resolve1.rs:33:11
Expand All @@ -85,16 +85,13 @@ LL | pub enum B {
| ---------- similarly named enum `B` defined here
...
LL | foo::<C>();
| ^
|
help: an enum with a similar name exists
|
LL | foo::<B>();
| ~
help: consider importing this struct
| ^ help: an enum with a similar name exists: `B`
|
LL | use bar::C;
note: struct `bar::C` exists but is inaccessible
--> $DIR/glob-resolve1.rs:18:5
|
LL | struct C;
| ^^^^^^^^^ not accessible

error[E0412]: cannot find type `D` in this scope
--> $DIR/glob-resolve1.rs:34:11
Expand All @@ -103,16 +100,13 @@ LL | pub enum B {
| ---------- similarly named enum `B` defined here
...
LL | foo::<D>();
| ^
|
help: an enum with a similar name exists
|
LL | foo::<B>();
| ~
help: consider importing this type alias
| ^ help: an enum with a similar name exists: `B`
|
LL | use bar::D;
note: type alias `bar::D` exists but is inaccessible
--> $DIR/glob-resolve1.rs:20:5
|
LL | type D = isize;
| ^^^^^^^^^^^^^^^ not accessible

error: aborting due to 8 previous errors

Expand Down
7 changes: 4 additions & 3 deletions src/test/ui/imports/issue-4366-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ error[E0412]: cannot find type `Bar` in this scope
LL | fn sub() -> Bar { 1 }
| ^^^ not found in this scope
|
help: consider importing this type alias
|
LL | use a::b::Bar;
note: type alias `a::b::Bar` exists but is inaccessible
--> $DIR/issue-4366-2.rs:11:9
|
LL | type Bar = isize;
| ^^^^^^^^^^^^^^^^^ not accessible

error[E0423]: expected function, found module `foo`
--> $DIR/issue-4366-2.rs:25:5
Expand Down
7 changes: 4 additions & 3 deletions src/test/ui/resolve/issue-42944.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ error[E0425]: cannot find function, tuple struct or tuple variant `Bx` in this s
LL | Bx(());
| ^^ not found in this scope
|
help: consider importing this tuple struct
|
LL | use foo::Bx;
note: tuple struct `foo::Bx` exists but is inaccessible
--> $DIR/issue-42944.rs:2:5
|
LL | pub struct Bx(());
| ^^^^^^^^^^^^^^^^^^ not accessible

error: aborting due to 2 previous errors

Expand Down
38 changes: 38 additions & 0 deletions src/test/ui/resolve/issue-88472.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Regression test for #88472, where a suggestion was issued to
// import an inaccessible struct.

#![warn(unused_imports)]
//~^ NOTE: the lint level is defined here

mod a {
struct Foo;
//~^ NOTE: struct `a::Foo` exists but is inaccessible
//~| NOTE: not accessible
}

mod b {
use crate::a::*;
//~^ WARNING: unused import
type Bar = Foo;
//~^ ERROR: cannot find type `Foo` in this scope [E0412]
//~| NOTE: not found in this scope
}

mod c {
enum Eee {}
//~^ NOTE: these enums exist but are inaccessible
//~| NOTE: `c::Eee`: not accessible

mod d {
enum Eee {}
//~^ NOTE: `c::d::Eee`: not accessible
}
}

mod e {
type Baz = Eee;
//~^ ERROR: cannot find type `Eee` in this scope [E0412]
//~| NOTE: not found in this scope
}

fn main() {}
Loading