Skip to content

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

Conversation

zackmdavis
Copy link
Member

@zackmdavis zackmdavis commented Jul 23, 2017

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.

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.
@rust-highfive
Copy link
Contributor

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@eddyb
Copy link
Member

eddyb commented Jul 24, 2017

Is there any way to get @rust-highfive to try randomly assigning again?

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Jul 24, 2017
@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2017
Copy link
Contributor

@nikomatsakis nikomatsakis left a 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.

format!("`{}` does not have this field", ty));
}
let available_field_names = self.available_field_names(variant);
err.note(&format!("available fields are: {}",
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, of course

let available_field_names = self.available_field_names(
struct_variant_def);
err.note(&format!("available fields are: {}",
available_field_names.join(", ")));
Copy link
Contributor

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.

@carols10cents carols10cents added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2017
Also, don't show the note if no fields are available (usually due to
privacy).
@zackmdavis
Copy link
Member Author

2dbfa39 adds the backticks, "... and n others" truncation, and declines to issue the note if we can't actually see any fields

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 4, 2017

📌 Commit 2dbfa39 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Aug 4, 2017

⌛ Testing commit 2dbfa39 with merge f2a5af7...

bors added a commit that referenced this pull request Aug 4, 2017
…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.
@bors
Copy link
Collaborator

bors commented Aug 4, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing f2a5af7 to master...

@bors bors merged commit 2dbfa39 into rust-lang:master Aug 4, 2017
@zackmdavis zackmdavis deleted the note_available_field_names_if_levenshtein_fails branch January 13, 2018 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List the available field names of a struct instead of reporting "field not found"
6 participants