Auto merge of rust-lang#21675 - huonw:less-false-positives, r=nikomatsakis

That is, when offering suggestions for unresolved method calls, avoid
suggesting traits for which implementing the trait for the receiver type
either makes little sense (e.g. type errors, or sugared unboxed
closures), or violates coherence.

The latter is approximated by ensuring that at least one of `{receiver
type, trait}` is local. This isn't precisely correct due to
multidispatch, but the error messages one encounters in such situation
are useless more often than not; it is better to be conservative and
miss some cases, than have overly many false positives (e.g. writing
`some_slice.map(|x| ...)` uselessly suggested that one should implement
`IteratorExt` for `&[T]`, while the correct fix is to call `.iter()`).

Closes rust-lang#21420.

Author: bors

src/librustc_typeck/check/method/suggest.rs

@@ -36,6 +36,11 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                               callee_expr: &ast::Expr,
                               error: MethodError)
 {
+    // avoid suggestions when we don't know what's going on.
+    if ty::type_is_error(rcvr_ty) {
+        return
+    }
+
     match error {
         MethodError::NoMatch(static_sources, out_of_scope_traits) => {
             let cx = fcx.tcx();
@@ -149,7 +154,7 @@ pub type AllTraitsVec = Vec<TraitInfo>;
 
 fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                                       span: Span,
-                                      _rcvr_ty: Ty<'tcx>,
+                                      rcvr_ty: Ty<'tcx>,
                                       method_name: ast::Name,
                                       valid_out_of_scope_traits: Vec<ast::DefId>)
 {
@@ -179,16 +184,32 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
         return
     }
 
-    // there's no implemented traits, so lets suggest some traits to implement
+    let type_is_local = type_derefs_to_local(fcx, span, rcvr_ty);
+
+    // there's no implemented traits, so lets suggest some traits to
+    // implement, by finding ones that have the method name, and are
+    // legal to implement.
     let mut candidates = all_traits(fcx.ccx)
-        .filter(|info| trait_method(tcx, info.def_id, method_name).is_some())
+        .filter(|info| {
+            // we approximate the coherence rules to only suggest
+            // traits that are legal to implement by requiring that
+            // either the type or trait is local. Multidispatch means
+            // this isn't perfect (that is, there are cases when
+            // implementing a trait would be legal but is rejected
+            // here).
+            (type_is_local || ast_util::is_local(info.def_id))
+                && trait_method(tcx, info.def_id, method_name).is_some()
+        })
         .collect::<Vec<_>>();
 
     if candidates.len() > 0 {
         // sort from most relevant to least relevant
         candidates.sort_by(|a, b| a.cmp(b).reverse());
         candidates.dedup();
 
+        // FIXME #21673 this help message could be tuned to the case
+        // of a type parameter: suggest adding a trait bound rather
+        // than implementing.
         let msg = format!(
             "methods from traits can only be called if the trait is implemented and in scope; \
              the following {traits_define} a method `{name}`, \
@@ -208,6 +229,39 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
     }
 }
 
+/// Checks whether there is a local type somewhere in the chain of
+/// autoderefs of `rcvr_ty`.
+fn type_derefs_to_local<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
+                                  span: Span,
+                                  rcvr_ty: Ty<'tcx>) -> bool {
+    check::autoderef(fcx, span, rcvr_ty, None,
+                     check::UnresolvedTypeAction::Ignore, check::NoPreference,
+                     |&: ty, _| {
+        let is_local = match ty.sty {
+            ty::ty_enum(did, _) | ty::ty_struct(did, _) => ast_util::is_local(did),
+
+            ty::ty_trait(ref tr) => ast_util::is_local(tr.principal_def_id()),
+
+            ty::ty_param(_) => true,
+
+            // the user cannot implement traits for unboxed closures, so
+            // there's no point suggesting anything at all, local or not.
+            ty::ty_closure(..) => return Some(false),
+
+            // everything else (primitive types etc.) is effectively
+            // non-local (there are "edge" cases, e.g. (LocalType,), but
+            // the noise from these sort of types is usually just really
+            // annoying, rather than any sort of help).
+            _ => false
+        };
+        if is_local {
+            Some(true)
+        } else {
+            None
+        }
+    }).2.unwrap_or(false)
+}
+
 #[derive(Copy)]
 pub struct TraitInfo {
     pub def_id: ast::DefId,

src/test/auxiliary/no_method_suggested_traits.rs

@@ -10,6 +10,9 @@
 
 pub use reexport::Reexported;
 
+pub struct Foo;
+pub enum Bar { X }
+
 pub mod foo {
     pub trait PubPub {
         fn method(&self) {}

src/test/compile-fail/method-suggestion-no-duplication.rs

@@ -10,7 +10,9 @@
 
 // issue #21405
 
-fn foo<F>(f: F) where F: FnMut(usize) {}
+struct Foo;
+
+fn foo<F>(f: F) where F: FnMut(Foo) {}
 
 fn main() {
     foo(|s| s.is_empty());

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

@@ -12,6 +12,9 @@
 
 extern crate no_method_suggested_traits;
 
+struct Foo;
+enum Bar { X }
+
 mod foo {
     trait Bar {
         fn method(&self) {}
@@ -25,23 +28,48 @@ mod foo {
 }
 
 fn main() {
+    // test the values themselves, and autoderef.
+
+
     1u32.method();
     //~^ HELP following traits are implemented but not in scope, perhaps add a `use` for one of them
     //~^^ ERROR does not implement
     //~^^^ HELP `foo::Bar`
     //~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
+    std::rc::Rc::new(&mut Box::new(&1u32)).method();
+    //~^ HELP following traits are implemented but not in scope, perhaps add a `use` for one of them
+    //~^^ ERROR does not implement
+    //~^^^ HELP `foo::Bar`
+    //~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
 
     'a'.method();
     //~^ ERROR does not implement
     //~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
     //~^^^ HELP `foo::Bar`
+    std::rc::Rc::new(&mut Box::new(&'a')).method();
+    //~^ ERROR does not implement
+    //~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
+    //~^^^ HELP `foo::Bar`
 
     1i32.method();
     //~^ ERROR does not implement
     //~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
     //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
+    std::rc::Rc::new(&mut Box::new(&1i32)).method();
+    //~^ ERROR does not implement
+    //~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
+    //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
 
-    1u64.method();
+    Foo.method();
+    //~^ ERROR does not implement
+    //~^^ HELP following traits define a method `method`, perhaps you need to implement one of them
+    //~^^^ 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`
+    std::rc::Rc::new(&mut Box::new(&Foo)).method();
     //~^ ERROR does not implement
     //~^^ HELP following traits define a method `method`, perhaps you need to implement one of them
     //~^^^ HELP `foo::Bar`
@@ -55,8 +83,52 @@ fn main() {
     //~^ ERROR does not implement
     //~^^ HELP the following trait defines a method `method2`, perhaps you need to implement it
     //~^^^ HELP `foo::Bar`
-    1u64.method3();
+    std::rc::Rc::new(&mut Box::new(&1u64)).method2();
     //~^ ERROR does not implement
-    //~^^ HELP the following trait defines a method `method3`, perhaps you need to implement it
+    //~^^ HELP the following trait defines a method `method2`, perhaps you need to implement it
+    //~^^^ HELP `foo::Bar`
+
+    no_method_suggested_traits::Foo.method2();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
+    //~^^^ HELP `foo::Bar`
+    std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Foo)).method2();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
+    //~^^^ HELP `foo::Bar`
+    no_method_suggested_traits::Bar::X.method2();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
+    //~^^^ HELP `foo::Bar`
+    std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Bar::X)).method2();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
+    //~^^^ HELP `foo::Bar`
+
+    Foo.method3();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
+    //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
+    std::rc::Rc::new(&mut Box::new(&Foo)).method3();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
+    //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
+    Bar::X.method3();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
     //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
+    std::rc::Rc::new(&mut Box::new(&Bar::X)).method3();
+    //~^ ERROR does not implement
+    //~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
+    //~^^^ HELP `no_method_suggested_traits::foo::PubPub`
+
+    // should have no help:
+    1us.method3(); //~ ERROR does not implement
+    std::rc::Rc::new(&mut Box::new(&1us)).method3(); //~ ERROR does not implement
+    no_method_suggested_traits::Foo.method3();  //~ ERROR does not implement
+    std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Foo)).method3();
+    //~^ ERROR does not implement
+    no_method_suggested_traits::Bar::X.method3();  //~ ERROR does not implement
+    std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Bar::X)).method3();
+    //~^ ERROR does not implement
 }