From e81ae40770a74dbdeb70a5abcb41e1eb3cc2a584 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 27 Jan 2015 11:29:30 +1100 Subject: [PATCH] Try to only suggest implementable traits for method calls. 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 #21420. --- src/librustc_typeck/check/method/suggest.rs | 60 +++++++++++++- .../auxiliary/no_method_suggested_traits.rs | 3 + .../method-suggestion-no-duplication.rs | 4 +- .../no-method-suggested-traits.rs | 78 ++++++++++++++++++- 4 files changed, 138 insertions(+), 7 deletions(-) diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index 3cf9a1a9456..6f64374ea4a 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -34,6 +34,11 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, method_name: ast::Name, 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(); @@ -135,7 +140,7 @@ pub type AllTraitsVec = Vec; 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) { @@ -165,9 +170,22 @@ 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::>(); if candidates.len() > 0 { @@ -175,6 +193,9 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, 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}`, \ @@ -194,6 +215,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, diff --git a/src/test/auxiliary/no_method_suggested_traits.rs b/src/test/auxiliary/no_method_suggested_traits.rs index 328561495ee..20cebb9be17 100644 --- a/src/test/auxiliary/no_method_suggested_traits.rs +++ b/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) {} diff --git a/src/test/compile-fail/method-suggestion-no-duplication.rs b/src/test/compile-fail/method-suggestion-no-duplication.rs index 627fc6f0b05..e807d2b9448 100644 --- a/src/test/compile-fail/method-suggestion-no-duplication.rs +++ b/src/test/compile-fail/method-suggestion-no-duplication.rs @@ -10,7 +10,9 @@ // issue #21405 -fn foo(f: F) where F: FnMut(usize) {} +struct Foo; + +fn foo(f: F) where F: FnMut(Foo) {} fn main() { foo(|s| s.is_empty()); diff --git a/src/test/compile-fail/no-method-suggested-traits.rs b/src/test/compile-fail/no-method-suggested-traits.rs index ba8121eb5cc..2c14dfad3b8 100644 --- a/src/test/compile-fail/no-method-suggested-traits.rs +++ b/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 }