Skip to content

Clarify suggetion for field used as method #40816

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.

Sign up for GitHub

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 2 commits into from
Mar 30, 2017
Merged

Clarify suggetion for field used as method #40816

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6341a8b
Clarify suggetion for field used as method
estebank Mar 25, 2017
872d3bc
Simplify labels and move tests to ui
estebank Mar 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/librustc_typeck/check/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,23 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let field_ty = field.ty(tcx, substs);

if self.is_fn_ty(&field_ty, span) {
err.span_note(span,
&format!("use `({0}.{1})(...)` if you \
meant to call the function \
stored in the `{1}` field",
expr_string,
item_name));
err.help(&format!("use `({0}.{1})(...)` if you \
Copy link
Contributor

Choose a reason for hiding this comment

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

oh, we do that already =)

meant to call the function \
stored in the `{1}` field",
expr_string,
item_name));
err.span_label(span,
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

&format!("`{}` is a field storing a \
function, not a method",
item_name));
} else {
err.span_note(span,
&format!("did you mean to write `{0}.{1}`?",
expr_string,
item_name));
err.help(&format!("did you mean to write `{0}.{1}` \
instead of `{0}.{1}(...)`?",
expr_string,
item_name));
err.span_label(span,
&format!("`{}` is a field, not a method",
item_name));
}
break;
}
Expand Down
6 changes: 4 additions & 2 deletions src/test/compile-fail/issue-18343.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ struct Obj<F> where F: FnMut() -> u32 {

fn main() {
let o = Obj { closure: || 42 };
o.closure(); //~ ERROR no method named `closure` found
//~^ NOTE use `(o.closure)(...)` if you meant to call the function stored in the `closure` field
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
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

}
39 changes: 26 additions & 13 deletions src/test/compile-fail/issue-2392.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,45 +48,58 @@ fn main() {

let o_closure = Obj { closure: || 42, not_closure: 42 };
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
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be a UI test too


o_closure.not_closure(); //~ ERROR no method named `not_closure` found
//~^ NOTE did you mean to write `o_closure.not_closure`?
o_closure.not_closure();
//~^ ERROR no method named `not_closure` found
//~| NOTE `not_closure` is a field, not a method
//~| HELP did you mean to write `o_closure.not_closure` instead of `o_closure.not_closure(...)`?

let o_func = Obj { closure: func, not_closure: 5 };
o_func.closure(); //~ ERROR no method named `closure` found
//~^ NOTE use `(o_func.closure)(...)` if you meant to call the function stored
//~^ HELP use `(o_func.closure)(...)` if you meant to call the function stored
//~| NOTE `closure` is a field storing a function, not a method

let boxed_fn = BoxedObj { boxed_closure: Box::new(func) };
boxed_fn.boxed_closure();//~ ERROR no method named `boxed_closure` found
//~^ NOTE use `(boxed_fn.boxed_closure)(...)` if you meant to call the function stored
//~^ HELP use `(boxed_fn.boxed_closure)(...)` if you meant to call the function stored
//~| NOTE `boxed_closure` is a field storing a function, not a method

let boxed_closure = BoxedObj { boxed_closure: Box::new(|| 42_u32) as Box<FnBox() -> u32> };
boxed_closure.boxed_closure();//~ ERROR no method named `boxed_closure` found
//~^ NOTE use `(boxed_closure.boxed_closure)(...)` if you meant to call the function stored
//~^ HELP use `(boxed_closure.boxed_closure)(...)` if you meant to call the function stored
//~| NOTE `boxed_closure` is a field storing a function, not a method

// test expression writing in the notes

let w = Wrapper { wrap: o_func };
w.wrap.closure();//~ ERROR no method named `closure` found
//~^ NOTE use `(w.wrap.closure)(...)` if you meant to call the function stored
//~^ HELP use `(w.wrap.closure)(...)` if you meant to call the function stored
//~| NOTE `closure` is a field storing a function, not a method

w.wrap.not_closure();//~ ERROR no method named `not_closure` found
//~^ NOTE did you mean to write `w.wrap.not_closure`?
w.wrap.not_closure();
//~^ ERROR no method named `not_closure` found
//~| NOTE `not_closure` is a field, not a method
//~| HELP did you mean to write `w.wrap.not_closure` instead of `w.wrap.not_closure(...)`?

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

impl FuncContainerOuter {
fn run(&self) {
unsafe {
(*self.container).f1(1); //~ ERROR no method named `f1` found
//~^ NOTE use `((*self.container).f1)(...)`
//~^ HELP use `((*self.container).f1)(...)`
//~| NOTE `f1` is a field storing a function, not a method
(*self.container).f2(1); //~ ERROR no method named `f2` found
//~^ NOTE use `((*self.container).f2)(...)`
//~^ HELP use `((*self.container).f2)(...)`
//~| NOTE `f2` is a field storing a function, not a method
(*self.container).f3(1); //~ ERROR no method named `f3` found
//~^ NOTE use `((*self.container).f3)(...)`
//~^ HELP use `((*self.container).f3)(...)`
//~| NOTE `f3` is a field storing a function, not a method
}
}
}
6 changes: 4 additions & 2 deletions src/test/compile-fail/issue-32128.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ fn main() {
})
};

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

Choose a reason for hiding this comment

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

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?

// (demo.example)(1);
}
9 changes: 6 additions & 3 deletions src/test/compile-fail/issue-33784.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,15 @@ fn main() {
let o = Obj { fn_ptr: empty, closure: || 42 };
let p = &o;
p.closure(); //~ ERROR no method named `closure` found
//~^ NOTE use `(p.closure)(...)` if you meant to call the function stored in the `closure` field
//~^ HELP use `(p.closure)(...)` if you meant to call the function stored in the `closure` field
//~| NOTE `closure` is a field storing a function, not a method
let q = &p;
q.fn_ptr(); //~ ERROR no method named `fn_ptr` found
//~^ NOTE use `(q.fn_ptr)(...)` if you meant to call the function stored in the `fn_ptr` field
//~^ HELP use `(q.fn_ptr)(...)` if you meant to call the function stored in the `fn_ptr` field
//~| NOTE `fn_ptr` is a field storing a function, not a method
let r = D(C { c_fn_ptr: empty });
let s = &r;
s.c_fn_ptr(); //~ ERROR no method named `c_fn_ptr` found
//~^ NOTE use `(s.c_fn_ptr)(...)` if you meant to call the function stored in the `c_fn_ptr`
//~^ HELP use `(s.c_fn_ptr)(...)` if you meant to call the function stored in the `c_fn_ptr`
//~| NOTE `c_fn_ptr` is a field storing a function, not a method
}