Skip to content

Commit e81ae40

Browse files
committed
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 rust-lang#21420.
1 parent 1d00c54 commit e81ae40

File tree

4 files changed

+138
-7
lines changed

4 files changed

+138
-7
lines changed

src/librustc_typeck/check/method/suggest.rs

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
3434
method_name: ast::Name,
3535
error: MethodError)
3636
{
37+
// avoid suggestions when we don't know what's going on.
38+
if ty::type_is_error(rcvr_ty) {
39+
return
40+
}
41+
3742
match error {
3843
MethodError::NoMatch(static_sources, out_of_scope_traits) => {
3944
let cx = fcx.tcx();
@@ -135,7 +140,7 @@ pub type AllTraitsVec = Vec<TraitInfo>;
135140

136141
fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
137142
span: Span,
138-
_rcvr_ty: Ty<'tcx>,
143+
rcvr_ty: Ty<'tcx>,
139144
method_name: ast::Name,
140145
valid_out_of_scope_traits: Vec<ast::DefId>)
141146
{
@@ -165,16 +170,32 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
165170
return
166171
}
167172

168-
// there's no implemented traits, so lets suggest some traits to implement
173+
let type_is_local = type_derefs_to_local(fcx, span, rcvr_ty);
174+
175+
// there's no implemented traits, so lets suggest some traits to
176+
// implement, by finding ones that have the method name, and are
177+
// legal to implement.
169178
let mut candidates = all_traits(fcx.ccx)
170-
.filter(|info| trait_method(tcx, info.def_id, method_name).is_some())
179+
.filter(|info| {
180+
// we approximate the coherence rules to only suggest
181+
// traits that are legal to implement by requiring that
182+
// either the type or trait is local. Multidispatch means
183+
// this isn't perfect (that is, there are cases when
184+
// implementing a trait would be legal but is rejected
185+
// here).
186+
(type_is_local || ast_util::is_local(info.def_id))
187+
&& trait_method(tcx, info.def_id, method_name).is_some()
188+
})
171189
.collect::<Vec<_>>();
172190

173191
if candidates.len() > 0 {
174192
// sort from most relevant to least relevant
175193
candidates.sort_by(|a, b| a.cmp(b).reverse());
176194
candidates.dedup();
177195

196+
// FIXME #21673 this help message could be tuned to the case
197+
// of a type parameter: suggest adding a trait bound rather
198+
// than implementing.
178199
let msg = format!(
179200
"methods from traits can only be called if the trait is implemented and in scope; \
180201
the following {traits_define} a method `{name}`, \
@@ -194,6 +215,39 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
194215
}
195216
}
196217

218+
/// Checks whether there is a local type somewhere in the chain of
219+
/// autoderefs of `rcvr_ty`.
220+
fn type_derefs_to_local<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
221+
span: Span,
222+
rcvr_ty: Ty<'tcx>) -> bool {
223+
check::autoderef(fcx, span, rcvr_ty, None,
224+
check::UnresolvedTypeAction::Ignore, check::NoPreference,
225+
|&: ty, _| {
226+
let is_local = match ty.sty {
227+
ty::ty_enum(did, _) | ty::ty_struct(did, _) => ast_util::is_local(did),
228+
229+
ty::ty_trait(ref tr) => ast_util::is_local(tr.principal_def_id()),
230+
231+
ty::ty_param(_) => true,
232+
233+
// the user cannot implement traits for unboxed closures, so
234+
// there's no point suggesting anything at all, local or not.
235+
ty::ty_closure(..) => return Some(false),
236+
237+
// everything else (primitive types etc.) is effectively
238+
// non-local (there are "edge" cases, e.g. (LocalType,), but
239+
// the noise from these sort of types is usually just really
240+
// annoying, rather than any sort of help).
241+
_ => false
242+
};
243+
if is_local {
244+
Some(true)
245+
} else {
246+
None
247+
}
248+
}).2.unwrap_or(false)
249+
}
250+
197251
#[derive(Copy)]
198252
pub struct TraitInfo {
199253
pub def_id: ast::DefId,

src/test/auxiliary/no_method_suggested_traits.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010

1111
pub use reexport::Reexported;
1212

13+
pub struct Foo;
14+
pub enum Bar { X }
15+
1316
pub mod foo {
1417
pub trait PubPub {
1518
fn method(&self) {}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010

1111
// issue #21405
1212

13-
fn foo<F>(f: F) where F: FnMut(usize) {}
13+
struct Foo;
14+
15+
fn foo<F>(f: F) where F: FnMut(Foo) {}
1416

1517
fn main() {
1618
foo(|s| s.is_empty());

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

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212

1313
extern crate no_method_suggested_traits;
1414

15+
struct Foo;
16+
enum Bar { X }
17+
1518
mod foo {
1619
trait Bar {
1720
fn method(&self) {}
@@ -25,23 +28,48 @@ mod foo {
2528
}
2629

2730
fn main() {
31+
// test the values themselves, and autoderef.
32+
33+
2834
1u32.method();
2935
//~^ HELP following traits are implemented but not in scope, perhaps add a `use` for one of them
3036
//~^^ ERROR does not implement
3137
//~^^^ HELP `foo::Bar`
3238
//~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
39+
std::rc::Rc::new(&mut Box::new(&1u32)).method();
40+
//~^ HELP following traits are implemented but not in scope, perhaps add a `use` for one of them
41+
//~^^ ERROR does not implement
42+
//~^^^ HELP `foo::Bar`
43+
//~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
3344

3445
'a'.method();
3546
//~^ ERROR does not implement
3647
//~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
3748
//~^^^ HELP `foo::Bar`
49+
std::rc::Rc::new(&mut Box::new(&'a')).method();
50+
//~^ ERROR does not implement
51+
//~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
52+
//~^^^ HELP `foo::Bar`
3853

3954
1i32.method();
4055
//~^ ERROR does not implement
4156
//~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
4257
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`
58+
std::rc::Rc::new(&mut Box::new(&1i32)).method();
59+
//~^ ERROR does not implement
60+
//~^^ HELP the following trait is implemented but not in scope, perhaps add a `use` for it:
61+
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`
4362

44-
1u64.method();
63+
Foo.method();
64+
//~^ ERROR does not implement
65+
//~^^ HELP following traits define a method `method`, perhaps you need to implement one of them
66+
//~^^^ HELP `foo::Bar`
67+
//~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
68+
//~^^^^^ HELP `no_method_suggested_traits::reexport::Reexported`
69+
//~^^^^^^ HELP `no_method_suggested_traits::bar::PubPriv`
70+
//~^^^^^^^ HELP `no_method_suggested_traits::qux::PrivPub`
71+
//~^^^^^^^^ HELP `no_method_suggested_traits::quz::PrivPriv`
72+
std::rc::Rc::new(&mut Box::new(&Foo)).method();
4573
//~^ ERROR does not implement
4674
//~^^ HELP following traits define a method `method`, perhaps you need to implement one of them
4775
//~^^^ HELP `foo::Bar`
@@ -55,8 +83,52 @@ fn main() {
5583
//~^ ERROR does not implement
5684
//~^^ HELP the following trait defines a method `method2`, perhaps you need to implement it
5785
//~^^^ HELP `foo::Bar`
58-
1u64.method3();
86+
std::rc::Rc::new(&mut Box::new(&1u64)).method2();
5987
//~^ ERROR does not implement
60-
//~^^ HELP the following trait defines a method `method3`, perhaps you need to implement it
88+
//~^^ HELP the following trait defines a method `method2`, perhaps you need to implement it
89+
//~^^^ HELP `foo::Bar`
90+
91+
no_method_suggested_traits::Foo.method2();
92+
//~^ ERROR does not implement
93+
//~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
94+
//~^^^ HELP `foo::Bar`
95+
std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Foo)).method2();
96+
//~^ ERROR does not implement
97+
//~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
98+
//~^^^ HELP `foo::Bar`
99+
no_method_suggested_traits::Bar::X.method2();
100+
//~^ ERROR does not implement
101+
//~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
102+
//~^^^ HELP `foo::Bar`
103+
std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Bar::X)).method2();
104+
//~^ ERROR does not implement
105+
//~^^ HELP following trait defines a method `method2`, perhaps you need to implement it
106+
//~^^^ HELP `foo::Bar`
107+
108+
Foo.method3();
109+
//~^ ERROR does not implement
110+
//~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
111+
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`
112+
std::rc::Rc::new(&mut Box::new(&Foo)).method3();
113+
//~^ ERROR does not implement
114+
//~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
115+
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`
116+
Bar::X.method3();
117+
//~^ ERROR does not implement
118+
//~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
61119
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`
120+
std::rc::Rc::new(&mut Box::new(&Bar::X)).method3();
121+
//~^ ERROR does not implement
122+
//~^^ HELP following trait defines a method `method3`, perhaps you need to implement it
123+
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`
124+
125+
// should have no help:
126+
1us.method3(); //~ ERROR does not implement
127+
std::rc::Rc::new(&mut Box::new(&1us)).method3(); //~ ERROR does not implement
128+
no_method_suggested_traits::Foo.method3(); //~ ERROR does not implement
129+
std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Foo)).method3();
130+
//~^ ERROR does not implement
131+
no_method_suggested_traits::Bar::X.method3(); //~ ERROR does not implement
132+
std::rc::Rc::new(&mut Box::new(&no_method_suggested_traits::Bar::X)).method3();
133+
//~^ ERROR does not implement
62134
}

0 commit comments

Comments
 (0)