Prefer implemented traits in suggestions.

If `a.method();` can't be resolved, we first look for implemented traits
globally and suggest those. If there are no such traits found, we only
then fall back to suggesting from the unfiltered list of traits.

Author: huonw

src/librustc_typeck/check/method/mod.rs

@@ -38,8 +38,9 @@ mod suggest;
 
 pub enum MethodError {
     // Did not find an applicable method, but we did find various
-    // static methods that may apply.
-    NoMatch(Vec<CandidateSource>),
+    // static methods that may apply, as well as a list of
+    // not-in-scope traits which may work.
+    NoMatch(Vec<CandidateSource>, Vec<ast::DefId>),
 
     // Multiple methods might apply.
     Ambiguity(Vec<CandidateSource>),
@@ -65,7 +66,7 @@ pub fn exists<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
 {
     match probe::probe(fcx, span, method_name, self_ty, call_expr_id) {
         Ok(_) => true,
-        Err(NoMatch(_)) => false,
+        Err(NoMatch(_, _)) => false,
         Err(Ambiguity(_)) => true,
     }
 }

src/librustc_typeck/check/method/probe.rs

@@ -11,6 +11,7 @@
 use super::{MethodError,Ambiguity,NoMatch};
 use super::MethodIndex;
 use super::{CandidateSource,ImplSource,TraitSource};
+use super::suggest;
 
 use check;
 use check::{FnCtxt, NoPreference};
@@ -25,6 +26,7 @@ use middle::infer::InferCtxt;
 use syntax::ast;
 use syntax::codemap::{Span, DUMMY_SP};
 use std::collections::HashSet;
+use std::mem;
 use std::rc::Rc;
 use util::ppaux::Repr;
 
@@ -42,6 +44,7 @@ struct ProbeContext<'a, 'tcx:'a> {
     extension_candidates: Vec<Candidate<'tcx>>,
     impl_dups: HashSet<ast::DefId>,
     static_candidates: Vec<CandidateSource>,
+    all_traits_search: bool,
 }
 
 struct CandidateStep<'tcx> {
@@ -127,7 +130,7 @@ pub fn probe<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
     // take place in the `fcx.infcx().probe` below.
     let steps = match create_steps(fcx, span, self_ty) {
         Some(steps) => steps,
-        None => return Err(NoMatch(Vec::new())),
+        None => return Err(NoMatch(Vec::new(), Vec::new())),
     };
 
     // Create a list of simplified self types, if we can.
@@ -208,9 +211,17 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
             steps: Rc::new(steps),
             opt_simplified_steps: opt_simplified_steps,
             static_candidates: Vec::new(),
+            all_traits_search: false,
         }
     }
 
+    fn reset(&mut self) {
+        self.inherent_candidates.clear();
+        self.extension_candidates.clear();
+        self.impl_dups.clear();
+        self.static_candidates.clear();
+    }
+
     fn tcx(&self) -> &'a ty::ctxt<'tcx> {
         self.fcx.tcx()
     }
@@ -446,6 +457,15 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
         }
     }
 
+    fn assemble_extension_candidates_for_all_traits(&mut self) {
+        let mut duplicates = HashSet::new();
+        for trait_info in suggest::all_traits(self.fcx.ccx) {
+            if duplicates.insert(trait_info.def_id) {
+                self.assemble_extension_candidates_for_trait(trait_info.def_id)
+            }
+        }
+    }
+
     fn assemble_extension_candidates_for_trait(&mut self,
                                                trait_def_id: ast::DefId) {
         debug!("assemble_extension_candidates_for_trait(trait_def_id={})",
@@ -715,7 +735,47 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
             }
         }
 
