-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
Typo suggestion for a variable with a name similar to struct fields #97240
Typo suggestion for a variable with a name similar to struct fields #97240
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
@bors r+ rollup |
📌 Commit 42e9f2d has been approved by |
Er, can I take some time to review this @Dylan-DPC? @bors r- |
Anyways, this looks mostly good, except is there a reason you only suggest if we have a matching field? This is a case where we have a matching method, but local variable also matches: struct Foo;
trait Config {
fn config(self);
fn something(self);
}
impl Config for Foo {
fn config() {}
fn something(self) {
let cofig = 1;
println!("{cofig}");
}
} What does the blessed tests look like with the suggestion moved to like line 500, where the old code would |
| | ||
help: you might have meant to use the available field | ||
| | ||
LL | println!("{self.config}"); |
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.
Drive-by comment: this probably shouldn't be emitted, as it won't compile.
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.
Yeah, I think that's a bit orthogonal to this diff, though. It might be possible to detect this case from HIR, but not related to the suggestion added here.
@TaKO8Ki, I would appreciate if you find some time to investigate suppressing this. Don't block this PR on that if it's more difficult than it seems -- if not immediately easy to fix, then could you file an issue and downgrade the suggestion from MachineApplicable
to MaybeIncorrect
? 😅
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.
I would appreciate if you find some time to investigate suppressing this. Don't block this PR on that if it's more difficult than it seems -- if not immediately easy to fix, then could you file an issue and downgrade the suggestion from MachineApplicable to MaybeIncorrect? 😅
Ok. I will investigate it.
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.
I opened #97311. Can I work on this problem in a new PR?
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.
Yeah, sounds good.
There are not any specific reasons. I will implement typo suggestions for all |
Sounds good. If there are too many false suggestions due to enabling it for the other |
@compiler-errors |
@bors r+ |
📌 Commit 39caed0 has been approved by |
Rollup of 5 pull requests Successful merges: - rust-lang#97240 (Typo suggestion for a variable with a name similar to struct fields) - rust-lang#97289 (Lifetime variance fixes for clippy) - rust-lang#97290 (Turn on `fast_submodules` unconditionally) - rust-lang#97336 (typo) - rust-lang#97337 (Fix stabilization version of `Ipv6Addr::to_ipv4_mapped`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
closes #97133