diff --git a/Cargo.lock b/Cargo.lock index d3e8f63ed4..8762993cde 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1249,6 +1249,7 @@ dependencies = [ "scopeguard 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "sha2 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", + "strsim 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)", "tar 0.4.24 (registry+https://github.com/rust-lang/crates.io-index)", "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", "term 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1430,6 +1431,11 @@ name = "strsim" version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "strsim" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "syn" version = "0.15.33" @@ -2044,6 +2050,7 @@ dependencies = [ "checksum stable_deref_trait 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dba1a27d3efae4351c8051072d619e3ade2820635c3958d826bfea39d59b54c8" "checksum string 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "b639411d0b9c738748b5397d5ceba08e648f4f1992231aa859af1a017f31f60b" "checksum strsim 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" +"checksum strsim 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)" = "032c03039aae92b350aad2e3779c352e104d919cb192ba2fabbd7b831ce4f0f6" "checksum syn 0.15.33 (registry+https://github.com/rust-lang/crates.io-index)" = "ec52cd796e5f01d0067225a5392e70084acc4c0013fa71d55166d38a8b307836" "checksum synstructure 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "73687139bf99285483c96ac0add482c3776528beac1d97d444f6e91f203a2015" "checksum tar 0.4.24 (registry+https://github.com/rust-lang/crates.io-index)" = "2dd071a2604c8fd8902ca42908856821ed7a06e3cd846f84d75873c978dec7fb" diff --git a/Cargo.toml b/Cargo.toml index 1fbce99b48..89eadec5b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ same-file = "1" scopeguard = "1" semver = "0.9" sha2 = "0.8" +strsim = "0.9.1" tar = "0.4" tempdir = "0.3.4" # FIXME(issue #1788) diff --git a/src/dist/manifest.rs b/src/dist/manifest.rs index 9c861ea0e1..0017902695 100644 --- a/src/dist/manifest.rs +++ b/src/dist/manifest.rs @@ -448,4 +448,11 @@ impl Component { pkg.to_string() } } + pub fn target(&self) -> String { + if let Some(ref t) = self.target { + format!("{}", t) + } else { + format!("") + } + } } diff --git a/src/errors.rs b/src/errors.rs index f6eccebc71..fabff0a267 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -303,9 +303,13 @@ error_chain! { description("toolchain does not support components") display("toolchain '{}' does not support components", t) } - UnknownComponent(t: String, c: String) { + UnknownComponent(t: String, c: String, s: Option) { description("toolchain does not contain component") - display("toolchain '{}' does not contain component {}", t, c) + display("toolchain '{}' does not contain component {}{}", t, c, if let Some(suggestion) = s { + format!("; did you mean '{}'?", suggestion) + } else { + "".to_string() + }) } AddingRequiredComponent(t: String, c: String) { description("required component cannot be added") diff --git a/src/toolchain.rs b/src/toolchain.rs index 237d50f1e6..f66d306acd 100644 --- a/src/toolchain.rs +++ b/src/toolchain.rs @@ -2,6 +2,7 @@ use crate::config::Cfg; use crate::dist::dist::ToolchainDesc; use crate::dist::download::DownloadCfg; use crate::dist::manifest::Component; +use crate::dist::manifest::Manifest; use crate::dist::manifestation::{Changes, Manifestation}; use crate::dist::prefix::InstallPrefix; use crate::env_var; @@ -556,6 +557,89 @@ impl<'a> Toolchain<'a> { } } + fn get_component_suggestion( + &self, + component: &Component, + manifest: &Manifest, + only_installed: bool, + ) -> Option { + use strsim::damerau_levenshtein; + + // Suggest only for very small differences + // High number can result in innacurate suggestions for short queries e.g. `rls` + const MAX_DISTANCE: usize = 3; + + let components = self.list_components(); + if let Ok(components) = components { + let short_name_distance = components + .iter() + .filter(|c| !only_installed || c.installed) + .map(|c| { + ( + damerau_levenshtein( + &c.component.name(manifest)[..], + &component.name(manifest)[..], + ), + c, + ) + }) + .min_by_key(|t| t.0) + .expect("There should be always at least one component"); + + let long_name_distance = components + .iter() + .filter(|c| !only_installed || c.installed) + .map(|c| { + ( + damerau_levenshtein( + &c.component.name_in_manifest()[..], + &component.name(manifest)[..], + ), + c, + ) + }) + .min_by_key(|t| t.0) + .expect("There should be always at least one component"); + + let mut closest_distance = short_name_distance; + let mut closest_match = short_name_distance.1.component.short_name(manifest); + + // Find closer suggestion + if short_name_distance.0 > long_name_distance.0 { + closest_distance = long_name_distance; + + // Check if only targets differ + if closest_distance.1.component.short_name_in_manifest() + == component.short_name_in_manifest() + { + closest_match = long_name_distance.1.component.target(); + } else { + closest_match = long_name_distance + .1 + .component + .short_name_in_manifest() + .to_string(); + } + } else { + // Check if only targets differ + if closest_distance.1.component.short_name(manifest) + == component.short_name(manifest) + { + closest_match = short_name_distance.1.component.target().to_string(); + } + } + + // If suggestion is too different don't suggest anything + if closest_distance.0 > MAX_DISTANCE { + return None; + } else { + return Some(closest_match.to_string()); + } + } else { + return None; + } + } + pub fn add_component(&self, mut component: Component) -> Result<()> { if !self.exists() { return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into()); @@ -604,6 +688,7 @@ impl<'a> Toolchain<'a> { return Err(ErrorKind::UnknownComponent( self.name.to_string(), component.description(&manifest), + self.get_component_suggestion(&component, &manifest, false), ) .into()); } @@ -674,6 +759,7 @@ impl<'a> Toolchain<'a> { return Err(ErrorKind::UnknownComponent( self.name.to_string(), component.description(&manifest), + self.get_component_suggestion(&component, &manifest, true), ) .into()); } diff --git a/tests/cli-v2.rs b/tests/cli-v2.rs index 0fe2a9cbda..77e1c38e8d 100644 --- a/tests/cli-v2.rs +++ b/tests/cli-v2.rs @@ -4,8 +4,8 @@ pub mod mock; use crate::mock::clitools::{ - self, expect_err, expect_not_stdout_ok, expect_ok, expect_stderr_ok, expect_stdout_ok, - set_current_dist_date, this_host_triple, Config, Scenario, + self, expect_err, expect_not_stderr_ok, expect_not_stdout_ok, expect_ok, expect_stderr_ok, + expect_stdout_ok, set_current_dist_date, this_host_triple, Config, Scenario, }; use std::fs; use std::io::Write; @@ -920,3 +920,109 @@ fn update_unavailable_force() { ); }); } + +#[test] +fn add_component_suggest_best_match() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + expect_err( + config, + &["rustup", "component", "add", "rsl"], + "did you mean 'rls'?", + ); + expect_err( + config, + &["rustup", "component", "add", "rsl-preview"], + "did you mean 'rls-preview'?", + ); + expect_err( + config, + &["rustup", "component", "add", "rustd"], + "did you mean 'rustc'?", + ); + expect_not_stderr_ok( + config, + &["rustup", "component", "add", "potato"], + "did you mean", + ); + }); +} + +#[test] +fn remove_component_suggest_best_match() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + expect_not_stderr_ok( + config, + &["rustup", "component", "remove", "rsl"], + "did you mean 'rls'?", + ); + expect_ok(config, &["rustup", "component", "add", "rls"]); + expect_err( + config, + &["rustup", "component", "remove", "rsl"], + "did you mean 'rls'?", + ); + expect_ok(config, &["rustup", "component", "add", "rls-preview"]); + expect_err( + config, + &["rustup", "component", "add", "rsl-preview"], + "did you mean 'rls-preview'?", + ); + expect_err( + config, + &["rustup", "component", "remove", "rustd"], + "did you mean 'rustc'?", + ); + }); +} + +#[test] +fn add_target_suggest_best_match() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + expect_err( + config, + &[ + "rustup", + "target", + "add", + &format!("{}a", clitools::CROSS_ARCH1)[..], + ], + &format!("did you mean '{}'", clitools::CROSS_ARCH1), + ); + expect_not_stderr_ok( + config, + &["rustup", "target", "add", "potato"], + "did you mean", + ); + }); +} + +#[test] +fn remove_target_suggest_best_match() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + expect_not_stderr_ok( + config, + &[ + "rustup", + "target", + "remove", + &format!("{}a", clitools::CROSS_ARCH1)[..], + ], + &format!("did you mean '{}'", clitools::CROSS_ARCH1), + ); + expect_ok(config, &["rustup", "target", "add", clitools::CROSS_ARCH1]); + expect_err( + config, + &[ + "rustup", + "target", + "remove", + &format!("{}a", clitools::CROSS_ARCH1)[..], + ], + &format!("did you mean '{}'", clitools::CROSS_ARCH1), + ); + }); +} diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs index 6bdcca49f6..75978e4dcb 100644 --- a/tests/mock/clitools.rs +++ b/tests/mock/clitools.rs @@ -207,6 +207,16 @@ pub fn expect_not_stdout_ok(config: &Config, args: &[&str], expected: &str) { } } +pub fn expect_not_stderr_ok(config: &Config, args: &[&str], expected: &str) { + let out = run(config, args[0], &args[1..], &[]); + if out.ok || out.stderr.contains(expected) { + print_command(args, &out); + println!("expected.ok: false"); + print_indented("expected.stderr.does_not_contain", expected); + panic!(); + } +} + pub fn expect_stderr_ok(config: &Config, args: &[&str], expected: &str) { let out = run(config, args[0], &args[1..], &[]); if !out.ok || !out.stderr.contains(expected) {