-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
typeck: if a private field exists, also check for a public method #33342
Conversation
r? @Aatch (rust_highfive has picked a reviewer for you, use r? to override) |
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
I believe there was some discussion in internals.rust-lang.org about making these kinds of messages |
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
@bors: r- another commit got added here, I don't know why bors still thought it was reviewed |
It also appears to have failed the rollup http://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt/builds/4104/steps/compile/logs/stdio |
@steveklabnik I didn't add another commit, I rebased and fixed up the existing after seeing that the rollup failed. |
// Also check if an accessible method exists, which is often what is meant. | ||
if method::exists(fcx, field.span, field.node, expr_t, expr.id, false) { | ||
err.note(&format!("a method called `{}` also exists, \ | ||
maybe a `()` to call it is missing?", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: a method named
{} or just `a method `{}
Also, I would prefer perhaps you wish to call it
as in this error message (instead of maybe a
() to call it is missing?
).
r=me with the nit addressed |
r? @jseyfried |
Thanks! |
📌 Commit 84034d4 has been approved by |
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
☔ The latest upstream changes (presumably #33425) made this pull request unmergeable. Please resolve the merge conflicts. |
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
typeck: if a private field exists, also check for a public method For example, `Vec::len` is both a field and a method, and usually encountering `vec.len` just means that the parens were forgotten. Fixes: rust-lang#26472 NOTE: I added the parameter `allow_private` to `method::exists` since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it to `false` as well, but I wanted to preserve compatibility for this case.
For example,
Vec::len
is both a field and a method, and usually encounteringvec.len
just means that the parens were forgotten.Fixes: #26472
NOTE: I added the parameter
allow_private
tomethod::exists
since I don't want to suggest inaccessible methods. For the second case, where only the method exists, I think it would make sense to set it tofalse
as well, but I wanted to preserve compatibility for this case.