-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add a check for swapped words when we can't find an identifier #67849
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,14 +52,16 @@ where | |
T: Iterator<Item = &'a Symbol>, | ||
{ | ||
let max_dist = dist.map_or_else(|| cmp::max(lookup.len(), 3) / 3, |d| d); | ||
let name_vec: Vec<&Symbol> = iter_names.collect(); | ||
|
||
let (case_insensitive_match, levenstein_match) = iter_names | ||
let (case_insensitive_match, levenshtein_match) = name_vec | ||
.iter() | ||
.filter_map(|&name| { | ||
let dist = lev_distance(lookup, &name.as_str()); | ||
if dist <= max_dist { Some((name, dist)) } else { None } | ||
}) | ||
// Here we are collecting the next structure: | ||
// (case_insensitive_match, (levenstein_match, levenstein_distance)) | ||
// (case_insensitive_match, (levenshtein_match, levenshtein_distance)) | ||
.fold((None, None), |result, (candidate, dist)| { | ||
( | ||
if candidate.as_str().to_uppercase() == lookup.to_uppercase() { | ||
|
@@ -73,10 +75,31 @@ where | |
}, | ||
) | ||
}); | ||
|
||
// Priority of matches: | ||
// 1. Exact case insensitive match | ||
// 2. Levenshtein distance match | ||
// 3. Sorted word match | ||
if let Some(candidate) = case_insensitive_match { | ||
Some(candidate) // exact case insensitive match has a higher priority | ||
Some(*candidate) | ||
} else if levenshtein_match.is_some() { | ||
levenshtein_match.map(|(candidate, _)| *candidate) | ||
} else { | ||
levenstein_match.map(|(candidate, _)| candidate) | ||
find_match_by_sorted_words(name_vec, lookup) | ||
} | ||
} | ||
|
||
fn find_match_by_sorted_words<'a>(iter_names: Vec<&'a Symbol>, lookup: &str) -> Option<Symbol> { | ||
iter_names.iter().fold(None, |result, candidate| { | ||
if sort_by_words(&candidate.as_str()) == sort_by_words(lookup) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you're only doing comparison on the sorted elements... |
||
Some(**candidate) | ||
} else { | ||
result | ||
} | ||
}) | ||
} | ||
|
||
fn sort_by_words(name: &str) -> String { | ||
let mut split_words: Vec<&str> = name.split('_').collect(); | ||
split_words.sort(); | ||
split_words.join("_") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...I wonder if it wouldn't be more efficient to return a That being said, this is only done in a case where we're already emitting an error, so it should be fine. |
||
} |
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.
I would like to avoid this
collect
by cloning the iterator instead and passing that tofind_match_by_sorted_words
, if possible.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 I'm not sure how to do that. If we call
cloned()
,iter_names
is moved and we can't use that iterator again, and we still can't use the cloned one twice. Is there another way to clone it so that we can still use the originaliter_names
afterwards?