Skip to content

Commit

Permalink
Rollup merge of rust-lang#57477 - euclio:clarify-lev-suggestion, r=za…
Browse files Browse the repository at this point in the history
…ckmdavis

clarify resolve typo suggestion

Include the kind of the binding that we're suggesting, and use a
structured suggestion.

Fixes rust-lang#53445.
  • Loading branch information
Centril committed Jan 14, 2019
2 parents 44a41b4 + 404ad50 commit 731f1d8
Show file tree
Hide file tree
Showing 40 changed files with 249 additions and 125 deletions.
3 changes: 2 additions & 1 deletion src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ impl Def {
}
}

/// A human readable kind name
/// A human readable name for the def kind ("function", "module", etc.).
pub fn kind_name(&self) -> &'static str {
match *self {
Def::Fn(..) => "function",
Expand Down Expand Up @@ -324,6 +324,7 @@ impl Def {
}
}

/// An English article for the def.
pub fn article(&self) -> &'static str {
match *self {
Def::AssociatedTy(..) | Def::AssociatedConst(..) | Def::AssociatedExistential(..) |
Expand Down
86 changes: 68 additions & 18 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ struct BindingError {
target: BTreeSet<Span>,
}

struct TypoSuggestion {
candidate: Symbol,

/// The kind of the binding ("crate", "module", etc.)
kind: &'static str,

/// An appropriate article to refer to the binding ("a", "an", etc.)
article: &'static str,
}

impl PartialOrd for BindingError {
fn partial_cmp(&self, other: &BindingError) -> Option<cmp::Ordering> {
Some(self.cmp(other))
Expand Down Expand Up @@ -1448,7 +1458,7 @@ impl PrimitiveTypeTable {
}
}

#[derive(Default, Clone)]
#[derive(Debug, Default, Clone)]
pub struct ExternPreludeEntry<'a> {
extern_crate_item: Option<&'a NameBinding<'a>>,
pub introduced_by_item: bool,
Expand Down Expand Up @@ -3291,8 +3301,19 @@ impl<'a> Resolver<'a> {
let mut levenshtein_worked = false;

// Try Levenshtein algorithm.
if let Some(candidate) = this.lookup_typo_candidate(path, ns, is_expected, span) {
err.span_label(ident_span, format!("did you mean `{}`?", candidate));
let suggestion = this.lookup_typo_candidate(path, ns, is_expected, span);
if let Some(suggestion) = suggestion {
let msg = format!(
"{} {} with a similar name exists",
suggestion.article, suggestion.kind
);
err.span_suggestion_with_applicability(
ident_span,
&msg,
suggestion.candidate.to_string(),
Applicability::MaybeIncorrect,
);

levenshtein_worked = true;
}

Expand Down Expand Up @@ -4183,19 +4204,25 @@ impl<'a> Resolver<'a> {
None
}

fn lookup_typo_candidate<FilterFn>(&mut self,
path: &[Segment],
ns: Namespace,
filter_fn: FilterFn,
span: Span)
-> Option<Symbol>
where FilterFn: Fn(Def) -> bool
fn lookup_typo_candidate<FilterFn>(
&mut self,
path: &[Segment],
ns: Namespace,
filter_fn: FilterFn,
span: Span,
) -> Option<TypoSuggestion>
where
FilterFn: Fn(Def) -> bool,
{
let add_module_candidates = |module: Module, names: &mut Vec<Name>| {
let add_module_candidates = |module: Module, names: &mut Vec<TypoSuggestion>| {
for (&(ident, _), resolution) in module.resolutions.borrow().iter() {
if let Some(binding) = resolution.borrow().binding {
if filter_fn(binding.def()) {
names.push(ident.name);
names.push(TypoSuggestion {
candidate: ident.name,
article: binding.def().article(),
kind: binding.def().kind_name(),
});
}
}
}
Expand All @@ -4209,7 +4236,11 @@ impl<'a> Resolver<'a> {
// Locals and type parameters
for (ident, def) in &rib.bindings {
if filter_fn(*def) {
names.push(ident.name);
names.push(TypoSuggestion {
candidate: ident.name,
article: def.article(),
kind: def.kind_name(),
});
}
}
// Items in scope
Expand All @@ -4222,7 +4253,13 @@ impl<'a> Resolver<'a> {
} else {
// Items from the prelude
if !module.no_implicit_prelude {
names.extend(self.extern_prelude.iter().map(|(ident, _)| ident.name));
names.extend(self.extern_prelude.iter().map(|(ident, _)| {
TypoSuggestion {
candidate: ident.name,
article: "a",
kind: "crate",
}
}));
if let Some(prelude) = self.prelude {
add_module_candidates(prelude, &mut names);
}
Expand All @@ -4234,7 +4271,13 @@ impl<'a> Resolver<'a> {
// Add primitive types to the mix
if filter_fn(Def::PrimTy(Bool)) {
names.extend(
self.primitive_type_table.primitive_types.iter().map(|(name, _)| name)
self.primitive_type_table.primitive_types.iter().map(|(name, _)| {
TypoSuggestion {
candidate: *name,
article: "a",
kind: "primitive type",
}
})
)
}
} else {
Expand All @@ -4251,9 +4294,16 @@ impl<'a> Resolver<'a> {

let name = path[path.len() - 1].ident.name;
// Make sure error reporting is deterministic.
names.sort_by_cached_key(|name| name.as_str());
match find_best_match_for_name(names.iter(), &name.as_str(), None) {
Some(found) if found != name => Some(found),
names.sort_by_cached_key(|suggestion| suggestion.candidate.as_str());

match find_best_match_for_name(
names.iter().map(|suggestion| &suggestion.candidate),
&name.as_str(),
None,
) {
Some(found) if found != name => names
.into_iter()
.find(|suggestion| suggestion.candidate == found),
_ => None,
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,7 @@ impl<'a> Resolver<'a> {
};
let ident = Ident::new(Symbol::intern(name), span);
self.lookup_typo_candidate(&[Segment::from_ident(ident)], MacroNS, is_macro, span)
.map(|suggestion| suggestion.candidate)
});

if let Some(suggestion) = suggestion {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/associated-types/associated-types-eq-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0412]: cannot find type `A` in this scope
--> $DIR/associated-types-eq-1.rs:10:12
|
LL | let _: A = x.boo(); //~ ERROR cannot find type `A` in this scope
| ^ did you mean `I`?
| ^ help: a type parameter with a similar name exists: `I`

error: aborting due to previous error

Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/empty/empty-struct-braces-expr.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ error[E0423]: expected value, found struct `Empty1`
LL | let e1 = Empty1; //~ ERROR expected value, found struct `Empty1`
| ^^^^^^
| |
| did you mean `XEmpty2`?
| did you mean `Empty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`

error[E0423]: expected function, found struct `Empty1`
--> $DIR/empty-struct-braces-expr.rs:16:14
|
LL | let e1 = Empty1(); //~ ERROR expected function, found struct `Empty1`
| ^^^^^^
| |
| did you mean `XEmpty2`?
| did you mean `Empty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`

error[E0423]: expected value, found struct variant `E::Empty3`
--> $DIR/empty-struct-braces-expr.rs:17:14
Expand All @@ -34,17 +34,17 @@ error[E0423]: expected value, found struct `XEmpty1`
LL | let xe1 = XEmpty1; //~ ERROR expected value, found struct `XEmpty1`
| ^^^^^^^
| |
| did you mean `XEmpty2`?
| did you mean `XEmpty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`

error[E0423]: expected function, found struct `XEmpty1`
--> $DIR/empty-struct-braces-expr.rs:21:15
|
LL | let xe1 = XEmpty1(); //~ ERROR expected function, found struct `XEmpty1`
| ^^^^^^^
| |
| did you mean `XEmpty2`?
| did you mean `XEmpty1 { /* fields */ }`?
| help: a unit struct with a similar name exists: `XEmpty2`

error[E0599]: no variant named `Empty3` found for type `empty_struct::XE` in the current scope
--> $DIR/empty-struct-braces-expr.rs:22:19
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/empty/empty-struct-braces-pat-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ error[E0532]: expected unit struct/variant or constant, found struct variant `XE
LL | XE::XEmpty3 => ()
| ^^^^-------
| | |
| | did you mean `XEmpty4`?
| | help: a unit variant with a similar name exists: `XEmpty4`
| did you mean `XE::XEmpty3 { /* fields */ }`?

error: aborting due to 2 previous errors
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/empty/empty-struct-braces-pat-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,35 @@ error[E0532]: expected tuple struct/variant, found struct `Empty1`
LL | Empty1() => () //~ ERROR expected tuple struct/variant, found struct `Empty1`
| ^^^^^^
| |
| did you mean `XEmpty6`?
| did you mean `Empty1 { /* fields */ }`?
| help: a tuple struct with a similar name exists: `XEmpty6`

error[E0532]: expected tuple struct/variant, found struct `XEmpty1`
--> $DIR/empty-struct-braces-pat-2.rs:18:9
|
LL | XEmpty1() => () //~ ERROR expected tuple struct/variant, found struct `XEmpty1`
| ^^^^^^^
| |
| did you mean `XEmpty6`?
| did you mean `XEmpty1 { /* fields */ }`?
| help: a tuple struct with a similar name exists: `XEmpty6`

error[E0532]: expected tuple struct/variant, found struct `Empty1`
--> $DIR/empty-struct-braces-pat-2.rs:21:9
|
LL | Empty1(..) => () //~ ERROR expected tuple struct/variant, found struct `Empty1`
| ^^^^^^
| |
| did you mean `XEmpty6`?
| did you mean `Empty1 { /* fields */ }`?
| help: a tuple struct with a similar name exists: `XEmpty6`

error[E0532]: expected tuple struct/variant, found struct `XEmpty1`
--> $DIR/empty-struct-braces-pat-2.rs:24:9
|
LL | XEmpty1(..) => () //~ ERROR expected tuple struct/variant, found struct `XEmpty1`
| ^^^^^^^
| |
| did you mean `XEmpty6`?
| did you mean `XEmpty1 { /* fields */ }`?
| help: a tuple struct with a similar name exists: `XEmpty6`

error: aborting due to 4 previous errors

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/empty/empty-struct-braces-pat-3.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ error[E0532]: expected tuple struct/variant, found struct variant `XE::XEmpty3`
LL | XE::XEmpty3() => ()
| ^^^^-------
| | |
| | did you mean `XEmpty5`?
| | help: a tuple variant with a similar name exists: `XEmpty5`
| did you mean `XE::XEmpty3 { /* fields */ }`?

error[E0532]: expected tuple struct/variant, found struct variant `E::Empty3`
Expand All @@ -25,7 +25,7 @@ error[E0532]: expected tuple struct/variant, found struct variant `XE::XEmpty3`
LL | XE::XEmpty3(..) => ()
| ^^^^-------
| | |
| | did you mean `XEmpty5`?
| | help: a tuple variant with a similar name exists: `XEmpty5`
| did you mean `XE::XEmpty3 { /* fields */ }`?

error: aborting due to 4 previous errors
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/empty/empty-struct-tuple-pat.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ error[E0532]: expected unit struct/variant or constant, found tuple variant `XE:
LL | XE::XEmpty5 => (),
| ^^^^-------
| |
| did you mean `XEmpty4`?
| help: a unit variant with a similar name exists: `XEmpty4`

error: aborting due to 4 previous errors

Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/empty/empty-struct-unit-pat.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,25 @@ error[E0532]: expected tuple struct/variant, found unit struct `Empty2`
--> $DIR/empty-struct-unit-pat.rs:21:9
|
LL | Empty2() => () //~ ERROR expected tuple struct/variant, found unit struct `Empty2`
| ^^^^^^ did you mean `XEmpty6`?
| ^^^^^^ help: a tuple struct with a similar name exists: `XEmpty6`

error[E0532]: expected tuple struct/variant, found unit struct `XEmpty2`
--> $DIR/empty-struct-unit-pat.rs:24:9
|
LL | XEmpty2() => () //~ ERROR expected tuple struct/variant, found unit struct `XEmpty2`
| ^^^^^^^ did you mean `XEmpty6`?
| ^^^^^^^ help: a tuple struct with a similar name exists: `XEmpty6`

error[E0532]: expected tuple struct/variant, found unit struct `Empty2`
--> $DIR/empty-struct-unit-pat.rs:27:9
|
LL | Empty2(..) => () //~ ERROR expected tuple struct/variant, found unit struct `Empty2`
| ^^^^^^ did you mean `XEmpty6`?
| ^^^^^^ help: a tuple struct with a similar name exists: `XEmpty6`

error[E0532]: expected tuple struct/variant, found unit struct `XEmpty2`
--> $DIR/empty-struct-unit-pat.rs:30:9
|
LL | XEmpty2(..) => () //~ ERROR expected tuple struct/variant, found unit struct `XEmpty2`
| ^^^^^^^ did you mean `XEmpty6`?
| ^^^^^^^ help: a tuple struct with a similar name exists: `XEmpty6`

error[E0532]: expected tuple struct/variant, found unit variant `E::Empty4`
--> $DIR/empty-struct-unit-pat.rs:34:9
Expand All @@ -34,7 +34,7 @@ error[E0532]: expected tuple struct/variant, found unit variant `XE::XEmpty4`
LL | XE::XEmpty4() => (),
| ^^^^-------
| |
| did you mean `XEmpty5`?
| help: a tuple variant with a similar name exists: `XEmpty5`

error[E0532]: expected tuple struct/variant, found unit variant `E::Empty4`
--> $DIR/empty-struct-unit-pat.rs:42:9
Expand All @@ -48,7 +48,7 @@ error[E0532]: expected tuple struct/variant, found unit variant `XE::XEmpty4`
LL | XE::XEmpty4(..) => (),
| ^^^^-------
| |
| did you mean `XEmpty5`?
| help: a tuple variant with a similar name exists: `XEmpty5`

error: aborting due to 8 previous errors

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/error-codes/E0423.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ error[E0423]: expected function, found struct `Foo`
LL | let f = Foo(); //~ ERROR E0423
| ^^^
| |
| did you mean `foo`?
| did you mean `Foo { /* fields */ }`?
| help: a function with a similar name exists: `foo`

error[E0423]: expected value, found struct `S`
--> $DIR/E0423.rs:12:32
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/error-festival.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0425]: cannot find value `y` in this scope
--> $DIR/error-festival.rs:14:5
|
LL | y = 2;
| ^ did you mean `x`?
| ^ help: a local variable with a similar name exists: `x`

error[E0603]: constant `FOO` is private
--> $DIR/error-festival.rs:22:10
Expand Down
Loading

0 comments on commit 731f1d8

Please sign in to comment.