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

suggest candidates for unresolved import #102876

Merged
merged 1 commit into from
Oct 11, 2022
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
38 changes: 36 additions & 2 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl TypoSuggestion {
}

/// A free importable items suggested in case of resolution failure.
#[derive(Debug, Clone)]
pub(crate) struct ImportSuggestion {
pub did: Option<DefId>,
pub descr: &'static str,
Expand Down Expand Up @@ -139,6 +140,7 @@ impl<'a> Resolver<'a> {
if instead { Instead::Yes } else { Instead::No },
found_use,
IsPattern::No,
IsImport::No,
path,
);
err.emit();
Expand Down Expand Up @@ -698,6 +700,7 @@ impl<'a> Resolver<'a> {
Instead::No,
FoundUse::Yes,
IsPattern::Yes,
IsImport::No,
vec![],
);
}
Expand Down Expand Up @@ -1481,6 +1484,7 @@ impl<'a> Resolver<'a> {
Instead::No,
FoundUse::Yes,
IsPattern::No,
IsImport::No,
vec![],
);

Expand Down Expand Up @@ -2449,6 +2453,34 @@ enum IsPattern {
No,
}

/// Whether a binding is part of a use statement. Used for diagnostics.
enum IsImport {
Yes,
No,
}

pub(crate) fn import_candidates(
session: &Session,
source_span: &IndexVec<LocalDefId, Span>,
err: &mut Diagnostic,
// This is `None` if all placement locations are inside expansions
use_placement_span: Option<Span>,
candidates: &[ImportSuggestion],
) {
show_candidates(
session,
source_span,
err,
use_placement_span,
candidates,
Instead::Yes,
FoundUse::Yes,
IsPattern::No,
IsImport::Yes,
Comment on lines +2478 to +2479
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these mutually exclusive? If so, they probably should be unified into one enum

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two parameters cannot be Yes at the same time, but can be No at the same time, or one of them is Yes, so if they are unified into one enumeration, at least three variants are required.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I meant -- the Yes+Yes state being invalid would be better represented as an enum.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I think we can rename IsPattern enum to Mode and add a new variant in it. Since this PR is already in a rollup, should I just modify this PR or submit a new one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A follow-up is fine.

vec![],
);
}

/// When an entity with a given name is not available in scope, we search for
/// entities with that name in all crates. This method allows outputting the
/// results of this search in a programmer-friendly way
Expand All @@ -2462,6 +2494,7 @@ fn show_candidates(
instead: Instead,
found_use: FoundUse,
is_pattern: IsPattern,
is_import: IsImport,
path: Vec<Segment>,
) {
if candidates.is_empty() {
Expand Down Expand Up @@ -2521,7 +2554,8 @@ fn show_candidates(
// produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if let FoundUse::Yes = found_use { "" } else { "\n" };
candidate.0 = format!("use {};\n{}", &candidate.0, additional_newline);
let add_use = if let IsImport::Yes = is_import { "" } else { "use " };
candidate.0 = format!("{}{};\n{}", add_use, &candidate.0, additional_newline);
}

err.span_suggestions(
Expand Down Expand Up @@ -2551,7 +2585,7 @@ fn show_candidates(

err.note(&msg);
}
} else {
} else if matches!(is_import, IsImport::No) {
assert!(!inaccessible_path_strings.is_empty());

let prefix =
Expand Down
28 changes: 26 additions & 2 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! A bunch of methods and structures more or less related to resolving imports.

use crate::diagnostics::Suggestion;
use crate::diagnostics::{import_candidates, Suggestion};
use crate::Determinacy::{self, *};
use crate::Namespace::{self, *};
use crate::{module_to_string, names_to_string};
use crate::{module_to_string, names_to_string, ImportSuggestion};
use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
use crate::{NameBinding, NameBindingKind, PathResult};
Expand Down Expand Up @@ -406,6 +406,7 @@ struct UnresolvedImportError {
label: Option<String>,
note: Option<String>,
suggestion: Option<Suggestion>,
candidate: Option<Vec<ImportSuggestion>>,
}

pub struct ImportResolver<'a, 'b> {
Expand Down Expand Up @@ -497,6 +498,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
label: None,
note: None,
suggestion: None,
candidate: None,
};
if path.contains("::") {
errors.push((path, err))
Expand Down Expand Up @@ -547,6 +549,16 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
}
diag.multipart_suggestion(&msg, suggestions, applicability);
}

if let Some(candidate) = &err.candidate {
import_candidates(
self.r.session,
&self.r.source_span,
&mut diag,
Some(err.span),
&candidate,
)
}
}

diag.emit();
Expand Down Expand Up @@ -664,6 +676,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
Some(finalize),
ignore_binding,
);

