Modify the Levenshtein-based suggestions to include imports

Author: Ravi Shankar

src/librustc_resolve/lib.rs

@@ -33,7 +33,6 @@ extern crate syntax;
 #[no_link]
 extern crate rustc_bitflags;
 extern crate rustc_front;
-
 extern crate rustc;
 
 use self::PatternBindingMode::*;
@@ -69,7 +68,7 @@ use syntax::ast::{TyUs, TyU8, TyU16, TyU32, TyU64, TyF64, TyF32};
 use syntax::attr::AttrMetaMethods;
 use syntax::parse::token::{self, special_names, special_idents};
 use syntax::codemap::{self, Span, Pos};
-use syntax::util::lev_distance::{lev_distance, max_suggestion_distance};
+use syntax::util::lev_distance::find_best_match_for_name;
 
 use rustc_front::intravisit::{self, FnKind, Visitor};
 use rustc_front::hir;
@@ -94,7 +93,6 @@ use std::cell::{Cell, RefCell};
 use std::fmt;
 use std::mem::replace;
 use std::rc::{Rc, Weak};
-use std::usize;
 
 use resolve_imports::{Target, ImportDirective, ImportResolutionPerNamespace};
 use resolve_imports::Shadowable;
@@ -121,7 +119,7 @@ macro_rules! execute_callback {
 
 enum SuggestionType {
     Macro(String),
-    Function(String),
+    Function(token::InternedString),
     NotFound,
 }
 
@@ -3352,39 +3350,22 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         NoSuggestion
     }
 
