Turn old edition lint (anonymous-parameters) into warn-by-default on 2015

[Manishearth]

This makes anonymous_parameters and keyword_idents warn-by-default on the 2015 edition. I would also like to do this for absolute_paths_not_starting_with_crate, but I feel that case is slightly less clear-cut.

Note that this only affects code on the 2015 edition, such code is illegal in future editions anyway.

This was spurred by dtolnay/syn#972: old edition syntax breaks tooling (like syn), and while the tooling should be free to find its balance on how much to support prior editions, it does seem like we should be nudging such code towards the newer edition, and we can do that by turning this Allow lint into a Warn.

In general, I feel like migration lints from an old edition should be made Warn after a year or so, and idiom lints for the new edition should be made Warn after a couple months.

cc @m-ou-se, this is for stuff from the 2015-2018 migration but you might be interested.

[rust-highfive]

r? @oli-obk

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

[Mark-Simulacrum]

cc @rust-lang/project-edition-2021

[Manishearth]

keyword_idents will warn on a lot of instances of try, so perhaps we should reconsider that one. For now pared down to anonymous_parameters only.

[oli-obk]

I guess this needs a @rust-lang/lang FCP

You also need to bless the ui tests.

[Manishearth]

Yeah I plan to, I didn't write this PR on a computer I do builds on

[Manishearth]

I'm also interested if the lang team thinks keyword_idents or absolute_paths_not_starting_with_crate should be Warn too.

[bors]

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

[nikomatsakis]

So this intersects with the work that we've been doing on writing up our general lint policy.

The argument of "newer tooling maybe not accept this" is an argument in favor of "one Rust" -- that is, generally pushing users on all editions to write code that is forward-compatible with future editions, even if they don't plan to migrate.

I think there is no controversy about doing that in cases where the code pattern is deprecated because it's considered harmful, bug-prone, etc. We've been trending towards avoiding it in cases where some aspect of the code pattern is changing in a new edition but the pattern is otherwise fine (for example, I would not want to warn about closures whose meaning might change in Rust 2021 except as part of migrating to Rust 2021).

So the question here I guess is whether we consider syntactic things like this harmful. I'd not call it "bug prone", but there is a definite cost to tooling, and perhaps a bit of a hazard for users who may be surprised when tooling doesn't accept their code or work well with it.

[Manishearth]

It's worth pointing out that we typically do not have a huge bar for adding new warnings. That's all this does, this will add a warning for crates that didn't have one; to me it seems okay to start doing stronger nudges on this issue. In 2018 there was a vague plan that some of the Allow migration lints would be upped in strength after a year or so, so it's not like this hasn't been discussed already. So I'm not sure if the harmful standard-of-proof applies here; I think "harmful" applies to hard errors and deny-by-default lints, but we're not doing either of those here.

But yeah absolutely it intersects with the work around edition lints in general.

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting. We're in favor of making this lint warn-by-default in 2015, both for future compatibility and because we want to deprecate the syntax for the benefit of tooling.

The lang team doesn't want this to be taken as a generalized precedent for arbitrary lint/error changes in previous editions; we're not necessarily always going to be in favor of adding warnings or (especially) errors in earlier editions. We're in favor in this case because this is already a hard error in the current edition, and this seems unlikely to generate a massive number of warnings in existing code.

[joshtriplett]

@rfcbot merge

[Manishearth]

Makes sense. I'm going to take that as a signal of "possibly, but it's complicated" and not spend time looking into it further.

[oli-obk]

The impl looks good to me, so this is just waiting on the FCP to finish, which I'm not sure I'll be around for. Feel free to r=me once the FCP finishes

[rfcbot]

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

[rfcbot]

The final comment period, with a disposition to merge, 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.

The RFC will be merged soon.

[Manishearth]

@bors r=oli-obk

[bors]

📌 Commit 664c3e7 has been approved by oli-obk

[ehuss]

I just wanted to note (for anyone investigating this in the future) that this hits the same issue as noted in #83213 (comment). If someone is using 2015, hits this warning and decides to allow it, then it will cause problems if they ever decide to transition to 2018. That's a pretty small audience, so I don't think it is a big concern, but I think it would be good to be careful when doing things like this until the ability for cargo fix --edition to override the level is addressed.

[Manishearth]

@ehuss Yep! I think in this case that is less of an issue because this is being landed after the new edition; the other PR is taking an existing lint and making it an edition concern.

[bors]

⌛ Testing commit 664c3e7 with merge 11d0528...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 11d0528 to master...