blob: 9f1fdbce8a86efeafa8933589863e0512ea63533 [file] [view] [edit]
# Proposing Lint Rules
Rule suggestions can start out as brief issues. If a maintainer thinks the rule could be a good
addition to Ruff, they will apply the `needs-design` label to suggest filling out the steps
described below in support of the rule's acceptance. Some of this information will be easier to
obtain with a working rule implementation and doesn't need to be present in the initial proposal.
A design for a new lint rule in Ruff should include the following components:
- A proposed name that follows our [rule naming convention](#rule-naming-convention)
- A proposed category that follows our [rule categorization guidelines](#rule-categorization-guidelines)
- A draft of the rule documentation with the following sections:
- "What it does": a one line description of what the rule checks
- "Why is this bad?": a longer explanation of the pattern flagged by the rule and why it causes problems in real projects
- "Example": a code example showing the problematic code, as well as a code block showing the fixed code
Some rules benefit from additional documentation. These sections usually emerge through the
implementation process and aren't required in a design proposal. "Fix safety" and "Options"
sections are required for rules with unsafe fixes and that rely on any settings, respectively,
but the rest of these are fully optional. You can `grep` for each heading to see where they are
often used.
- "Known problems": any known limitations of the rule, such as false positives or negatives
- "Fix availability": if the rule only has an autofix in some cases, explain why
- "Fix safety": if the rule’s fix is ever unsafe, explain why
- "Options": if the rule depends upon any configuration options, list them
- "See also": if there are other similar or synergistic rules, list them
- "References": if there are any relevant external references to Python or other documentation, list them
A few examples of great rule documentation include [`mutable-argument-default` (`B006`)][b006],
[`quoted-annotation` (`UP037`)][up037], and [`used-dummy-variable` (`RUF052`)][ruf052].
- An example diagnostic including the proposed name, primary message, and fix title (if applicable)
This is another nice bonus that isn't required for a design proposal but concisely reveals a lot
of helpful information about a rule. For example:
```markdown
my-new-rule: primary diagnostic message
--> example.py:1:1
1 | import math
| ^^^^
help: fix title
```
When choosing a diagnostic range (marked by `^^^^` above), also consider that the start of the
range determines where `noqa` comments will be valid
## Rule naming convention
Like Clippy, Ruff's rule names should make grammatical and logical sense when read as "ignore
${rule}" or "ignore ${rule} items", as in the context of suppression comments.
For example, `AssertFalse` fits this convention: it flags `assert False` statements, and so a
suppression comment would be framed as "ignore `assert False`".
As such, rule names should...
- Highlight the pattern that is being linted against, rather than the preferred alternative.
For example, `AssertFalse` guards against `assert False` statements.
- _Not_ contain instructions on how to fix the violation, which instead belong in the rule
documentation and the `fix_title`.
- _Not_ contain a redundant prefix, like `Disallow` or `Banned`, which are already implied by the
convention.
When re-implementing rules from other linters, we prioritize adhering to this convention over
preserving the original rule name.
## Rule categorization guidelines
Choosing a category is a crucial part of the rule proposal and acceptance process. To paraphrase the
[Clippy documentation](https://rust-lang.github.io/rfcs/2476-clippy-uno.html#what-lints-belong-in-clippy),
if a rule doesn't fit in the categories, it probably doesn't fit in Ruff. Descriptions of each category can
be found in the [rule category documentation](linter.md#rule-categories),
but the flow chart below is intended to facilitate category assignment.
```mermaid
---
config:
flowchart:
nodeSpacing: 20
rankSpacing: 25
padding: 8
themeVariables:
fontSize: 13px
---
flowchart TD
A("Formatting, security,<br/>or language restriction?")
A -->|Yes| B["Formatting · Security · Restriction"]
A -->|No| C("Too noisy or opinionated?")
C -->|Yes| D["Pedantic"]
C -->|No| E("Incorrect or deprecated?")
E -->|Yes| F("Definitely wrong<br/>today?")
F -->|Yes| G["Correctness"]
F -->|No| H["Suspicious"]
E -->|No| I("Primary improvement?")
I -->|Simpler| J["Complexity"]
I -->|Faster| K["Performance"]
I -->|Idiomatic| L["Style"]
```
The first question filters out special categories of rules: those that relate to the visual
presentation of code (`formatting`), those that relate to `security` vulnerabilities, and those that
impose `restriction`s on certain features. "Restriction" here specifically means an arbitrary or
severe restriction, not the broad way in which any lint rule could be considered to restrict usage.
Examples of restriction lints are rules like `assert` (`S101`) and `print` (`T201`), which ban basic
language features across the board.
If none of these special categories is quite right, the next question asks you to judge whether the
rule is too noisy or opinionated for general use. This is somewhat subjective, but an [ecosystem
report](https://docs.astral.sh/ruff/contributing/#ecosystem-report) can be helpful to see how many
diagnostics are emitted in real projects.
A large number of diagnostics doesn't immediately make a rule `pedantic`, but many false positives
or diagnostics that reasonable Python users would disagree with do.
If a rule is not overly pedantic, we next consider the intention of the rule. If the main goal is
detecting code that is incorrect, the options narrow to `correctness` or `suspicious`. Rules in the
`correctness` category typically cause immediate issues like syntax or runtime errors, or silently
do something the user didn't intend. Similarly, `suspicious` lints flag the same kind of code, but
in cases where Ruff can't be sure what the user intended. A perfect example of a `suspicious` rule
is `mutable-argument-default` (`B006`). This classic footgun is almost always a mistake, but in some
cases, it may be intentional, in which case a `noqa` or `ruff: ignore` comment should be used. Such
suppression comments should essentially never be reasonable for a `correctness` lint but are fine
for `suspicious` lints. The `suspicious` category also includes deprecations, which aren't incorrect
today but will cause errors in the future.
The final branch of the flow chart deals with stylistic lints, which are again somewhat subjective
to differentiate between changes that make code simpler often also make the code faster and
more idiomatic. Thus, the question prompts you to consider the _primary_ improvement. Rules that
primarily make code simpler are `complexity` lints, those that primarily make code faster or use
less memory are `performance` lints, and those that primarily make code more idiomatic are `style`.
## Other guidelines
Following these steps should generally ensure that a rule is a good fit for Ruff. A couple of
additional things to watch out for are:
- Rules that conflict with other tools, or especially other rules
Although we have many existing `formatting` rules that overlap and even conflict with our
formatter, we are not eager to add more. Similarly, we should avoid rules that mainly support
type checker usage, when type checkers themselves emit similar diagnostics. Most clearly, we
should avoid rules that overlap or conflict with other lint rules. This often suggests that the
existing rule should instead be made configurable to toggle between the two behaviors.
Checking both the input and output examples from your rule proposal with `ALL` rules selected in
the [linter], with the [formatter], and with a [type checker] like ty is a good quick check for
conflicts.
- Rules that apply to third-party libraries
Most Ruff rules should be helpful for large numbers of Python developers. This means that rules
should generally apply to Python language features or functionality from the standard library.
However, rules for widely-used third-party libraries can also meet this bar and be good
candidates for inclusion in Ruff.
- Rules that require additional configuration
Most rules should function correctly once enabled without requiring additional settings. If this
isn’t possible, the rule should typically “fail safe” and avoid emitting diagnostics.
`banned-api` (`TID251`) is an example of such a rule that has no effect without configuring
`lint.flake8-tidy-imports.banned-api`. Avoid rules that emit a ton of diagnostics until some
kind of allowlist is configured.
- Rules that are hard to explain
This guideline is inspired by ESLint’s “Generic” [rule guideline](https://eslint.org/docs/latest/contribute/propose-new-rule#core-rule-guidelines):
> Rules cannot be so specific that users will have trouble understanding when to use them. A
> rule is typically too specific if describing what it does requires more than two "and"s (if a
> and b and c and d, then this rule warns).
Watch out for this kind of pattern when writing your `Why is this bad?` or `Known problems`
section, or if you have a hard time categorizing the rule. The `pedantic` category exists to
hold rules that are controversial or niche, but very niche rules may still not be good fits for
Ruff.
[b006]: https://docs.astral.sh/ruff/rules/mutable-argument-default/
[formatter]: https://play.ruff.rs/1265904d-f03c-4d22-aa87-1e6ca16708c2?secondary=Format
[linter]: https://play.ruff.rs/1265904d-f03c-4d22-aa87-1e6ca16708c2
[ruf052]: https://docs.astral.sh/ruff/rules/used-dummy-variable/
[type checker]: https://play.ty.dev/b2d4212e-1243-4d75-a340-ae6ff2e2a6ca
[up037]: https://docs.astral.sh/ruff/rules/quoted-annotation/