-    fn find_best_match_for_name(&mut self, name: &str) -> SuggestionType {
-        let mut maybes: Vec<token::InternedString> = Vec::new();
-        let mut values: Vec<usize> = Vec::new();
-
+    fn find_best_match(&mut self, name: &str) -> SuggestionType {
         if let Some(macro_name) = self.session.available_macros
-                                 .borrow().iter().find(|n| n.as_str() == name) {
+                                  .borrow().iter().find(|n| n.as_str() == name) {
             return SuggestionType::Macro(format!("{}!", macro_name));
         }
 
-        for rib in self.value_ribs.iter().rev() {
-            for (&k, _) in &rib.bindings {
-                maybes.push(k.as_str());
-                values.push(usize::MAX);
-            }
-        }
-
-        let mut smallest = 0;
-        for (i, other) in maybes.iter().enumerate() {
-            values[i] = lev_distance(name, &other);
+        let names = self.value_ribs
+                    .iter()
+                    .rev()
+                    .flat_map(|rib| rib.bindings.keys());
 
-            if values[i] <= values[smallest] {
-                smallest = i;
+        if let Some(found) = find_best_match_for_name(names, name, None) {
+            if name != &*found {
+                return SuggestionType::Function(found);
             }
-        }
-
-        let max_distance = max_suggestion_distance(name);
-        if !values.is_empty() && values[smallest] <= max_distance && name != &maybes[smallest][..] {
-
-            SuggestionType::Function(maybes[smallest].to_string())
-
-        } else {
-            SuggestionType::NotFound
-        }
+        } SuggestionType::NotFound
     }
 
     fn resolve_expr(&mut self, expr: &Expr) {
@@ -3495,7 +3476,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                                     NoSuggestion => {
                                         // limit search to 5 to reduce the number
                                         // of stupid suggestions
-                                        match self.find_best_match_for_name(&path_name) {
+                                        match self.find_best_match(&path_name) {
                                             SuggestionType::Macro(s) => {
                                                 format!("the macro `{}`", s)
                                             }

src/librustc_resolve/resolve_imports.rs

@@ -32,11 +32,11 @@ use rustc::middle::privacy::*;
 use syntax::ast::{NodeId, Name};
 use syntax::attr::AttrMetaMethods;
 use syntax::codemap::Span;
+use syntax::util::lev_distance::find_best_match_for_name;
 
 use std::mem::replace;
 use std::rc::Rc;
 
-
 /// Contains data for specific types of import directives.
 #[derive(Copy, Clone,Debug)]
 pub enum ImportDirectiveSubclass {
@@ -424,17 +424,22 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         };
 
         // We need to resolve both namespaces for this to succeed.
-        //
 
         let mut value_result = UnknownResult;
         let mut type_result = UnknownResult;
+        let mut lev_suggestion = "".to_owned();
 
         // Search for direct children of the containing module.
         build_reduced_graph::populate_module_if_necessary(self.resolver, &target_module);
 
         match target_module.children.borrow().get(&source) {
             None => {
-                // Continue.
+                let names = target_module.children.borrow();
+                if let Some(name) = find_best_match_for_name(names.keys(),
+                                                             &source.as_str(),
+                                                             None) {
+                    lev_suggestion = format!(". Did you mean to use `{}`?", name);
+                }
             }
             Some(ref child_name_bindings) => {
                 // pub_err makes sure we don't give the same error twice.
@@ -494,6 +499,17 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                         // therefore accurately report that the names are
                         // unbound.
 
+                        if lev_suggestion.is_empty() {  // skip if we already have a suggestion
+                            let names = target_module.import_resolutions.borrow();
+                            if let Some(name) = find_best_match_for_name(names.keys(),
+                                                                         &source.as_str(),
+                                                                         None) {
+                                lev_suggestion =
+                                    format!(". Did you mean to use the re-exported import `{}`?",
+                                            name);
+                            }
+                        }
+
                         if value_result.is_unknown() {
                             value_result = UnboundResult;
                         }
@@ -671,9 +687,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                            target);
 
         if value_result.is_unbound() && type_result.is_unbound() {
-            let msg = format!("There is no `{}` in `{}`",
+            let msg = format!("There is no `{}` in `{}`{}",
                               source,
-                              module_to_string(&target_module));
+                              module_to_string(&target_module), lev_suggestion);
             return ResolveResult::Failed(Some((directive.span, msg)));
         }
         let value_used_public = value_used_reexport || value_used_public;

src/librustc_typeck/check/mod.rs

@@ -122,7 +122,7 @@ use syntax::codemap::{self, Span, Spanned};
 use syntax::owned_slice::OwnedSlice;
 use syntax::parse::token::{self, InternedString};
 use syntax::ptr::P;
-use syntax::util::lev_distance::lev_distance;
+use syntax::util::lev_distance::find_best_match_for_name;
 
 use rustc_front::intravisit::{self, Visitor};
 use rustc_front::hir;
@@ -2996,28 +2996,22 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
                                  tcx: &ty::ctxt<'tcx>,
                                  skip : Vec<InternedString>) {
         let name = field.node.as_str();
+        let names = variant.fields
+                    .iter()
+                    .filter_map(|ref field| {
+                        // ignore already set fields and private fields from non-local crates
+                        if skip.iter().any(|x| *x == field.name.as_str()) ||
+                           (variant.did.krate != LOCAL_CRATE && field.vis != Visibility::Public) {
+                               None
+                        } else {
+                            Some(&field.name)
+                        }
+                    });
+
         // only find fits with at least one matching letter
-        let mut best_dist = name.len();
-        let mut best = None;
-        for elem in &variant.fields {
-            let n = elem.name.as_str();
-            // ignore already set fields
-            if skip.iter().any(|x| *x == n) {
-                continue;
-            }
-            // ignore private fields from non-local crates
-            if variant.did.krate != LOCAL_CRATE && elem.vis != Visibility::Public {
-                continue;
-            }
-            let dist = lev_distance(&n, &name);
-            if dist < best_dist {
-                best = Some(n);
-                best_dist = dist;
-            }
-        }
-        if let Some(n) = best {
+        if let Some(name) = find_best_match_for_name(names, &name, Some(name.len())) {
             tcx.sess.span_help(field.span,
-                &format!("did you mean `{}`?", n));
+                &format!("did you mean `{}`?", name));
         }
     }
 

src/libsyntax/ext/base.rs

@@ -24,7 +24,7 @@ use parse::token;
 use parse::token::{InternedString, intern, str_to_ident};
 use ptr::P;
 use util::small_vector::SmallVector;
-use util::lev_distance::{lev_distance, max_suggestion_distance};
+use util::lev_distance::find_best_match_for_name;
 use ext::mtwt;
 use fold::Folder;
 
@@ -780,15 +780,8 @@ impl<'a> ExtCtxt<'a> {
     }
 
     pub fn suggest_macro_name(&mut self, name: &str, span: Span) {
-        let mut min: Option<(Name, usize)> = None;
-        let max_dist = max_suggestion_distance(name);
-        for macro_name in self.syntax_env.names.iter() {
-            let dist = lev_distance(name, &macro_name.as_str());
-            if dist <= max_dist && (min.is_none() || min.unwrap().1 > dist) {
-                min = Some((*macro_name, dist));
-            }
-        }
-        if let Some((suggestion, _)) = min {
+        let names = &self.syntax_env.names;
+        if let Some(suggestion) = find_best_match_for_name(names.iter(), name, None) {
             self.fileline_help(span, &format!("did you mean `{}!`?", suggestion));
         }
     }

src/libsyntax/util/lev_distance.rs

@@ -8,50 +8,64 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use ast::Name;
 use std::cmp;
+use parse::token::InternedString;
 
-pub fn lev_distance(me: &str, t: &str) -> usize {
-    if me.is_empty() { return t.chars().count(); }
-    if t.is_empty() { return me.chars().count(); }
+/// To find the Levenshtein distance between two strings
+pub fn lev_distance(a: &str, b: &str) -> usize {
+    // cases which don't require further computation
+    if a.is_empty() {
+        return b.chars().count();
+    } else if b.is_empty() {
+        return a.chars().count();
+    }
 
-    let mut dcol: Vec<_> = (0..t.len() + 1).collect();
+    let mut dcol: Vec<_> = (0..b.len() + 1).collect();
     let mut t_last = 0;
 
-    for (i, sc) in me.chars().enumerate() {
-
+    for (i, sc) in a.chars().enumerate() {
         let mut current = i;
         dcol[0] = current + 1;
 
-        for (j, tc) in t.chars().enumerate() {
-
+        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;
             t_last = j;
         }
-    }
-
-    dcol[t_last + 1]
+    } dcol[t_last + 1]
 }
 
-pub fn max_suggestion_distance(name: &str) -> usize {
-    use std::cmp::max;
-    // As a loose rule to avoid obviously incorrect suggestions, clamp the
-    // maximum edit distance we will accept for a suggestion to one third of
-    // the typo'd name's length.
-    max(name.len(), 3) / 3
+/// To find the best match for a given string from an iterator of names
+/// 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
+pub fn find_best_match_for_name<'a, T>(iter_names: T,
+                                       lookup: &str,
+                                       dist: Option<usize>) -> Option<InternedString>
+    where T: Iterator<Item = &'a Name> {
+    let max_dist = dist.map_or_else(|| cmp::max(lookup.len(), 3) / 3, |d| d);
+    iter_names
+    .filter_map(|name| {
+        let dist = lev_distance(lookup, &name.as_str());
+        match dist <= max_dist {    // filter the unwanted cases
+            true => Some((name.as_str(), dist)),
+            false => None,
+        }
+    })
+    .min_by_key(|&(_, val)| val)    // extract the tuple containing the minimum edit distance
+    .map(|(s, _)| s)                // and return only the string
 }
 
 #[test]
 fn test_lev_distance() {
-    use std::char::{ from_u32, MAX };
+    use std::char::{from_u32, MAX};
     // Test bytelength agnosticity
     for c in (0..MAX as u32)
              .filter_map(|i| from_u32(i))

src/test/compile-fail/unresolved-import.rs

@@ -8,24 +8,26 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// ignore-tidy-linelength
+
 use foo::bar; //~ ERROR unresolved import `foo::bar`. Maybe a missing `extern crate foo`?
 
-use bar::baz as x; //~ ERROR unresolved import `bar::baz`. There is no `baz` in `bar`
+use bar::Baz as x; //~ ERROR unresolved import `bar::Baz`. There is no `Baz` in `bar`. Did you mean to use `Bar`?
 
-use food::baz; //~ ERROR unresolved import `food::baz`. There is no `baz` in `food`
+use food::baz; //~ ERROR unresolved import `food::baz`. There is no `baz` in `food`. Did you mean to use the re-exported import `bag`?
 
-use food::{quux as beans}; //~ ERROR unresolved import `food::quux`. There is no `quux` in `food`
+use food::{beens as Foo}; //~ ERROR unresolved import `food::beens`. There is no `beens` in `food`. Did you mean to use the re-exported import `beans`?
 
 mod bar {
-    struct bar;
+    pub struct Bar;
 }
 
 mod food {
-    pub use self::zug::baz::{self as bag, quux as beans};
+    pub use self::zug::baz::{self as bag, foobar as beans};
 
     mod zug {
         pub mod baz {
-            pub struct quux;
+            pub struct foobar;
         }
     }
 }