Port clippy lint redundant_field_names to compiler

[fee1-dead]

No description provided.

[rust-highfive]

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

[rust-highfive]

r? @cjgillot

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

[rust-highfive]

⚠️ Warning ⚠️

• These commits modify compiler targets. (See the Target Tier Policy.)

[camsteffen]

I think we should remove the lint from Clippy in the same PR.

The lint really should be named redundant_field_initializers or field_init_same_name or field_init_repeats_name.

Are we okay with uplifting this, warn by default, with the possibility of enhancing it if we allow Struct { &foo }? (I think that's fine).

Can we put this in the nonstandard-style group? It seems like rustc is lacking a "style" group for slightly more picky lints. Someone will say that such lints belong in Clippy, but I'm not so sure. This lint feels like it could be in rustc.

If this lint triggers in macros for $a: $b, that's probably a deal-breaker. Maybe we can detect cases like $a: $a.

[fee1-dead]

Can we put this in the nonstandard-style group?

I don't think so because AFAIK that group is only used for lints with nonstandard naming conventions.

[bors]

☔ The latest upstream changes (presumably #86754) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #86031) made this pull request unmergeable. Please resolve the merge conflicts.

[workingjubilee]

This could be considered in the same category, but it is not an instance of code that seems like it was intended to be used, or reflective of a misunderstanding as with a false invocation of unsafe, it is simply a bit of repetition. This has usually not merited a lint in rustc, and indeed the language actually favors repetition as with e.g. trailing commas where it would make things easier to rename and refactor... as might be a sensible choice here.

Many lints have been uplifted from clippy in the past, and I would honestly be happy to see more. But what I mean is that previously many of the lints that were uplifted from clippy were correctness lints (of which there are even now many that seem like good candidates) and were both motivated and granted on such a reason. Most of the interest I have seen expressed that aim, and your excerpt comes right next to these quotes:

Care should be taken when adding warnings to avoid warning fatigue, and avoid false-positives where there really isn't a problem with the code.

Code that is very likely to be incorrect, dangerous, or confusing, but the language technically allows

Yes, there are projects that have an MSRV below 1.17: serde is one of them.

[Mark-Simulacrum]

@joshtriplett said in this zulip thread that he would like to see clippy lints uplifted.

I am pretty sure that the interpretation here is not that all clippy lints should be uplifted, but rather that approval of lints for uplift via targeted PRs or T-lang MCPs is much more likely to be a successful path toward a decision, as opposed to the prior art at the time of that comment, which was an issue proposing lots of lints to be moved.

Personally, I tend to agree with @workingjubilee that it feels like this lint definitely belongs either in clippy or as an allow-by-default lint; it's primarily a stylistic lint, and doesn't necessarily mean that all people will want to write their code like this. I'd personally be frustrated when seeing a warning on this pattern because the code is equally correct either way and not really significantly more verbose or anything like that.

[camsteffen]

Without a lint category, it doesn't really make sense to uplift. I think rustc should have an allow-by-default category for lints like this:

• clearly more idiomatic
• purely stylistic, non-critical, low-risk
• lints usage of a language feature or prelude items (generally not std API usage)
• the scope of the lint is clear and simple with no perceivable future enhancements

Some candidates: needless_return, write_with_newline, writeln_empty_string, redundant_pattern, redundant_static_lifetimes, single_match, unused_unit

There is a very large gap between rustc lints and Clippy lints in terms of stability, false positive rate, lint pickiness, opinionatedness. Of course some of the "blame" falls squarely on Clippy to fix bugs, remove un-useful lints, etc. But Clippy will likely never have the level of stability that rustc has as it is sort of a playground for new lints. A rustc lint category can bridge that gap a bit for people who want rustc to be just a little more picky, but in a highly stable way.

[workingjubilee]

I understand Clippy has a lot of highly experimental lints, but are we sure there isn't any chance that Clippy could make the first move on growing such a "low stress" mode where it just emits warnings on lints with stronger consensus behind them? It would seem an appropriate "proof of concept" for the idea, at least.

Clippy is often run in CI for Rust projects and if it denies something then CI doesn't pass. And a lot of projects run -D warnings in CI, so if Clippy is pretty unstable on warnings then that is probably a bit of a problem, too. It may be a good idea to more actively rein that in somewhat, or at least use the nursery category a bit more freely when it becomes apparent a lint is more half-baked than expected.

[bors]

☔ The latest upstream changes (presumably #88316) made this pull request unmergeable. Please resolve the merge conflicts.

[nikomatsakis]

@rfcbot fcp close

We discussed in the @rust-lang/lang meeting today. The consensus of the meeting was that we ought to close this PR. We generally prefer to limit uplift to lints that fix known bugs and have a very limited set of "false warnings". This case seems to be a conventions question and we would prefer to keep such work within clippy.

Thanks @fee1-dead for raising the question!

[rfcbot]

Team member @nikomatsakis has proposed to close this. The next step is review by the rest of the tagged team members:

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[nikomatsakis]

One side note: My actual preference would be to remove most style lints but to have rustfmt silently remove and edit cases like this. That way we don't bother our users with it but we also see the edits applied consistently across Rust code. I think that would be best discussed with the @rust-lang/wg-rustfmt team, though, and perhaps even wants an RFC for broader discussion (I think it's a bit of a policy change from how rustfmt does things now).

[calebcartwright]

One side note: My actual preference would be to remove most style lints but to have rustfmt silently remove and edit cases like this. That way we don't bother our users with it but we also see the edits applied consistently across Rust code. I think that would be best discussed with the @rust-lang/wg-rustfmt team, though, and perhaps even wants an RFC for broader discussion (I think it's a bit of a policy change from how rustfmt does things now).

fwiw we do already have a number of these covered via config options in rustfmt (e.g. I gather use_field_init_shorthand could be of use in this case), but they're often turned off by default.

As you alluded to, we don't have the flexibility today to intentionally change rustfmt in such a way that an updated version introduces formatting changes. However, there's at least a couple changes I think we need to try to get pushed through anyway to get rid of some odd formatting behavior that's no longer necessary, and that could potentially provide a window for the some of the other changes you have in mind too (if we do end up introducing formatting changes then they'd ideally come in one batch so users can rustup update && cargo fmt once and be done with it).

Will likely be a topic of discussion at an upcoming dev tools meeting

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[scottmcm]

I'm totally ok with this staying in clippy, so I've ✅'d here.

That said, I wonder if there's a more restricted subset of this that's unambiguous enough to have in rustc, like how unused_parens doesn't actually lint on all unnecessary parens -- we don't lint (1 * 2) + 3, for example.

For example, I might not say that rustc should lint on

   Foo {
       x: foo(),
       y: y,
       z: bar(),
   }

but I would be happy for it to lint on

    Foo { x: x, y: y, z: z }

(like one might see in Foo::new)

[rfcbot]

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.