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

Account for hygiene in typo suggestions, and use them to point to shadowed names #103111

Merged
merged 2 commits into from
Oct 21, 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
13 changes: 8 additions & 5 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use rustc_span::hygiene::MacroKind;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, Span};
use rustc_span::{BytePos, Span, SyntaxContext};

use crate::imports::{Import, ImportKind, ImportResolver};
use crate::late::{PatternSource, Rib};
Expand All @@ -47,13 +47,15 @@ pub(crate) type Suggestion = (Vec<(Span, String)>, String, Applicability);
/// similarly named label and whether or not it is reachable.
pub(crate) type LabelSuggestion = (Ident, bool);

#[derive(Debug)]
pub(crate) enum SuggestionTarget {
/// The target has a similar name as the name used by the programmer (probably a typo)
SimilarlyNamed,
/// The target is the only valid item that can be used in the corresponding context
SingleItem,
}

#[derive(Debug)]
pub(crate) struct TypoSuggestion {
pub candidate: Symbol,
pub res: Res,
Expand Down Expand Up @@ -482,11 +484,12 @@ impl<'a> Resolver<'a> {
module: Module<'a>,
names: &mut Vec<TypoSuggestion>,
filter_fn: &impl Fn(Res) -> bool,
ctxt: Option<SyntaxContext>,
) {
for (key, resolution) in self.resolutions(module).borrow().iter() {
if let Some(binding) = resolution.borrow().binding {
let res = binding.res();
if filter_fn(res) {
if filter_fn(res) && ctxt.map_or(true, |ctxt| ctxt == key.ident.span.ctxt()) {
names.push(TypoSuggestion::typo_from_res(key.ident.name, res));
}
}
Expand Down Expand Up @@ -1181,10 +1184,10 @@ impl<'a> Resolver<'a> {
Scope::CrateRoot => {
let root_ident = Ident::new(kw::PathRoot, ident.span);
let root_module = this.resolve_crate_root(root_ident);
this.add_module_candidates(root_module, &mut suggestions, filter_fn);
this.add_module_candidates(root_module, &mut suggestions, filter_fn, None);
}
Scope::Module(module, _) => {
this.add_module_candidates(module, &mut suggestions, filter_fn);
this.add_module_candidates(module, &mut suggestions, filter_fn, None);
}
Scope::MacroUsePrelude => {
suggestions.extend(this.macro_use_prelude.iter().filter_map(
Expand Down Expand Up @@ -1221,7 +1224,7 @@ impl<'a> Resolver<'a> {
Scope::StdLibPrelude => {
if let Some(prelude) = this.prelude {
let mut tmp_suggestions = Vec::new();
this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn);
this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn, None);
suggestions.extend(
tmp_suggestions
.into_iter()
Expand Down
70 changes: 61 additions & 9 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ pub(super) enum LifetimeElisionCandidate {
}

/// Only used for diagnostics.
#[derive(Debug)]
struct BaseError {
msg: String,
fallback_label: String,
Expand All @@ -140,6 +141,22 @@ struct BaseError {
suggestion: Option<(Span, &'static str, String)>,
}

#[derive(Debug)]
enum TypoCandidate {
Typo(TypoSuggestion),
Shadowed(Res),
None,
}

impl TypoCandidate {
fn to_opt_suggestion(self) -> Option<TypoSuggestion> {
match self {
TypoCandidate::Typo(sugg) => Some(sugg),
TypoCandidate::Shadowed(_) | TypoCandidate::None => None,
}
}
}

impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
fn def_span(&self, def_id: DefId) -> Option<Span> {
match def_id.krate {
Expand Down Expand Up @@ -496,7 +513,8 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
}

// Try Levenshtein algorithm.
let typo_sugg = self.lookup_typo_candidate(path, source.namespace(), is_expected);
let typo_sugg =
self.lookup_typo_candidate(path, source.namespace(), is_expected).to_opt_suggestion();
if path.len() == 1 && self.self_type_is_available() {
if let Some(candidate) = self.lookup_assoc_candidate(ident, ns, is_expected) {
let self_is_available = self.self_value_is_available(path[0].ident.span);
Expand Down Expand Up @@ -660,7 +678,18 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
let is_expected = &|res| source.is_expected(res);
let ident_span = path.last().map_or(span, |ident| ident.ident.span);
let typo_sugg = self.lookup_typo_candidate(path, source.namespace(), is_expected);
if let TypoCandidate::Shadowed(res) = typo_sugg
&& let Some(id) = res.opt_def_id()
&& let Some(sugg_span) = self.r.opt_span(id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be opt_def_span? If not, r=me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather not. Using that would suggest imported names from other crates too. The aim is rather to suggest a shadowed name from the local crate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, I just would rather we pointed at the ident for the item and not the whole thing.

{
err.span_label(
sugg_span,
format!("you might have meant to refer to this {}", res.descr()),
);
return true;
}
let mut fallback = false;
let typo_sugg = typo_sugg.to_opt_suggestion();
if !self.r.add_typo_suggestion(err, typo_sugg, ident_span) {
fallback = true;
match self.diagnostic_metadata.current_let_binding {
Expand Down Expand Up @@ -1581,22 +1610,38 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
path: &[Segment],
ns: Namespace,
filter_fn: &impl Fn(Res) -> bool,
) -> Option<TypoSuggestion> {
) -> TypoCandidate {
let mut names = Vec::new();
if path.len() == 1 {
let mut ctxt = path.last().unwrap().ident.span.ctxt();

// Search in lexical scope.
// Walk backwards up the ribs in scope and collect candidates.
for rib in self.ribs[ns].iter().rev() {
let rib_ctxt = if rib.kind.contains_params() {
ctxt.normalize_to_macros_2_0()
} else {
ctxt.normalize_to_macro_rules()
};

// Locals and type parameters
for (ident, &res) in &rib.bindings {
if filter_fn(res) {
if filter_fn(res) && ident.span.ctxt() == rib_ctxt {
names.push(TypoSuggestion::typo_from_res(ident.name, res));
}
}

if let RibKind::MacroDefinition(def) = rib.kind && def == self.r.macro_def(ctxt) {
// If an invocation of this macro created `ident`, give up on `ident`
// and switch to `ident`'s source from the macro definition.
ctxt.remove_mark();
continue;
}

// Items in scope
if let RibKind::ModuleRibKind(module) = rib.kind {
// Items from this module
self.r.add_module_candidates(module, &mut names, &filter_fn);
self.r.add_module_candidates(module, &mut names, &filter_fn, Some(ctxt));

if let ModuleKind::Block = module.kind {
// We can see through blocks
Expand All @@ -1622,7 +1667,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
}));

if let Some(prelude) = self.r.prelude {
self.r.add_module_candidates(prelude, &mut names, &filter_fn);
self.r.add_module_candidates(prelude, &mut names, &filter_fn, None);
}
}
break;
Expand All @@ -1641,7 +1686,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
if let PathResult::Module(ModuleOrUniformRoot::Module(module)) =
self.resolve_path(mod_path, Some(TypeNS), None)
{
self.r.add_module_candidates(module, &mut names, &filter_fn);
self.r.add_module_candidates(module, &mut names, &filter_fn, None);
}
}

Expand All @@ -1654,10 +1699,17 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
name,
None,
) {
Some(found) if found != name => {
names.into_iter().find(|suggestion| suggestion.candidate == found)
Some(found) => {
let Some(sugg) = names.into_iter().find(|suggestion| suggestion.candidate == found) else {
return TypoCandidate::None;
};
if found == name {
TypoCandidate::Shadowed(sugg.res)
} else {
TypoCandidate::Typo(sugg)
}
}
_ => None,
_ => TypoCandidate::None,
}
}

Expand Down
18 changes: 16 additions & 2 deletions src/test/ui/hygiene/globs.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
error[E0425]: cannot find function `f` in this scope
--> $DIR/globs.rs:22:9
|
LL | pub fn g() {}
| ---------- similarly named function `g` defined here
...
LL | f();
| ^ not found in this scope
| ^
|
help: a function with a similar name exists
|
LL | g();
| ~
help: consider importing this function
|
LL | use foo::f;
Expand All @@ -12,8 +19,11 @@ LL | use foo::f;
error[E0425]: cannot find function `g` in this scope
--> $DIR/globs.rs:15:5
|
LL | pub fn f() {}
| ---------- similarly named function `f` defined here
...
LL | g();
| ^ not found in this scope
| ^
...
LL | / m! {
LL | | use bar::*;
Expand All @@ -23,6 +33,10 @@ LL | | }
| |_____- in this macro invocation
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
help: a function with a similar name exists
|
LL | f();
| ~
help: consider importing this function
|
LL | use bar::g;
Expand Down
8 changes: 1 addition & 7 deletions src/test/ui/hygiene/rustc-macro-transparency.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,8 @@ LL | semitransparent;
error[E0423]: expected value, found macro `opaque`
--> $DIR/rustc-macro-transparency.rs:30:5
|
LL | struct Opaque;
| -------------- similarly named unit struct `Opaque` defined here
...
LL | opaque;
| ^^^^^^
| |
| not a value
| help: a unit struct with a similar name exists (notice the capitalization): `Opaque`
| ^^^^^^ not a value

error: aborting due to 3 previous errors

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/lexical-scopes.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
error[E0574]: expected struct, variant or union type, found type parameter `T`
--> $DIR/lexical-scopes.rs:3:13
|
LL | struct T { i: i32 }
| ------------------- you might have meant to refer to this struct
LL | fn f<T>() {
| - found this type parameter
LL | let t = T { i: 0 };
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/macros/macro-context.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ error[E0425]: cannot find value `i` in this scope
--> $DIR/macro-context.rs:3:13
|
LL | () => ( i ; typeof );
| ^ help: a local variable with a similar name exists: `a`
| ^ not found in this scope
...
LL | let i = m!();
| ---- in this macro invocation
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/proc-macro/gen-macro-rules-hygiene.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ error[E0425]: cannot find value `local_use` in this scope
--> $DIR/gen-macro-rules-hygiene.rs:12:1
|
LL | gen_macro_rules!();
| ^^^^^^^^^^^^^^^^^^ not found in this scope
| ^^^^^^^^^^^^^^^^^^ help: a local variable with a similar name exists: `local_def`
...
LL | generated!();
| ------------ in this macro invocation
Expand All @@ -24,7 +24,7 @@ error[E0425]: cannot find value `local_def` in this scope
--> $DIR/gen-macro-rules-hygiene.rs:21:9
|
LL | local_def;
| ^^^^^^^^^ not found in this scope
| ^^^^^^^^^ help: a local variable with a similar name exists: `local_use`

error: aborting due to 3 previous errors

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/proc-macro/mixed-site-span.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ error[E0425]: cannot find value `local_use` in this scope
--> $DIR/mixed-site-span.rs:13:9
|
LL | proc_macro_rules!();
| ^^^^^^^^^^^^^^^^^^^ not found in this scope
| ^^^^^^^^^^^^^^^^^^^ help: a local variable with a similar name exists: `local_def`
|
= note: this error originates in the macro `proc_macro_rules` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0425]: cannot find value `local_def` in this scope
--> $DIR/mixed-site-span.rs:17:9
|
LL | local_def;
| ^^^^^^^^^ not found in this scope
| ^^^^^^^^^ help: a local variable with a similar name exists: `local_use`

error[E0412]: cannot find type `ItemUse` in crate `$crate`
--> $DIR/mixed-site-span.rs:24:1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
error[E0574]: expected struct, variant or union type, found type parameter `Baz`
--> $DIR/point-at-type-parameter-shadowing-another-type.rs:16:13
|
LL | impl<Baz> Foo<Baz> for Bar {
| --- found this type parameter
LL | / struct Baz {
LL | | num: usize,
LL | | }
| |_- you might have meant to refer to this struct
LL |
LL | impl<Baz> Foo<Baz> for Bar {
| --- found this type parameter
...
LL | Baz { num } => num,
| ^^^ not a struct, variant or union type
LL | Baz { num } => num,
| ^^^ not a struct, variant or union type

error: aborting due to previous error

Expand Down