extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-06-30 14:40:06 +02:00
Code Issues Packages Projects Releases Wiki Activity

Improve "Did you mean?" suggestions (#6579)

Browse Source 
* Copy lev_distance.rs from the rust compiler

* Minor changes to code from rust compiler

* "Did you mean" suggestions: test instrumented to generate markdown report

* Did you mean suggestions: delete test instrumentation

* Fix tests

* Fix test

`foo` has a genuine match: `for`

* Improve tests
This commit is contained in:
Dan Davison
2022-09-20 20:46:01 -04:00
committed by GitHub
parent 9aed95408d
commit ad0c6bf7d5
11 changed files with 348 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
crates/nu-command/tests/commands/get.rs
View File

@ -137,8 +137,8 @@ fn errors_fetching_by_column_not_present() {
sandbox.with_files(vec![FileWithContent(
"sample.toml",
r#"
[taconushell]
sentence_words = ["Yo", "quiero", "taconushell"]
[tacos]
sentence_words = ["Yo", "quiero", "tacos"]
[pizzanushell]
sentence-words = ["I", "want", "pizza"]
"#,
@ -153,7 +153,7 @@ fn errors_fetching_by_column_not_present() {
));
assert!(actual.err.contains("Name not found"),);
assert!(actual.err.contains("did you mean 'taconushell'"),);
assert!(actual.err.contains("did you mean 'tacos'"),);
})
}

14
crates/nu-command/tests/commands/source_env.rs
View File

@ -194,7 +194,7 @@ fn source_env_eval_export_env_hide() {
let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
assert!(actual.err.contains("did you mean"));
assert!(actual.err.contains("cannot find column"));
})
}
@ -275,21 +275,21 @@ fn source_env_is_scoped() {
sandbox.with_files(vec![FileWithContentToBeTrimmed(
"spam.nu",
r#"
def foo [] { 'foo' }
alias bar = 'bar'
def no-name-similar-to-this [] { 'no-name-similar-to-this' }
alias nor-similar-to-this = 'nor-similar-to-this'
"#,
)]);
let inp = &[r#"source-env spam.nu"#, r#"foo"#];
let inp = &[r#"source-env spam.nu"#, r#"no-name-similar-to-this"#];
let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
assert!(actual.err.contains("did you mean"));
assert!(actual.err.contains("can't run executable"));
let inp = &[r#"source-env spam.nu"#, r#"bar"#];
let inp = &[r#"source-env spam.nu"#, r#"nor-similar-to-this"#];
let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
assert!(actual.err.contains("did you mean"));
assert!(actual.err.contains("can't run executable"));
})
}

2
crates/nu-command/tests/commands/str_/into_string.rs
View File

@ -181,7 +181,7 @@ fn from_error() {
"#
));
assert_eq!(actual.out, "nu::shell::name_not_found");
assert_eq!(actual.out, "nu::shell::column_not_found");
}
#[test]

2
crates/nu-command/tests/commands/use_.rs
View File

@ -97,7 +97,7 @@ fn use_eval_export_env_hide() {
let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
assert!(actual.err.contains("did you mean"));
assert!(actual.err.contains("cannot find column"));
})
}

224
crates/nu-protocol/src/lev_distance.rs Normal file
View File

