Skip to content

Implement suggestions for traits to import. #21008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 17, 2015

Conversation

huonw
Copy link
Member

@huonw huonw commented Jan 12, 2015

For a call like foo.bar() where the method bar can't be resolved,
the compiler will search for traits that have methods with name bar to
give a more informative error, providing a list of possibilities.

Closes #7643.

@rust-highfive
Copy link
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@huonw
Copy link
Member Author

huonw commented Jan 12, 2015

r? @nikomatsakis

This... isn't quite ready yet. The test's full output is:

no-method-suggested-traits.rs:23:10: 23:18 error: type `u32` does not implement any method in scope named `method`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: methods from traits can only be called if the trait is implemented and in scope; the following traits define a method `method`:
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:17:5: 19:6 help: candidate #1: `foo::Bar`
no-method-suggested-traits.rs:17     trait Bar { //~ HELP `foo::Bar`
no-method-suggested-traits.rs:18         fn method(&self);
no-method-suggested-traits.rs:19     }
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #2: `no_method_suggested_traits::foo::PubPub`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #3: `no_method_suggested_traits::bar::PubPriv`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #4: `no_method_suggested_traits::qux::PrivPub`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #5: `no_method_suggested_traits::quz::PrivPriv`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #6: `no_method_suggested_traits::reexport::Reexported`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #7: `no_method_suggested_traits::reexport::Reexported`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
error: aborting due to previous error

In particular: it lists private traits too.

@huonw
Copy link
Member Author

huonw commented Jan 12, 2015

