Use the proper term when using non-existing variant by estebank · Pull Request #46024 · rust-lang/rust

estebank · 2017-11-16T02:58:40Z

When using a non-existing variant, function or associated item, refer to
the proper term, instead of defaulting to "associated item" in
diagnostics.

Fix#28972.

error[E0599]: no variant named `Quux` found for type `Foo` in the current scope
--> file.rs:7:9
|
7 | Foo::Quux(..) =>(),
| ^^^^^^^^^^^^^

When using a non-existing variant, function or associated item, refer to the proper term, instead of defaulting to "associated item" in diagnostics.

estebank · 2017-11-16T07:27:36Z

The output of the pointing to the source ADT that doesn't have the variant/method/field doesn't really work that well for some types. I'm gonna expand on it to keep std out of it.

estebank · 2017-11-16T07:28:28Z

+ ::: $DIR/auxiliary/no_method_suggested_traits.rs
+ |
+14 | pub enum Bar { X }
+ | ------------ method `method3` not found here


It should be method method2 not found for this

estebank · 2017-11-16T07:28:55Z

+ ::: /checkout/src/liballoc/rc.rs
+ |
+284 | pub struct Rc<T: ?Sized> {
+ | ------------------------ method `method3` not found for this


We should avoid pointing at ADTs in std.

Pointing to any other crate isn't very helpful imo. std isn't special. Just giving the full module path to them should suffice.

estebank · 2017-11-16T17:27:23Z

@oli-obk I don't know if this will actually do what I want it to do. After the test suite runs I'll see. If the output is still suboptimal, I'll leave the PR with only the first commit for merging.

Looks on travis like it's doing the right thing now

petrochenkov · 2017-11-18T22:45:07Z

 let xe1 = XEmpty1(); //~ ERROR expected function, found struct `XEmpty1`
- let xe3 = XE::Empty3; //~ ERROR no associated item named `Empty3` found for type
- let xe3 = XE::Empty3(); //~ ERROR no associated item named `Empty3` found for type
+ let xe3 = XE::Empty3; //~ ERROR no variant named `Empty3` found for type


In this context nothing suggests for sure that Empty3 is a variant and not an associated constant or method.
At the same time, in other contexts like tuple struct patterns match { E::X(..) => {} } we can say for sure that X is a variant, but in this PR diagnostics are not context-dependent, only guessing based on case etc is used.
What I'd want to see is a context-dependent message worded and formatted uniformly with other "expected, (not) found" resolution diagnostics.
This is a bit vague and this PR is still an improvement, so I won't block this PR on these changes.
(I'm basically adding a "rewrite this from scratch" item in my TODO list now.)

petrochenkov · 2017-11-18T22:45:45Z

+ } else {
+ match (item_name.as_str().chars().next(), actual.is_fresh_ty()) {
+ (Some(name), false) if name.is_lowercase() => {
+ "function or associated item"


Could you add tests for all these new variations of diagnostics?

petrochenkov · 2017-11-18T22:46:36Z

r=me after adding tests (#46024 (comment))

kennytm · 2017-11-22T11:25:01Z

Hi @estebank, could you add the tests as requested by @petrochenkov in #46024 (review)?

estebank

Added tests cases.

@bors r=petrochenkov

estebank · 2017-11-22T17:41:52Z

+ } else {
+ match (item_name.as_str().chars().next(), actual.is_fresh_ty()) {
+ (Some(name), false) if name.is_lowercase() => {
+ "function or associated item"


kennytm · 2017-11-22T17:46:07Z

@bors r=petrochenkov

These commands don't work in review comments 😄

bors · 2017-11-22T17:46:09Z

📌 Commit c90b26d has been approved by petrochenkov

kennytm · 2017-11-22T21:17:08Z

@bors r-

Trailing whitespace in the tests.

[00:04:28] tidy error: /checkout/src/test/compile-fail/issue-23173.rs:33: trailing whitespace

kennytm · 2017-11-22T21:18:10Z

(This line)

estebank · 2017-11-23T02:39:25Z

@kennytm fixed.
@bors r=petrochenkov

bors · 2017-11-23T02:39:25Z

📌 Commit b450aff has been approved by petrochenkov

bors · 2017-11-23T05:53:15Z

⌛ Testing commit b450aff with merge a6031a2...

Use the proper term when using non-existing variant When using a non-existing variant, function or associated item, refer to the proper term, instead of defaulting to "associated item" in diagnostics. Fix#28972. ``` error[E0599]: no variant named `Quux` found for type `Foo` in the current scope --> file.rs:7:9 | 7 | Foo::Quux(..) =>(), | ^^^^^^^^^^^^^ ```

bors · 2017-11-23T08:20:28Z

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing a6031a2 to master...

estebank force-pushed the no-variant branch from 6365590 to 2a97508Compare November 16, 2017 04:38

Use the proper term when using non-existing variant
6a972cd
When using a non-existing variant, function or associated item, refer to the proper term, instead of defaulting to "associated item" in diagnostics.

estebank force-pushed the no-variant branch from 2a97508 to 6a972cdCompare November 16, 2017 04:39

estebank changed the title ~~[WIP] Use the proper term when using non-existing variant~~Use the proper term when using non-existing variantNov 16, 2017

estebank force-pushed the no-variant branch from 72f4b8f to adda916Compare November 16, 2017 06:25

petrochenkov self-assigned this Nov 16, 2017

Point to ADT definition when not finding variant, method, assoc type
f796fcd

estebank force-pushed the no-variant branch from adda916 to f796fcdCompare November 16, 2017 07:25

estebank commented Nov 16, 2017
View reviewed changes

estebank force-pushed the no-variant branch from f796fcd to 6a972cdCompare November 16, 2017 17:12

estebank commented Nov 16, 2017
View reviewed changes

estebank force-pushed the no-variant branch from b42132a to 291b77aCompare November 16, 2017 22:50

Use local spans only
0559107

estebank force-pushed the no-variant branch from 291b77a to 0559107Compare November 17, 2017 00:21

petrochenkov reviewed Nov 18, 2017
View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2017

estebank commented Nov 22, 2017
View reviewed changes

kennytm reviewed Nov 22, 2017
View reviewed changes

Comment threadsrc/test/compile-fail/issue-23173.rs Outdated
kennytmNov 22, 2017
Copy link
Copy Markdown
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This line)

Add test to new branches
b450aff

estebank force-pushed the no-variant branch from c90b26d to b450affCompare November 23, 2017 02:37

bors merged commit b450aff into rust-lang:masterNov 23, 2017

bors mentioned this pull request Nov 23, 2017
Check //~ERROR comments in ui tests #46116
Merged

SNCPlay42 mentioned this pull request Dec 8, 2018
E0599 assumes associated items of enums are variants #56517
Closed

estebank deleted the no-variant branch November 9, 2023 05:25

Uh oh!

Uh oh!

Conversation

estebank commented Nov 16, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

estebank commented Nov 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Nov 18, 2017

Uh oh!

kennytm commented Nov 22, 2017

Uh oh!

estebank left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kennytm commented Nov 22, 2017

Uh oh!

bors commented Nov 22, 2017

Uh oh!

kennytm commented Nov 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebank commented Nov 23, 2017

Uh oh!

bors commented Nov 23, 2017

Uh oh!

bors commented Nov 23, 2017

Uh oh!

bors commented Nov 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

estebank commented Nov 16, 2017•
edited
Loading