Clarify suggetion for field used as method by estebank · Pull Request #40816 · rust-lang/rust

estebank · 2017-03-25T06:46:56Z

Instead of

error: no method named `src_addr` found for type `&wire::ipv4::Repr` in the current scope
   --> src/wire/ipv4.rs:409:34
    |
409 |         packet.set_src_addr(self.src_addr());
    |                                  ^^^^^^^^
    |
note: did you mean to write `self.src_addr`?
   --> src/wire/ipv4.rs:409:34
    |
409 |         packet.set_src_addr(self.src_addr());
    |                                  ^^^^^^^^

present

error: no method named `src_addr` found for type `&wire::ipv4::Repr` in the current scope
   --> src/wire/ipv4.rs:409:34
    |
409 |         packet.set_src_addr(self.src_addr());
    |                                  ^^^^^^^^ field, not a method
    |
    = help: did you mean to write `self.src_addr` instead of `self.src_addr(...)`?

Fix #38321.

rust-highfive · 2017-03-25T06:47:06Z

r? @nikomatsakis

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

Instead of ``` error: no method named `src_addr` found for type `&wire::ipv4::Repr` in the current scope --> src/wire/ipv4.rs:409:34 | 409 | packet.set_src_addr(self.src_addr()); | ^^^^^^^^ | note: did you mean to write `self.src_addr`? --> src/wire/ipv4.rs:409:34 | 409 | packet.set_src_addr(self.src_addr()); | ^^^^^^^^ ``` present ``` error: no method named `src_addr` found for type `&wire::ipv4::Repr` in the current scope --> src/wire/ipv4.rs:409:34 | 409 | packet.set_src_addr(self.src_addr()); | ^^^^^^^^ `src_addr` is a field, not a method | = help: did you mean to write `self.src_addr` instead of `self.src_addr(...)`? ```

nikomatsakis · 2017-03-27T20:24:53Z

I wonder if we should suggest (self.src_addr)(...), at least in the event that the field is callable? It's non-obvious.

nikomatsakis · 2017-03-27T20:25:02Z

But I guess that feels like a separate issue. =)

nikomatsakis · 2017-03-27T20:25:20Z

src/librustc_typeck/check/method/suggest.rs

-                                                                stored in the `{1}` field",
-                                                               expr_string,
-                                                               item_name));
+                                        err.help(&format!("use `({0}.{1})(...)` if you \


oh, we do that already =)

nikomatsakis · 2017-03-27T20:26:07Z

src/librustc_typeck/check/method/suggest.rs

+                                                           stored in the `{1}` field",
+                                                          expr_string,
+                                                          item_name));
+                                        err.span_label(span,


Personally, I think I would give the same span_label either way (i.e., just "is a field, not a method") but change the help text. I think this syntax is a bit long and specific for a label.

I just realized I misread this comment. I'm keeping different labels, but much shortened (field, not a method vs field storing a function, not a method). Is that ok?

personally, I find "field storing a function" confusing. It makes it sound (to me) like this is an error specifically because the field stores a function (where indeed it would be an error for any field).

Changed so that both cases have the same label as originally requested.

nikomatsakis · 2017-03-27T20:29:55Z

src/test/compile-fail/issue-18343.rs

+    o.closure();
+    //~^ ERROR no method named `closure` found
+    //~| HELP use `(o.closure)(...)` if you meant to call the function stored in the `closure` field
+    //~| NOTE `closure` is a field storing a function, not a method


can you change this to a ui test? I'd personally also rename it to something like suggestion-for-field-with-fn-type-issue-18343.rs, since issue-18343.rs seems like a name suitable only for "fix random ICE regression test" to me.

nikomatsakis · 2017-03-27T20:30:14Z

src/test/compile-fail/issue-2392.rs

    o_closure.closure(); //~ ERROR no method named `closure` found
-    //~^ NOTE use `(o_closure.closure)(...)` if you meant to call the function stored
+    //~^ HELP use `(o_closure.closure)(...)` if you meant to call the function stored
+    //~| NOTE `closure` is a field storing a function, not a method


I think this should be a UI test too

nikomatsakis · 2017-03-27T20:31:19Z

src/test/compile-fail/issue-32128.rs

+    demo.example(1);
+    //~^ ERROR no method named `example`
+    //~| HELP use `(demo.example)(...)`
+    //~| NOTE `example` is a field storing a function, not a method


really all of these feel like ui tests to me :)

maybe make a directory like src/test/ui/suggestions/confuse-field-and-method/ and just move the (not renamed) issue-123.rs files into that directory?

nikomatsakis

r=me once you make those ui tests

estebank · 2017-03-28T00:10:59Z

@nikomatsakis all done!

nikomatsakis · 2017-03-29T13:48:00Z

@bors r+

bors · 2017-03-29T13:48:01Z

📌 Commit 872d3bc has been approved by nikomatsakis

Clarify suggetion for field used as method Instead of ```rust error: no method named `src_addr` found for type `&wire::ipv4::Repr` in the current scope --> src/wire/ipv4.rs:409:34 | 409 | packet.set_src_addr(self.src_addr()); | ^^^^^^^^ | note: did you mean to write `self.src_addr`? --> src/wire/ipv4.rs:409:34 | 409 | packet.set_src_addr(self.src_addr()); | ^^^^^^^^ ``` present ```rust error: no method named `src_addr` found for type `&wire::ipv4::Repr` in the current scope --> src/wire/ipv4.rs:409:34 | 409 | packet.set_src_addr(self.src_addr()); | ^^^^^^^^ field, not a method | = help: did you mean to write `self.src_addr` instead of `self.src_addr(...)`? ``` Fix rust-lang#38321.

Rollup of 6 pull requests - Successful merges: #40780, #40814, #40816, #40832, #40901, #40907 - Failed merges:

rust-highfive assigned nikomatsakis Mar 25, 2017

estebank force-pushed the issue-38321 branch 4 times, most recently from e34af47 to 6341a8b Compare March 25, 2017 19:57

nikomatsakis reviewed Mar 27, 2017

View reviewed changes

nikomatsakis requested changes Mar 27, 2017

View reviewed changes

estebank force-pushed the issue-38321 branch from 144ee45 to 3a621fc Compare March 27, 2017 22:06

Simplify labels and move tests to ui

872d3bc

estebank force-pushed the issue-38321 branch from 3a621fc to 872d3bc Compare March 29, 2017 02:43

frewsxcv mentioned this pull request Mar 29, 2017

Rollup of 6 pull requests #40911

Merged

bors added a commit that referenced this pull request Mar 29, 2017

Auto merge of #40911 - frewsxcv:rollup, r=frewsxcv

c82f132

Rollup of 6 pull requests - Successful merges: #40780, #40814, #40816, #40832, #40901, #40907 - Failed merges:

bors merged commit 872d3bc into rust-lang:master Mar 30, 2017

zackmdavis mentioned this pull request Mar 30, 2017

use help rather than span note for no method error; a slight rephrasing #39441

Closed

Uh oh!

Conversation

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

Uh oh!

Uh oh!

rust-highfive commented Mar 25, 2017

Uh oh!

nikomatsakis commented Mar 27, 2017

Uh oh!

nikomatsakis commented Mar 27, 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!

nikomatsakis left a comment

Choose a reason for hiding this comment

Uh oh!

estebank commented Mar 28, 2017

Uh oh!

nikomatsakis commented Mar 29, 2017

Uh oh!

bors commented Mar 29, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

estebank commented Mar 25, 2017 •

edited

Loading