@ -0,0 +1,224 @@
// This file is copied from the rust compiler project:
// https://github.com/rust-lang/rust/blob/cf9ed0dd5836201843d28bbad50abfbe1913af2a/compiler/rustc_span/src/lev_distance.rs#L1
// https://github.com/rust-lang/rust/blob/cf9ed0dd5836201843d28bbad50abfbe1913af2a/LICENSE-MIT
//
// - the rust compiler-specific symbol::Symbol has been replaced by &str
// - unstable feature .then_some has been replaced by an if ... else expression
//! Levenshtein distances.
//!
//! The [Levenshtein distance] is a metric for measuring the difference between two strings.
//!
//! [Levenshtein distance]: https://en.wikipedia.org/wiki/Levenshtein_distance
use std::cmp;
/// Finds the Levenshtein distance between two strings.
///
/// Returns None if the distance exceeds the limit.
pub fn lev_distance(a: &str, b: &str, limit: usize) -> Option<usize> {
let n = a.chars().count();
let m = b.chars().count();
let min_dist = if n < m { m - n } else { n - m };
if min_dist > limit {
return None;
}
if n == 0 || m == 0 {
return if min_dist <= limit {
Some(min_dist)
} else {
None
};
}
let mut dcol: Vec<_> = (0..=m).collect();
for (i, sc) in a.chars().enumerate() {
let mut current = i;
dcol[0] = current + 1;
for (j, tc) in b.chars().enumerate() {
let next = dcol[j + 1];
if sc == tc {
dcol[j + 1] = current;
} else {
dcol[j + 1] = cmp::min(current, next);
dcol[j + 1] = cmp::min(dcol[j + 1], dcol[j]) + 1;
}
current = next;
}
}
if dcol[m] <= limit {
Some(dcol[m])
} else {
None
}
}
/// Provides a word similarity score between two words that accounts for substrings being more
/// meaningful than a typical Levenshtein distance. The lower the score, the closer the match.
/// 0 is an identical match.
///
/// Uses the Levenshtein distance between the two strings and removes the cost of the length
/// difference. If this is 0 then it is either a substring match or a full word match, in the
/// substring match case we detect this and return `1`. To prevent finding meaningless substrings,
/// eg. "in" in "shrink", we only perform this subtraction of length difference if one of the words
/// is not greater than twice the length of the other. For cases where the words are close in size
/// but not an exact substring then the cost of the length difference is discounted by half.
///
/// Returns `None` if the distance exceeds the limit.
pub fn lev_distance_with_substrings(a: &str, b: &str, limit: usize) -> Option<usize> {
let n = a.chars().count();
let m = b.chars().count();
// Check one isn't less than half the length of the other. If this is true then there is a
// big difference in length.
let big_len_diff = (n * 2) < m || (m * 2) < n;
let len_diff = if n < m { m - n } else { n - m };
let lev = lev_distance(a, b, limit + len_diff)?;
// This is the crux, subtracting length difference means exact substring matches will now be 0
let score = lev - len_diff;
// If the score is 0 but the words have different lengths then it's a substring match not a full
// word match
let score = if score == 0 && len_diff > 0 && !big_len_diff {
1 // Exact substring match, but not a total word match so return non-zero
} else if !big_len_diff {
// Not a big difference in length, discount cost of length difference
score + (len_diff + 1) / 2
} else {
// A big difference in length, add back the difference in length to the score
score + len_diff
};
if score <= limit {
Some(score)
} else {
None
}
}
/// Finds the best match for given word in the given iterator where substrings are meaningful.
///
/// A version of [`find_best_match_for_name`] that uses [`lev_distance_with_substrings`] as the score
/// for word similarity. This takes an optional distance limit which defaults to one-third of the
/// given word.
///
/// Besides the modified Levenshtein, we use case insensitive comparison to improve accuracy
/// on an edge case with a lower(upper)case letters mismatch.
pub fn find_best_match_for_name_with_substrings<'c>(
candidates: &[&'c str],
lookup: &str,
dist: Option<usize>,
) -> Option<&'c str> {
find_best_match_for_name_impl(true, candidates, lookup, dist)
}
/// Finds the best match for a given word in the given iterator.
///
/// As a loose rule to avoid the obviously incorrect suggestions, it takes
/// an optional limit for the maximum allowable edit distance, which defaults
/// to one-third of the given word.
///
/// Besides Levenshtein, we use case insensitive comparison to improve accuracy
/// on an edge case with a lower(upper)case letters mismatch.
#[allow(dead_code)]
pub fn find_best_match_for_name<'c>(
candidates: &[&'c str],
lookup: &str,
dist: Option<usize>,
) -> Option<&'c str> {
find_best_match_for_name_impl(false, candidates, lookup, dist)
}
#[cold]
fn find_best_match_for_name_impl<'c>(
use_substring_score: bool,
candidates: &[&'c str],
lookup: &str,
dist: Option<usize>,
) -> Option<&'c str> {
let lookup_uppercase = lookup.to_uppercase();
// Priority of matches:
// 1. Exact case insensitive match
// 2. Levenshtein distance match
// 3. Sorted word match
if let Some(c) = candidates
.iter()
.find(|c| c.to_uppercase() == lookup_uppercase)
{
return Some(*c);
}
let mut dist = dist.unwrap_or_else(|| cmp::max(lookup.len(), 3) / 3);
let mut best = None;
for c in candidates {
match if use_substring_score {
lev_distance_with_substrings(lookup, c, dist)
} else {
lev_distance(lookup, c, dist)
} {
Some(0) => return Some(*c),
Some(d) => {
dist = d - 1;
best = Some(*c);
}
None => {}
}
}
if best.is_some() {
return best;
}
find_match_by_sorted_words(candidates, lookup)
}
fn find_match_by_sorted_words<'c>(iter_names: &[&'c str], lookup: &str) -> Option<&'c str> {
iter_names.iter().fold(None, |result, candidate| {
if sort_by_words(candidate) == sort_by_words(lookup) {
Some(*candidate)
} else {
result
}
})
}
fn sort_by_words(name: &str) -> String {
let mut split_words: Vec<&str> = name.split('_').collect();
// We are sorting primitive &strs and can use unstable sort here.
split_words.sort_unstable();
split_words.join("_")
}
// This file is copied from the rust compiler project:
// https://github.com/rust-lang/rust/blob/cf9ed0dd5836201843d28bbad50abfbe1913af2a/compiler/rustc_span/src/lev_distance.rs#L1
// https://github.com/rust-lang/rust/blob/cf9ed0dd5836201843d28bbad50abfbe1913af2a/LICENSE-MIT
// Permission is hereby granted, free of charge, to any
// person obtaining a copy of this software and associated
// documentation files (the "Software"), to deal in the
// Software without restriction, including without
// limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of
// the Software, and to permit persons to whom the Software
// is furnished to do so, subject to the following
// conditions:
// The above copyright notice and this permission notice
// shall be included in all copies or substantial portions
// of the Software.
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
// Footer