-        Err(NoMatch(self.static_candidates))
+        let static_candidates = mem::replace(&mut self.static_candidates, vec![]);
+
+        let out_of_scope_traits = if !self.all_traits_search {
+            // things failed, and we haven't yet looked through all
+            // traits, so lets do that now:
+            self.reset();
+            self.all_traits_search = true;
+
+            let span = self.span;
+            let tcx = self.tcx();
+
+            self.assemble_extension_candidates_for_all_traits();
+
+            match self.pick() {
+                Ok(p) => vec![p.method_ty.container.id()],
+                Err(Ambiguity(v)) => v.into_iter().map(|source| {
+                    match source {
+                        TraitSource(id) => id,
+                        ImplSource(impl_id) => {
+                            match ty::trait_id_of_impl(tcx, impl_id) {
+                                Some(id) => id,
+                                None => tcx.sess.span_bug(span,
+                                                          "found inherent method when looking \
+                                                           at traits")
+                            }
+                        }
+                    }
+                }).collect(),
+                // it'd be really weird for this assertion to trigger,
+                // given the `vec![]` in the else branch below
+                Err(NoMatch(_, others)) => {
+                    assert!(others.is_empty());
+                    vec![]
+                }
+            }
+        } else {
+            // we've just looked through all traits and didn't find
+            // anything at all.
+            vec![]
+        };
+        Err(NoMatch(static_candidates, out_of_scope_traits))
     }
 
     fn pick_step(&mut self, step: &CandidateStep<'tcx>) -> Option<PickResult<'tcx>> {

src/librustc_typeck/check/method/suggest.rs

@@ -35,7 +35,7 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                               error: MethodError)
 {
     match error {
-        MethodError::NoMatch(static_sources) => {
+        MethodError::NoMatch(static_sources, out_of_scope_traits) => {
             let cx = fcx.tcx();
             let method_ustring = method_name.user_string(cx);
 
@@ -75,7 +75,7 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                 report_candidates(fcx, span, method_name, static_sources);
             }
 
-            suggest_traits_to_import(fcx, span, rcvr_ty, method_name)
+            suggest_traits_to_import(fcx, span, rcvr_ty, method_name, out_of_scope_traits)
         }
 
         MethodError::Ambiguity(sources) => {
@@ -136,10 +136,35 @@ pub type AllTraitsVec = Vec<TraitInfo>;
 fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                                       span: Span,
                                       _rcvr_ty: Ty<'tcx>,
-                                      method_name: ast::Name)
+                                      method_name: ast::Name,
+                                      valid_out_of_scope_traits: Vec<ast::DefId>)
 {
     let tcx = fcx.tcx();
+    let method_ustring = method_name.user_string(tcx);
+
+    if !valid_out_of_scope_traits.is_empty() {
+        let mut candidates = valid_out_of_scope_traits;
+        candidates.sort();
+        let msg = format!(
+            "methods from traits can only be called if the trait is in scope; \
+             the following {traits_are} implemented and {define} a method `{name}`:",
+            traits_are = if candidates.len() == 1 {"trait is"} else {"traits are"},
+            define = if candidates.len() == 1 {"defines"} else {"define"},
+            name = method_ustring);
+
+        fcx.sess().fileline_help(span, &msg[]);
+
+        for (i, trait_did) in candidates.iter().enumerate() {
+            fcx.sess().fileline_help(span,
+                                     &*format!("candidate #{}: `{}`",
+                                               i + 1,
+                                               ty::item_path_str(fcx.tcx(), *trait_did)))
+
+        }
+        return
+    }
 
+    // there's no implemented traits, so lets suggest some traits to implement
     let mut candidates = all_traits(fcx.ccx)
         .filter(|info| trait_method(tcx, info.def_id, method_name).is_some())
         .collect::<Vec<_>>();
@@ -148,22 +173,16 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
         // sort from most relevant to least relevant
         candidates.sort_by(|a, b| a.cmp(b).reverse());
 
-        let method_ustring = method_name.user_string(tcx);
+        let msg = format!(
+            "methods from traits can only be called if the trait is implemented and \
+             in scope; no such traits are but the following {traits_define} a method `{name}`:",
+            traits_define = if candidates.len() == 1 {"trait defines"} else {"traits define"},
+            name = method_ustring);
 
-        span_help!(fcx.sess(), span,
-                   "methods from traits can only be called if the trait is implemented \
-                    and in scope; the following trait{s} define{inv_s} a method `{name}`:",
-                   s = if candidates.len() == 1 {""} else {"s"},
-                   inv_s = if candidates.len() == 1 {"s"} else {""},
-                   name = method_ustring);
+        fcx.sess().fileline_help(span, &msg[]);
 
         for (i, trait_info) in candidates.iter().enumerate() {
-            // provide a good-as-possible span; the span of
-            // the trait if it is local, or the span of the
-            // method call itself if not
-            let trait_span = fcx.tcx().map.def_id_span(trait_info.def_id, span);
-
-            fcx.sess().fileline_help(trait_span,
+            fcx.sess().fileline_help(span,
                                      &*format!("candidate #{}: `{}`",
                                                i + 1,
                                                ty::item_path_str(fcx.tcx(), trait_info.def_id)))
@@ -173,7 +192,7 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
 
 #[derive(Copy)]
 pub struct TraitInfo {
-    def_id: ast::DefId,
+    pub def_id: ast::DefId,
 }
 
 impl TraitInfo {
@@ -206,7 +225,7 @@ impl Ord for TraitInfo {
 }
 
 /// Retrieve all traits in this crate and any dependent crates.
-fn all_traits<'a>(ccx: &'a CrateCtxt) -> AllTraits<'a> {
+pub fn all_traits<'a>(ccx: &'a CrateCtxt) -> AllTraits<'a> {
     if ccx.all_traits.borrow().is_none() {
         use syntax::visit;
 
@@ -268,7 +287,7 @@ fn all_traits<'a>(ccx: &'a CrateCtxt) -> AllTraits<'a> {
     }
 }
 
-struct AllTraits<'a> {
+pub struct AllTraits<'a> {
     borrow: cell::Ref<'a Option<AllTraitsVec>>,
     idx: usize
 }

src/test/auxiliary/no_method_suggested_traits.rs

@@ -12,8 +12,13 @@ pub use reexport::Reexported;
 
 pub mod foo {
     pub trait PubPub {
-        fn method(&self);
+        fn method(&self) {}
+
+        fn method3(&self) {}
     }
+
+    impl PubPub for u32 {}
+    impl PubPub for i32 {}
 }
 pub mod bar {
     trait PubPriv {

src/test/compile-fail/no-method-suggested-traits.rs

@@ -13,18 +13,50 @@
 extern crate no_method_suggested_traits;
 
 mod foo {
-    trait Bar { //~ HELP `foo::Bar`
-        fn method(&self);
+    trait Bar {
+        fn method(&self) {}
+
+        fn method2(&self) {}
     }
+
+    impl Bar for u32 {}
+
+    impl Bar for char {}
 }
 
 fn main() {
     1u32.method();
     //~^ ERROR does not implement
+    //~^^ HELP the following traits are implemented and define a method `method`
+    //~^^^ HELP `foo::Bar`
+    //~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
+
+    'a'.method();
+    //~^ ERROR does not implement
+    //~^^ HELP the following trait is implemented and defines a method `method`
+    //~^^^ HELP `foo::Bar`
+
+    1i32.method();
+    //~^ ERROR does not implement
+    //~^^ HELP the following trait is implemented and defines a method `method`
+    //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
+
+    1u64.method();
+    //~^ ERROR does not implement
     //~^^ HELP the following traits define a method `method`
+    //~^^^ HELP `foo::Bar`
+    //~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
+    //~^^^^^ HELP `no_method_suggested_traits::reexport::Reexported`
+    //~^^^^^^ HELP `no_method_suggested_traits::bar::PubPriv`
+    //~^^^^^^^ HELP `no_method_suggested_traits::qux::PrivPub`
+    //~^^^^^^^^ HELP `no_method_suggested_traits::quz::PrivPriv`
+
+    1u64.method2();
+    //~^ ERROR does not implement
+    //~^^ HELP the following trait defines a method `method2`
+    //~^^^ HELP `foo::Bar`
+    1u64.method3();
+    //~^ ERROR does not implement
+    //~^^ HELP the following trait defines a method `method3`
     //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
-    //~^^^^ HELP `no_method_suggested_traits::reexport::Reexported`
-    //~^^^^^ HELP `no_method_suggested_traits::bar::PubPriv`
-    //~^^^^^^ HELP `no_method_suggested_traits::qux::PrivPub`
-    //~^^^^^^^ HELP `no_method_suggested_traits::quz::PrivPriv`
 }