(Also, the first help's wording is awkward.)

Would appreciate any guidance for either.

@nagisa
Copy link
Member

nagisa commented Jan 12, 2015

Repeating the span for each help message sounds redundant.

.any(|item| {
debug!("report_error: item: {:?}", *item);
match *item {
ty::MethodTraitItem(ref meth) => meth.name == method_name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you may want to filter out private methods here ( meth.vis )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a trait, so there's no distinction.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, you're right.

@huonw
Copy link
Member Author

huonw commented Jan 12, 2015

Improved it slightly, but it has the reexported path backwards (i.e. filtering by visibility removed the pub use'd path, not the actually-private one... meaning the listed path isn't actually a valid import):

no-method-suggested-traits.rs:23:10: 23:18 error: type `u32` does not implement any method in scope named `method`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: methods from traits can only be called if the trait is implemented and in scope; the following traits define a method `method`:
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:17:5: 19:6 help: candidate #1: `foo::Bar`
no-method-suggested-traits.rs:17     trait Bar { //~ HELP `foo::Bar`
no-method-suggested-traits.rs:18         fn method(&self);
no-method-suggested-traits.rs:19     }
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #2: `no_method_suggested_traits::foo::PubPub`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:23:10: 23:18 help: candidate #3: `no_method_suggested_traits::reexport::Reexported`
no-method-suggested-traits.rs:23     1u32.method();
                                          ^~~~~~~~
error: aborting due to previous error

@@ -340,6 +345,42 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,

report_candidates(fcx, span, method_name, static_sources);
}

let candidates = all_traits(fcx.ccx)
.filter(|did| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goodness but do we need some helpers for these kinds of things...

@nikomatsakis
Copy link
Contributor

Also, let's pull this code into its own file method/suggest.rs or something.

@huonw huonw force-pushed the trait-suggestions branch from 6713a40 to 012ca02 Compare January 13, 2015 23:52
For a call like `foo.bar()` where the method `bar` can't be resolved,
the compiler will search for traits that have methods with name `bar` to
give a more informative error, providing a list of possibilities.

Closes rust-lang#7643.
@huonw huonw force-pushed the trait-suggestions branch from 012ca02 to 06ad8bb Compare January 14, 2015 00:08
@huonw
Copy link
Member Author

huonw commented Jan 14, 2015

Updated with tweaks and better organisation. The test output now looks like:

no-method-suggested-traits.rs:22:10: 22:18 error: type `u32` does not implement any method in scope named `method`
no-method-suggested-traits.rs:22     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:22:10: 22:18 help: methods from traits can only be called if the trait is implemented and in scope; the following traits define a method `method`:
no-method-suggested-traits.rs:22     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:18:6: 18:6 help: candidate #1: `foo::Bar`
no-method-suggested-traits.rs:22:18: 22:18 help: candidate #2: `no_method_suggested_traits::foo::PubPub`
no-method-suggested-traits.rs:22:18: 22:18 help: candidate #3: `no_method_suggested_traits::bar::PubPriv`
no-method-suggested-traits.rs:22:18: 22:18 help: candidate #4: `no_method_suggested_traits::qux::PrivPub`
no-method-suggested-traits.rs:22:18: 22:18 help: candidate #5: `no_method_suggested_traits::quz::PrivPriv`
no-method-suggested-traits.rs:22:18: 22:18 help: candidate #6: `no_method_suggested_traits::reexport::Reexported`
no-method-suggested-traits.rs:22:18: 22:18 help: candidate #7: `no_method_suggested_traits::reexport::Reexported`
error: aborting due to previous error

@huonw
Copy link
Member Author

huonw commented Jan 14, 2015

I've made a few attempts to e.g. extract whether a non-local trait can be accessed from the current crate but I have not been able to achieve it in a correct-enough way as yet.

@nikomatsakis
Copy link
Contributor

@huonw do you find this is a big issue in practice? I would have expected we could kind of just suggest everything and the user can sort it out (and/or leave the screening of unreachable traits as a fixme).

@nikomatsakis
Copy link
Contributor

anyway, I think we could land the code as is, but with some relatively simple tweaks we could at least indicate whether the trait is implemented -- so we could do something like this, only print out the traits that are implemented but not imported, and if none are implemented, suggest traits to implement. That's probably good enough.

@huonw
Copy link
Member Author

huonw commented Jan 16, 2015

Updated again, now with is-trait-implemented filtering.

no-method-suggested-traits.rs:28:10: 28:18 error: type `u32` does not implement any method in scope named `method`
no-method-suggested-traits.rs:28     1u32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:28:18: 28:18 help: methods from traits can only be called if the trait is in scope; the following traits are implemented and define a method `method`:
no-method-suggested-traits.rs:28:18: 28:18 help: candidate #1: `foo::Bar`
no-method-suggested-traits.rs:28:18: 28:18 help: candidate #2: `no_method_suggested_traits::foo::PubPub`
no-method-suggested-traits.rs:34:9: 34:17 error: type `char` does not implement any method in scope named `method`
no-method-suggested-traits.rs:34     'a'.method();
                                         ^~~~~~~~
no-method-suggested-traits.rs:34:17: 34:17 help: methods from traits can only be called if the trait is in scope; the following trait is implemented and defines a method `method`:
no-method-suggested-traits.rs:34:17: 34:17 help: candidate #1: `foo::Bar`
no-method-suggested-traits.rs:39:10: 39:18 error: type `i32` does not implement any method in scope named `method`
no-method-suggested-traits.rs:39     1i32.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:39:18: 39:18 help: methods from traits can only be called if the trait is in scope; the following trait is implemented and defines a method `method`:
no-method-suggested-traits.rs:39:18: 39:18 help: candidate #1: `no_method_suggested_traits::foo::PubPub`
no-method-suggested-traits.rs:44:10: 44:18 error: type `u64` does not implement any method in scope named `method`
no-method-suggested-traits.rs:44     1u64.method();
                                          ^~~~~~~~
no-method-suggested-traits.rs:44:18: 44:18 help: 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 `method`:
no-method-suggested-traits.rs:44:18: 44:18 help: candidate #1: `foo::Bar`
no-method-suggested-traits.rs:44:18: 44:18 help: candidate #2: `no_method_suggested_traits::foo::PubPub`
no-method-suggested-traits.rs:44:18: 44:18 help: candidate #3: `no_method_suggested_traits::bar::PubPriv`
no-method-suggested-traits.rs:44:18: 44:18 help: candidate #4: `no_method_suggested_traits::qux::PrivPub`
no-method-suggested-traits.rs:44:18: 44:18 help: candidate #5: `no_method_suggested_traits::quz::PrivPriv`
no-method-suggested-traits.rs:44:18: 44:18 help: candidate #6: `no_method_suggested_traits::reexport::Reexported`
no-method-suggested-traits.rs:44:18: 44:18 help: candidate #7: `no_method_suggested_traits::reexport::Reexported`
no-method-suggested-traits.rs:54:10: 54:19 error: type `u64` does not implement any method in scope named `method2`
no-method-suggested-traits.rs:54     1u64.method2();
                                          ^~~~~~~~~
no-method-suggested-traits.rs:54:19: 54:19 help: methods from traits can only be called if the trait is implemented and in scope; no such traits are but the following trait defines a method `method2`:
no-method-suggested-traits.rs:54:19: 54:19 help: candidate #1: `foo::Bar`
no-method-suggested-traits.rs:58:10: 58:19 error: type `u64` does not implement any method in scope named `method3`
no-method-suggested-traits.rs:58     1u64.method3();
                                          ^~~~~~~~~
no-method-suggested-traits.rs:58:19: 58:19 help: methods from traits can only be called if the trait is implemented and in scope; no such traits are but the following trait defines a method `method3`:
no-method-suggested-traits.rs:58:19: 58:19 help: candidate #1: `no_method_suggested_traits::foo::PubPub`
error: aborting due to 6 previous errors

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.
@huonw huonw force-pushed the trait-suggestions branch from 62e47ad to 0a55aac Compare January 16, 2015 11:54
@nikomatsakis
Copy link
Contributor

Nice, I'll read over the code shortly. I think we should modify the error message to make it clearer what it means to be "in scope" -- like we should say "try adding use foo::bar or something"

@nikomatsakis
Copy link
Contributor

So I had a few nits on huonw@0a55aac but @huonw said he wouldn't be around much over the next two days. So I'm giving r+ anyhow. For the record:

  • we could probably factor out the recursive call in a cleaner way than using a mutable boolean
  • I think the error messages could be rephrased to be more explicit. For example, "the following traits are implemented but not in scope; perhaps add a use statement" or something like that.

Nonetheless, I think this is a big step up in clarify, so I'd like to land.

bors added a commit that referenced this pull request Jan 16, 2015
For a call like `foo.bar()` where the method `bar` can't be resolved,
the compiler will search for traits that have methods with name `bar` to
give a more informative error, providing a list of possibilities.

Closes #7643.
@bors bors merged commit 0a55aac into rust-lang:master Jan 17, 2015
@huonw huonw deleted the trait-suggestions branch January 17, 2015 01:18
bors added a commit that referenced this pull request Jan 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiler should offer suggestions from other implemented traits when methods aren't found
8 participants