Repository: rust
Issue: Policy for lint expansions
Labels: A-lints,A-diagnostics,T-lang,proposed-final-comment-period,disposition-merge,C-discussion,I-lang-radar

## Background

When a lint is expanded to include many new cases, it adds significant complexity to the rollout of a toolchain in the ecosystem. In particular, certain challenges occur in large codebases. Maintainers are stuck with the choice of

1. Disabling the existing lint while the toolchain is updated and new cases are fixed.
2. Fixing cases manually, and updating the toolchain immediately so that other developers see the warnings and don't add more such code.

Both of these come with the problem of _racing_ with other developers in a codebase who may land new code which triggers the expanded lint in a new compiler, but does _not_ trigger the lint in an old compiler.

While it would be nice to solve this "raciness" once and for all, there are other considerations at play. Instead, we propose a set of guidelines to help decide whether an existing lint should be expanded, as well as mitigations for the above problems when expansions occur.

These guidelines are based on the lang team [discussion](https://hackmd.io/WUApsX9eSiyDMlWxh6xBmA?view#Fallout-from-expansion-of-redundant-import-checking-rust121708) of [#121708].

cc @estebank @petrochenkov

[#121708]: https://github.com/rust-lang/rust/issues/121708

## Guidelines

### Deciding on new lint vs expansion

We don't have hard-and-fast rules for when to use a new lint versus expanding an existing one, but we want to take several factors into account:

- How important is fixing the issue the lint identifies, to the maintainer of the code or the health of the ecosystem?
- Are the cases of the old and new lints of the same severity?
- Is it possible to fix code to no longer trigger the lint without making the code depend on the new version of Rust?
- Is there a clear description of the distinction between the old and new cases, such that it's easy to give a logical name to the new cases?
- What fraction of the ecosystem will encounter the lint? (Crater can give a good idea of this.)
- Is there an automatic machine-applicable fix?

We'd like to avoid having a disproportionately painful impact on the ecosystem compared to the value provided. We'd also like to avoid having a painful impact on large codebases that need to migrate their code to take the new lint into account.

When proposing or reviewing an expansion of an existing lint, please take the above factors into account. If in doubt about whether to use a new or existing lint, please consult with lang, and seek a decision via FCP.

### Mitigations for lint expansions

When an existing lint is expanded to include many new cases, please consider providing at least one of:

1. A new lint name under the existing group, so that users may opt out of the expansion at least temporarily. This is particularly desirable if there's a clear description of the new cases being added.
2. A MachineApplicable fix for the lint. This fix must produce code that compiles without _new_ lints or errors under the most recent stable compiler. This helps maintainers, particularly of large codebases, quickly fix all existing instances so they can upgrade/deny the lint, without racing with new code additions that trigger the lint. (Note, though, that a MachineApplicable fix doesn't suffice if applying the fix would make code fail to compile on the immediately prior version of Rust.)

To evaluate whether a lint produces "many new cases", try doing a crater run on the top 1000 crates. If the lint addition impacts more than 1% of the top-1000 crates on crates.io, or is producing a large number of warnings within a small number of crates in that set, treat it as producing many new cases. See below for details on how to run crater.

A crater run is not required before landing for every lint expansion. Reviewers should use their best judgment to decide if one is required.

#### Crater command

To measure the impact of a lint on the top 1000 crates, you can use the following crater command:

`
@craterbot run name=<name> start=master#<hash1>+rustflags=-D<lint_name> end=master#<hash2>+rustflags=-D<lint_name> crates=top-1000 mode=check-only p=1
`

See the [crater docs](https://github.com/rust-lang/crater/blob/master/docs/bot-usage.md#tutorial-creating-an-experiment-for-a-pr) for more information.

## Lint revert policy

In general, if a lint expansion does not meet the above guidelines, and in particular *especially* if it has neither a new lint name nor a machine applicable fix, it should be flagged for review and likely reverted before it hits stable Rust.