Skip to content
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

Fix extra non_snake_case warning for shorthand field bindings #89473

Merged
merged 1 commit into from
Oct 5, 2021

Conversation

FabianWolff
Copy link
Contributor

Fixes #89469. The problem is the innermost if condition here:

fn check_pat(&mut self, cx: &LateContext<'_>, p: &hir::Pat<'_>) {
if let PatKind::Binding(_, hid, ident, _) = p.kind {
if let hir::Node::Pat(parent_pat) = cx.tcx.hir().get(cx.tcx.hir().get_parent_node(hid))
{
if let PatKind::Struct(_, field_pats, _) = &parent_pat.kind {
for field in field_pats.iter() {
if field.ident != ident {
// Only check if a new name has been introduced, to avoid warning
// on both the struct definition and this pattern.
self.check_snake_case(cx, "variable", &ident);
}
}
return;
}
}
self.check_snake_case(cx, "variable", &ident);
}
}

This code runs for every PatKind::Binding, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to self.check_snake_case() will indeed be skipped in the check_pat() call for A; but when check_pat() is called for B, the loop will still iterate over A, and field.ident (= A) != ident (= B) will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that check_pat() was actually called for.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2021
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 4, 2021

📌 Commit 9626f2b has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 4, 2021
Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 5, 2021
…ingjubilee

Rollup of 15 pull requests

Successful merges:

 - rust-lang#87993 (Stabilize try_reserve)
 - rust-lang#88090 (Perform type inference in range pattern)
 - rust-lang#88780 (Added abs_diff for integer types.)
 - rust-lang#89270 (path.push() should work as expected on windows verbatim paths)
 - rust-lang#89413 (Correctly handle supertraits for min_specialization)
 - rust-lang#89456 (Update to the final LLVM 13.0.0 release)
 - rust-lang#89466 (Fix bug with query modifier parsing)
 - rust-lang#89473 (Fix extra `non_snake_case` warning for shorthand field bindings)
 - rust-lang#89474 (rustdoc: Improve doctest pass's name and module's name)
 - rust-lang#89478 (Fixed numerus of error message)
 - rust-lang#89480 (Add test for issue 89118.)
 - rust-lang#89487 (Try to recover from a `=>` -> `=` or `->` typo in a match arm)
 - rust-lang#89494 (Deny `where` clauses on `auto` traits)
 - rust-lang#89511 (:arrow_up: rust-analyzer)
 - rust-lang#89536 (update Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
@bors bors merged commit c2bfe45 into rust-lang:master Oct 5, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extra non_snake_case warning when punning field name in pattern
6 participants