let no_ambiguity = self.r.ambiguity_errors.len() == prev_ambiguity_errors_len;
import.vis.set(orig_vis);
let module = match path_res {
Expand Down Expand Up @@ -706,12 +719,14 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
String::from("a similar path exists"),
Applicability::MaybeIncorrect,
)),
candidate: None,
},
None => UnresolvedImportError {
span,
label: Some(label),
note: None,
suggestion,
candidate: None,
},
};
return Some(err);
Expand Down Expand Up @@ -754,6 +769,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
label: Some(String::from("cannot glob-import a module into itself")),
note: None,
suggestion: None,
candidate: None,
});
}
}
Expand Down Expand Up @@ -919,11 +935,19 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
}
};

let parent_suggestion =
self.r.lookup_import_candidates(ident, TypeNS, &import.parent_scope, |_| true);

Some(UnresolvedImportError {
span: import.span,
label: Some(label),
note,
suggestion,
candidate: if !parent_suggestion.is_empty() {
Some(parent_suggestion)
} else {
None
},
})
} else {
// `resolve_ident_in_module` reported a privacy error.
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/extenv/issue-55897.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ error[E0432]: unresolved import `env`
|
LL | use env;
| ^^^ no `env` in the root
|
help: consider importing this module instead
|
LL | use std::env;
| ~~~~~~~~~

error: cannot determine resolution for the macro `env`
--> $DIR/issue-55897.rs:6:22
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/imports/issue-56125.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,18 @@ error[E0432]: unresolved import `empty::issue_56125`
|
LL | use empty::issue_56125;
| ^^^^^^^^^^^^^^^^^^ no `issue_56125` in `m3::empty`
|
help: consider importing one of these items instead
|
LL | use crate::m3::last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use crate::m3::non_last_segment::non_last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use issue_56125::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use issue_56125::last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
and 1 other candidate

error[E0659]: `issue_56125` is ambiguous
--> $DIR/issue-56125.rs:6:9
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/imports/issue-57015.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0432]: unresolved import `single_err::something`
|
LL | use single_err::something;
| ^^^^^^^^^^^^^^^^^^^^^ no `something` in `single_err`
|
help: consider importing this module instead
|
LL | use glob_ok::something;
| ~~~~~~~~~~~~~~~~~~~

error: aborting due to previous error

Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/rfc-2126-extern-absolute-paths/not-allowed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ error[E0432]: unresolved import `alloc`
|
LL | use alloc;
| ^^^^^ no external crate `alloc`
|
help: consider importing one of these items instead
|
LL | use core::alloc;
| ~~~~~~~~~~~~
LL | use std::alloc;
| ~~~~~~~~~~~

error: aborting due to previous error

Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/simd/portable-intrinsics-arent-exposed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ error[E0432]: unresolved import `std::simd::intrinsics`
|
LL | use std::simd::intrinsics;
| ^^^^^^^^^^^^^^^^^^^^^ no `intrinsics` in `simd`
|
help: consider importing this module instead
|
LL | use std::intrinsics;
| ~~~~~~~~~~~~~~~~

error: aborting due to 2 previous errors

Expand Down
14 changes: 10 additions & 4 deletions src/test/ui/test-attrs/inaccessible-test-modules.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,16 @@ error[E0432]: unresolved import `test`
--> $DIR/inaccessible-test-modules.rs:6:5
|
LL | use test as y;
| ----^^^^^
| |
| no `test` in the root
| help: a similar name exists in the module: `test`
| ^^^^^^^^^ no `test` in the root
|
help: a similar name exists in the module
|
LL | use test as y;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this suggesting? Turning test into test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the original problem of the test module and not a problem caused by this PR

| ~~~~
help: consider importing this module instead
|
LL | use test::test;
| ~~~~~~~~~~~

error: aborting due to 2 previous errors

Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/unresolved/unresolved-candidates.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
mod a {
pub trait Trait {}
}

mod b {
use Trait; //~ ERROR unresolved import `Trait`
}

mod c {
impl Trait for () {} //~ ERROR cannot find trait `Trait` in this scope
}

fn main() {}
26 changes: 26 additions & 0 deletions src/test/ui/unresolved/unresolved-candidates.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0432]: unresolved import `Trait`
--> $DIR/unresolved-candidates.rs:6:9
|
LL | use Trait;
| ^^^^^ no `Trait` in the root
|
help: consider importing this trait instead
|
LL | use a::Trait;
| ~~~~~~~~~

error[E0405]: cannot find trait `Trait` in this scope
--> $DIR/unresolved-candidates.rs:10:10
|
LL | impl Trait for () {}
| ^^^^^ not found in this scope
|
help: consider importing this trait
|
LL | use a::Trait;
|

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0405, E0432.
For more information about an error, try `rustc --explain E0405`.