Add version mismatch help message for unimplemented trait #66561
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cramertj (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
| @@ -0,0 +1,21 @@ | |||
| // aux-build:crate_a1.rs | |||
| // aux-build:crate_a2.rs | |||
There was a problem hiding this comment.
There's one new test for the case where the diagnostic is correct. Do we also want testcases that verify that the diagnostic is not emitted?
E.g. there exists a trait with same path but it's not implemented for the struct.
There was a problem hiding this comment.
Negative tests are always good to have, particularly because incorrect suggestions are sometimes worse than no suggestions. Can you create one for the case you mention?
There was a problem hiding this comment.
I'm adding a few cases:
- A type that doesn't implement any of the two traits
- A type that doesn't implement any of the two traits, but a different variant of the type implements the wrong trait
- A type that doesn't implement any of the two traits, but a different variant of the type implements the correct trait
| @@ -0,0 +1,21 @@ | |||
| // aux-build:crate_a1.rs | |||
| // aux-build:crate_a2.rs | |||
There was a problem hiding this comment.
Negative tests are always good to have, particularly because incorrect suggestions are sometimes worse than no suggestions. Can you create one for the case you mention?
Issue rust-lang#22750 The error reporting for E0277 (the trait `X` is not implemented for `Foo`) now checks whether `Foo` implements a trait with the same path as `X`, which probably means that the programmer wanted to actually use only one version of the trait `X` instead of the two.
TimoFreiberg
force-pushed
the
trait-name-report
branch
from
9b41879
to
2a0292f
Compare
|
@bors r+ |
|
📌 Commit 2a0292f has been approved by |
bors
added
S-waiting-on-bors
Add version mismatch help message for unimplemented trait Improves issue #22750 The error reporting for E0277 (the trait `X` is not implemented for `Foo`) now checks whether `Foo` implements a trait with the same path as `X`, which probably means that the programmer wanted to actually use only one version of the trait `X` instead of the two. Still open: * the same diagnostic should be added for [the trait method case](#22750 (comment)) * Showing the real crate versions would be nice, but rustc currently doesn't have that information [according to Esteban](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/diagnostics.20for.20crate.20version.20mismatch/near/180572989)
|
☀️ Test successful - checks-azure |
|
📣 Toolstate changed by #66561! Tested on commit 0f6f66f. 🎉 rustc-guide on linux: test-fail → test-pass (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra). |
Tested on commit rust-lang/rust@0f6f66f. Direct link to PR: <rust-lang/rust#66561> 🎉 rustc-guide on linux: test-fail → test-pass (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).
|
Hi! :-) The diagnostic suggestion added here isn't being shown in my case - I don't know whether it comes under the things known to not be implemented, or whether this is unexpected - I've filed #89143. |
Improves issue #22750
The error reporting for E0277 (the trait
Xis not implemented forFoo)now checks whether
Fooimplements a trait with the same path asX,which probably means that the programmer wanted to actually use only one
version of the trait
Xinstead of the two.Still open: