Merge pull request #1824 from Glamhoth/suggestions

Make suggestions based on Damerau-Levenshtein distance

Author: kinnison

Cargo.lock

@@ -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)

Cargo.toml

@@ -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!("")
+        }
+    }
 }

src/dist/manifest.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<String>) {
             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")

src/errors.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<String> {
+        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());
                 }

src/toolchain.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),
+        );
+    });
+}

tests/cli-v2.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) {