1
crates/nu-protocol/src/lib.rs
View File

@ -5,6 +5,7 @@ pub mod engine;
mod example;
mod exportable;
mod id;
mod lev_distance;
mod module;
mod pipeline_data;
mod shell_error;

172
crates/nu-protocol/src/shell_error.rs
View File

@ -810,83 +810,23 @@ pub fn into_code(err: &ShellError) -> Option<String> {
err.code().map(|code| code.to_string())
}
pub fn did_you_mean(possibilities: &[String], tried: &str) -> Option<String> {
let mut possible_matches: Vec<_> = possibilities
.iter()
.map(|word| {
let edit_distance = levenshtein_distance(&word.to_lowercase(), &tried.to_lowercase());
(edit_distance, word.to_owned())
})
.collect();
pub fn did_you_mean(possibilities: &[String], input: &str) -> Option<String> {
let possibilities: Vec<&str> = possibilities.iter().map(|s| s.as_str()).collect();
possible_matches.sort();
if let Some((_, first)) = possible_matches.into_iter().next() {
Some(first)
} else {
None
}
}
// Borrowed from here https://github.com/wooorm/levenshtein-rs
pub fn levenshtein_distance(a: &str, b: &str) -> usize {
let mut result = 0;
/* Shortcut optimizations / degenerate cases. */
if a == b {
return result;
}
let length_a = a.chars().count();
let length_b = b.chars().count();
if length_a == 0 {
return length_b;
}
if length_b == 0 {
return length_a;
}
/* Initialize the vector.
*
* This is why it’s fast, normally a matrix is used,
* here we use a single vector. */
let mut cache: Vec<usize> = (1..).take(length_a).collect();
let mut distance_a;
let mut distance_b;
/* Loop. */
for (index_b, code_b) in b.chars().enumerate() {
result = index_b;
distance_a = index_b;
for (index_a, code_a) in a.chars().enumerate() {
distance_b = if code_a == code_b {
distance_a
} else {
distance_a + 1
};
distance_a = cache[index_a];
result = if distance_a > result {
if distance_b > result {
result + 1
} else {
distance_b
}
} else if distance_b > distance_a {
distance_a + 1
} else {
distance_b
};
cache[index_a] = result;
let suggestion =
crate::lev_distance::find_best_match_for_name_with_substrings(&possibilities, input, None)
.map(|s| s.to_string());
if let Some(suggestion) = &suggestion {
if suggestion.len() == 1 && suggestion.to_lowercase() != input.to_lowercase() {
return None;
}
}
suggestion
}
result
pub fn levenshtein_distance(a: &str, b: &str) -> usize {
crate::lev_distance::lev_distance(a, b, usize::max_value())
.expect("It is impossible to exceed the supplied limit since all types involved are usize.")
}
#[cfg(test)]
@ -894,26 +834,70 @@ mod tests {
use super::did_you_mean;
#[test]
fn did_you_mean_works_with_wrong_case() {
let possibilities = &["OS".into(), "PWD".into()];
let actual = did_you_mean(possibilities, "pwd");
let expected = Some(String::from("PWD"));
assert_eq!(actual, expected)
}
#[test]
fn did_you_mean_works_with_typo() {
let possibilities = &["OS".into(), "PWD".into()];
let actual = did_you_mean(possibilities, "PWF");
let expected = Some(String::from("PWD"));
assert_eq!(actual, expected)
}
#[test]
fn did_you_mean_works_with_wrong_case_and_typo() {
let possibilities = &["OS".into(), "PWD".into()];
let actual = did_you_mean(possibilities, "pwf");
let expected = Some(String::from("PWD"));
assert_eq!(actual, expected)
fn did_you_mean_examples() {
let all_cases = [
(
vec!["a", "b"],
vec![
("a", Some("a"), ""),
("A", Some("a"), ""),
(
"c",
None,
"Not helpful to suggest an arbitrary choice when none are close",
),
("ccccccccccccccccccccccc", None, "Not helpful to suggest an arbitrary choice when none are close"),
],
),
(
vec!["OS", "PWD", "PWDPWDPWDPWD"],
vec![
("pwd", Some("PWD"), "Exact case insensitive match yields a match"),
("pwdpwdpwdpwd", Some("PWDPWDPWDPWD"), "Exact case insensitive match yields a match"),
("PWF", Some("PWD"), "One-letter typo yields a match"),
("pwf", None, "Case difference plus typo yields no match"),
("Xwdpwdpwdpwd", None, "Case difference plus typo yields no match"),
]
),
(
vec!["foo", "bar", "baz"],
vec![
("fox", Some("foo"), ""),
("FOO", Some("foo"), ""),
("FOX", None, ""),
(
"ccc",
None,
"Not helpful to suggest an arbitrary choice when none are close",
),
(
"zzz",
None,
"'baz' does share a character, but rustc rule is edit distance must be <= 1/3 of the length of the user input",
),
],
),
(
vec!["aaaaaa"],
vec![
("XXaaaa", Some("aaaaaa"), "Distance of 2 out of 6 chars: close enough to meet rustc's rule"),
("XXXaaa", None, "Distance of 3 out of 6 chars: not close enough to meet rustc's rule"),
("XaaaaX", Some("aaaaaa"), "Distance of 2 out of 6 chars: close enough to meet rustc's rule"),
("XXaaaaXX", None, "Distance of 4 out of 6 chars: not close enough to meet rustc's rule")
]
),
];
for (possibilities, cases) in all_cases {
let possibilities: Vec<String> = possibilities.iter().map(|s| s.to_string()).collect();
for (input, expected_suggestion, discussion) in cases {
let suggestion = did_you_mean(&possibilities, input);
assert_eq!(
suggestion.as_deref(),
expected_suggestion,
"Expected the following reasoning to hold but it did not: '{}'",
discussion
);
}
}
}
}