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

Question re inconsistent_struct_constructor requirement #11846

Closed
smoelius opened this issue Nov 20, 2023 · 5 comments · Fixed by #13737
Closed

Question re inconsistent_struct_constructor requirement #11846

smoelius opened this issue Nov 20, 2023 · 5 comments · Fixed by #13737

Comments

@smoelius
Copy link
Contributor

The lint's documentation states (emphasis added):

Checks for struct constructors where all fields are shorthand and the order of the field init shorthand in the constructor is inconsistent with the order in the struct definition.

What is the reason for requiring that all fields be shorthand? On the face of it, I could see value in additionally flagging fields with initializers.

@ronnodas
Copy link

ronnodas commented Nov 21, 2023

Something like

struct MyStruct {
    vector: Vec<u32>,
    length: usize
}
fn main() {
    let vector = vec![1,2,3];
    MyStruct { length: vector.len(), vector};
}

won't compile if you switch the order of fields.

@smoelius1
Copy link

I think maybe I was being unclear. If the example is changed as below, Clippy flags it (playground). I'm suggesting the unmodified example be flagged as well.

struct MyStruct {
    vector: Vec<u32>,
    length: usize
}
fn main() {
    let vector = vec![1,2,3];
    let length = vector.len();
    MyStruct { length, vector};
}

@smoelius
Copy link
Contributor Author

I would like to remove the "all fields are shorthand" requirement. How should I proceed?

  1. Remove the requirement altogether.
  2. Make it configurable with the current behavior as the default.
  3. Make it configurable with no "all fields are shorthand" requirement as the default.

@ronnodas
Copy link

Note that the lint is currently MachineApplicable. Do you suggest that the "fixed" version of the example in my previous comment should be the following?

struct MyStruct {
    vector: Vec<u32>,
    length: usize
}
fn main() {
    let vector = vec![1,2,3];
    let length = vector.len();
    MyStruct { vector, length };
}

What if there is already a variable length in scope? Even if not, it is not clear to me that defining an extra variable is in general better than having an inconsistent ordering of the fields so I disagree with option 1 (and maybe also option 3).

@smoelius
Copy link
Contributor Author

Note that the lint is currently MachineApplicable. Do you suggest that the "fixed" version of the example in my previous comment should be the following? ...

No, sorry, I did not mean to imply that. But I see you're point. It sounds like 2 may be the best option.

github-merge-queue bot pushed a commit that referenced this issue Dec 27, 2024
…uirement configurable (#13737)

Fixes #11846.

This PR has three commits:
- The first commit adds an `initializer-suggestions` configuration to
control suggestion applicability when initializers are present. The
following are the options:
  - "none": do not suggest
- "maybe-incorrect": suggest, but do not apply suggestions with `--fix`
  - "machine-applicable": suggest and apply suggestions with `--fix`
- The second commit fixes suggestions to handle field attributes
(problem [noticed by
@samueltardieu](#13737 (comment))).
- The third commit adds `initializer-suggestions = "machine-applicable"`
to Clippy's `clippy.toml` and applies the suggestions. (Nothing seems to
break.)

---

changelog: make `inconsistent_struct_constructor` "all fields are
shorthand" requirement configurable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants