-
Notifications
You must be signed in to change notification settings - Fork 13.4k
field does not exist error: note fields if Levenshtein suggestion fails #43442
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
field does not exist error: note fields if Levenshtein suggestion fails #43442
Conversation
When trying to access or initialize a nonexistent field, if we can't infer what field was meant (by virtue of the purported field in the source being a small Levenshtein distance away from an actual field, suggestive of a typo), issue a note listing all the available fields. To reduce terminal clutter, we don't issue the note when we have a `find_best_match_for_name` Levenshtein suggestion: the suggestion is probably right. The third argument of the call to `find_best_match_for_name` is changed to `None`, accepting the default maximum Levenshtein distance of one-third of the identifier supplied for correction. The previous value of `Some(name.len())` was overzealous, inappropriately very Levenshtein-distant suggestions when the attempted field access could not plausibly be a mere typo. For example, if a struct has fields `mule` and `phone`, but I type `.donkey`, I'd rather the error have a note listing that the available fields are, in fact, `mule` and `phone` (which is the behavior induced by this patch) rather than the error asking "did you mean `phone`?" (which is the behavior on master). The "only find fits with at least one matching letter" comment was accurate when it was first introduced in 09d9924 (January 2015), but is a vicious lie in its present context before a call to `find_best_match_for_name` and must be destroyed (replacing every letter is a Levenshtein distance of name.len()). The present author claims that this suffices to resolve rust-lang#42599.
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
Is there any way to get @rust-highfive to try randomly assigning again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I hate silly suggestions. But there are some stylistic nits.
src/librustc_typeck/check/mod.rs
Outdated
format!("`{}` does not have this field", ty)); | ||
} | ||
let available_field_names = self.available_field_names(variant); | ||
err.note(&format!("available fields are: {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, of course
src/librustc_typeck/check/mod.rs
Outdated
let available_field_names = self.available_field_names( | ||
struct_variant_def); | ||
err.note(&format!("available fields are: {}", | ||
available_field_names.join(", "))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm; ideally we would put backticks around these field names. But, more importantly, we generally have a rule that we only list out things exhaustively up to a certain limit (typically 5), and then after that we will say "...and others" or something. See e.g. this code in method suggestions.
Also, don't show the note if no fields are available (usually due to privacy).
2dbfa39 adds the backticks, "... and n others" truncation, and declines to issue the note if we can't actually see any fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
@bors r+ |
📌 Commit 2dbfa39 has been approved by |
…shtein_fails, r=nikomatsakis field does not exist error: note fields if Levenshtein suggestion fails When trying to access or initialize a nonexistent field, if we can't infer what field was meant (by virtue of the purported field in the source being a small Levenshtein distance away from an actual field, suggestive of a typo), issue a note listing all the available fields. To reduce terminal clutter, we don't issue the note when we have a `find_best_match_for_name` Levenshtein suggestion: the suggestion is probably right. The third argument of the call to `find_best_match_for_name` is changed to `None`, accepting the default maximum Levenshtein distance of one-third of the identifier supplied for correction. The previous value of `Some(name.len())` was overzealous, inappropriately very Levenshtein-distant suggestions when the attempted field access could not plausibly be a mere typo. For example, if a struct has fields `mule` and `phone`, but I type `.donkey`, I'd rather the error have a note listing that the available fields are, in fact, `mule` and `phone` (which is the behavior induced by this patch) rather than the error asking "did you mean `phone`?" (which is the behavior on master). The "only find fits with at least one matching letter" comment was accurate when it was first introduced in 09d9924 (January 2015), but is a vicious lie in its present context before a call to `find_best_match_for_name` and must be destroyed (replacing every letter is within a Levenshtein distance of name.len()). The present author claims that this suffices to resolve #42599.
☀️ Test successful - status-appveyor, status-travis |
When trying to access or initialize a nonexistent field, if we can't infer what
field was meant (by virtue of the purported field in the source being a small
Levenshtein distance away from an actual field, suggestive of a typo), issue a
note listing all the available fields. To reduce terminal clutter, we don't
issue the note when we have a
find_best_match_for_name
Levenshteinsuggestion: the suggestion is probably right.
The third argument of the call to
find_best_match_for_name
is changed toNone
, accepting the default maximum Levenshtein distance of one-third of theidentifier supplied for correction. The previous value of
Some(name.len())
was overzealous, inappropriately very Levenshtein-distant suggestions when the
attempted field access could not plausibly be a mere typo. For example, if a
struct has fields
mule
andphone
, but I type.donkey
, I'd rather theerror have a note listing that the available fields are, in fact,
mule
andphone
(which is the behavior induced by this patch) rather than the errorasking "did you mean
phone
?" (which is the behavior on master). The "onlyfind fits with at least one matching letter" comment was accurate when it was
first introduced in 09d9924 (January 2015), but is a vicious lie in its
present context before a call to
find_best_match_for_name
and must bedestroyed (replacing every letter is within a Levenshtein distance of name.len()).
The present author claims that this suffices to resolve #42599.