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

false negative if_same_then_else / new lint catch wrongly ordered struct init shorthand #6352

Closed
matthiaskrgr opened this issue Nov 20, 2020 · 2 comments · Fixed by #6769
Closed
Assignees
Labels
A-lint Area: New lints C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-negative Issue: The lint should have been triggered on code, but wasn't

Comments

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Nov 20, 2020

Upstream rustc issue: rust-lang/rust#79210

This is definitely also clippy territory!
We have the if_same_than_else lint which probably should catch the upstream example, but I wonder if we could have a lint that catches the field ordering being suspicious.

I tried this code:

#[derive(Debug)]
struct S {
    a: i32,
    b: i32,
}

fn main() {
    let a = 0_i32;
    let b = 1_i32;
    let s: S = if false {
        S { a, b }
    } else {
        S { b, a } /* wanted: S {a: b, b: a}*/
    };
    println!("{:?}", s);
}

The problem is that S { a, b } and S { b, a } is essentially the same, but the if_same_then_else lint did not see this (probably because of the field ordering).
Alternatively, I wonder if it makes sense to have a lint that generally warns we use the shorthand init pattern

struct S { a: i32, b: i32, c: i32 }
let a = 1;
let b = 2;
let c = 3;
let s = S { c, a, b }

where the struct members are not listed in the order that they are defined in the struct definition.

clippy 0.0.212 (fe98231 2020-11-19)

@matthiaskrgr matthiaskrgr added C-bug Category: Clippy is not doing the correct thing A-lint Area: New lints labels Nov 20, 2020
@phansch phansch added the I-false-negative Issue: The lint should have been triggered on code, but wasn't label Dec 18, 2020
@camsteffen camsteffen added the E-medium Call for participation: Medium difficulty level problem and requires some initial experience. label Feb 7, 2021
@Y-Nak
Copy link
Contributor

Y-Nak commented Feb 19, 2021

As @matthiaskrgr suggested, I'll add the new lint to fix this issue (and the lint is quite useful regardless of this issue itself).
@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-false-negative Issue: The lint should have been triggered on code, but wasn't
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@matthiaskrgr @phansch @camsteffen @Y-Nak and others