Skip to content

Clippy Book Updates: Add lint_passes chapter #9740

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
- [Development](development/README.md)
- [Basics](development/basics.md)
- [Adding Lints](development/adding_lints.md)
- [Defining Lints](development/define_lints.md)
- [Writing Tests](development/write_tests.md)
- [Lint Passes](development/lint_passes.md)
- [Common Tools](development/common_tools_writing_lints.md)
- [Infrastructure](development/infrastructure/README.md)
- [Syncing changes between Clippy and rust-lang/rust](development/infrastructure/sync.md)
Expand Down
189 changes: 189 additions & 0 deletions book/src/development/define_lints.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
# Define New Lints

The first step in the journey of a new lint is to define the definition
and registration of the lint in Clippy's codebase.
We can use the Clippy dev tools to handle this step since setting up the
lint involves some boilerplate code.

## Name the Lint

A wise software engineer Phil Karlton once said:
> There are only two hard things in Computer Science: cache invalidation and naming things.

Naming a lint is no exception.
Therefore, in case of uncertainty about if the name you chose fits the lint,
please do the following:

1. Consult our [lint naming guidelines][lint_naming]
2. Ping a [Clippy team member][clippy_team_members] in our [Zulip] chat
3. Comment on the corresponding Github issue (less preferrable as comments might be overlooked)

For now, let's imagine that your fellow developers at work tend to define some of their
functions with the name `foo` and forget to re-name it to a more meaningful name
when they submit a pull request.

`foo` is a highly non-descriptive name for a function, so we want to detect this
bad naming and fix it early on in the development process.

For this, we will create a lint that detects these `foo` functions and
help our fellow developers correct this bad practice. Note that in Clippy,
lints are generally written in snake cases.
We can name this new lint `foo_functions`.

## Add and Register the Lint

Now that a name is chosen, we shall register `foo_functions` as a lint to the codebase.
There are two ways to register a lint.

### Standalone

If you believe that this new lint is a standalone lint, you can run the following
command in your Clippy project:

```sh
$ cargo dev new_lint --name=foo_functions --pass=late --category=pedantic
```

There are two things to note here:

1. We set `--pass=late` in this command to do a late lint pass. The alternative
is an `early` lint pass. We will discuss this difference in [Lint Passes](lint_passes.md).
1. If not provided, the `category` of this new lint will default to `nursery`.
See Clippy's [lint groups](../lints.md) for more information on categories.

The `cargo dev new_lint` command will create a new file: `clippy_lints/src/foo_functions.rs`
as well as [register the lint](#lint-registration).

Overall, you should notice that the following files are modified or created:

```sh
$ git status
On branch foo_functions
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: CHANGELOG.md
modified: clippy_lints/src/lib.register_lints.rs
modified: clippy_lints/src/lib.register_pedantic.rs
modified: clippy_lints/src/lib.rs
modified: src/docs.rs

Untracked files:
(use "git add <file>..." to include in what will be committed)
clippy_lints/src/foo_functions.rs
src/docs/foo_functions.txt
tests/ui/foo_functions.rs
```

### Specific Type

If you believe that this new lint belong to a specific type of lints,
you can run `cargo dev new_lint` with a `--type` option.

Since our `foo_functions` lint is related to function calls, one could
argue that we should put it into a group of lints that detect some behaviors
of functions, we can put it in the `functions` group.

Let's run the following command in your Clippy project:

```sh
$ cargo dev new_lint --name=foo_functions --type=functions --category=pedantic
```

This command will create, among other things, a new file:
`clippy_lints/src/{type}/foo_functions.rs`.
In our case, the path will be `clippy_lints/src/functions/foo_functions.rs`.

Notice how this command has a `--type` flag instead of `--pass`. Unlike a standalone
definition, this lint won't be registered in the traditional sense. Instead, you will
call your lint from within the type's lint pass, found in `clippy_lints/src/{type}/mod.rs`.

A _type_ is just the name of a directory in `clippy_lints/src`, like `functions` in
the example command. Clippy groups together some lints that share common behaviors,
so if your lint falls into one, it would be best to add it to that type.
Read more about [lint groups](#lint-groups) below.

Overall, you should notice that the following files are modified or created:

```sh
$ git status
On branch foo_functions
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: CHANGELOG.md
modified: clippy_lints/src/functions/mod.rs
modified: clippy_lints/src/lib.register_lints.rs
modified: clippy_lints/src/lib.register_pedantic.rs
modified: src/docs.rs

Untracked files:
(use "git add <file>..." to include in what will be committed)
clippy_lints/src/functions/foo_functions.rs
src/docs/foo_functions.txt
tests/ui/foo_functions.rs
```

## Lint registration

If we run the `cargo dev new_lint` command for a new lint,
the lint will be automatically registered and there is nothing more to do.

However, sometimes we might want to declare a new lint by hand.
In this case, we'd use `cargo dev update_lints` command.

When `cargo dev update_lints` is used, we might need to register the lint pass
manually in the `register_plugins` function in `clippy_lints/src/lib.rs`:

```rust
store.register_early_pass(|| Box::new(foo_functions::FooFunctions));
```

As you might have guessed, where there's something early, there is something late:
in Clippy there is a `register_late_pass` method as well.
More on early vs. late passes in a later chapter.

Without a call to one of `register_early_pass` or `register_late_pass`,
the lint pass in question will not be run.

One reason that `cargo dev update_lints` does not automate this step is that
multiple lints might use the same lint pass, so registering the lint pass may
have already been done when adding a new lint.

Another reason for not automating this step is that the order
that the passes are registered determines the order the passes actually run,
which in turn affects the order that any emitted lints are output in.

## Lint groups

As of the writing of this documentation update, there are 11 categories (a.k.a. _types_)
of lints besides the numerous standalone lints living under `clippy_lints/src/`:

- `cargo`
- `casts`
- `functions`
- `loops`
- `matches`
- `methods`
- `misc_early`
- `operators`
- `transmute`
- `types`
- `unit_types`

These categories group together lints that share some common behaviors.
For instance, as we have mentioned earlier, `functions` groups together lints
that deal with some aspects of function calls in Rust.

Some other common categories are `loops` and `methods`. `loops` group is for
lints that involve `for` loops, `while` loops, `range` loops, etc.
`methods` group is for lints that target method calls.

For more information, feel free to compare the lint files under any category
with [All Clippy lints][all_lints] or
ask one of the maintainers.

[all_lints]: https://rust-lang.github.io/rust-clippy/master/
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
[clippy_team_members]: https://www.rust-lang.org/governance/teams/dev-tools#Clippy%20team
[Zulip]: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy
139 changes: 139 additions & 0 deletions book/src/development/lint_passes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
# Lint passes

Before working on the logic of a new lint, there is an important decision
that every Clippy developers must make: to use
[`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass].

In short, the `LateLintPass` has access to type information while the
`EarlyLintPass` doesn't. If you don't need access to type information, use the
`EarlyLintPass`.

- [Lint passes](#lint-passes)
- [`EarlyLintPass`](#earlylintpass)
- [Example](#example)
- [`LateLintPass`](#latelintpass)
- [Example](#example-1)
- [Additional Readings for Beginners](#additional-readings-for-beginners)

## `EarlyLintPass`

Inside the documentation of [`EarlyLintPass`][early_lint_pass], we can see that
most methods defined for this trait utilizes [`EarlyContext`][early_context].
In `EarlyContext`'s documentation, it states:

> Context for lint checking of the AST, after expansion, before lowering to HIR.

As entailed, `EarlyLintPass` works only on the Abstract Syntax Tree (AST) level.
Since AST is generated during the [lexing and parsing][lexing_and_parsing] phase
of code compilation, so `EarlyLintPass` is a good choice for a new lint if
the lint only deals with syntax-related issues.

While linting speed has not been a concern for Clippy, the `EarlyLintPass` is
faster and it should be your choice if you know for sure a lint does not need
type information.

As a reminder, run the following command to generate boilerplates for lints
using `EarlyLintPass`:

```sh
$ cargo dev new_lint --name=<your_new_lint> --pass=early --category=<your_category_choice>
```

### Example

Take a look at the following code:

```rust
let x = OurUndefinedType;
x.non_existing_method();
```

From the AST perspective, both lines are "gramatically" correct.
The assignment uses a `let` and ends with a semicolon. The invocation
of a method looks fine, too. As programmers, we might raise a few
questions already, but the parser is okay with it. This is what we
mean when we say `EarlyLintPass` deals with only syntax on the AST level.

Think about the `foo_functions` lint we mentioned in
[define new lints](define_lints.md#lint-name) chapter.

We want the `foo_functions` lint to detect functions with `foo` as their name.
Writing a lint that only checks for the name of a function means that we only
work with the AST and don't have to access the type system at all (that is where
`LateLintPass` comes into the picture).

## `LateLintPass`

In contrast to `EarlyLintPass`, `LateLintPass` contains type information.

Examining the documentation on [`LateLintPass`][late_lint_pass] closely,
we can observe that just about every method defined for this trait utilizes a
[`LateContext`][late_context].

In `LateContext`'s documentation we will find methods that
deal with type-checking which do not exist in `EarlyContext`, such as:

- [`maybe_typeck_results`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/context/struct.LateContext.html#method.maybe_typeck_results)
- [`typeck_results`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/context/struct.LateContext.html#method.typeck_results)

### Example

Let us take a look with the following example:

```rust
let x = OurUndefinedType;
x.non_existing_method();
```

These two lines of code are syntactically correct code from the perspective
of the AST. We have an assignment and invoke a method on the variable that
is of a type. Gramatically, everything is in order for the parser.

However, going down a level and looking at the type information,
the compiler will notice that both `OurUndefinedType` and `non_existing_method()`
are undefined.

To access such type information means to implement `LateLintPass` on a lint.
When you browse through Clippy's lints, you will notice that almost every lint
is implemented in a `LateLintPass`, specifically because we often need to check
not only for syntactic issues but also type information.

Moreover, in `EarlyLintPass`, nodes are only identified by their position in the AST.
This means that you can't just get an `id` and request a certain node.
For most lints that is fine, but we have some lints that require the inspection of other nodes,
which is easier at the HIR level. In these cases, `LateLintPass` is the better choice.

As a reminder, run the following command to generate boilerplates for lints
using `LateLintPass`:

```sh
$ cargo dev new_lint --name=<your_new_lint> --pass=late --category=<your_category_choice>
```

## Additional Readings for Beginners

Should a reader be relatively new to the linting world or has never taken a class on compilers
and interpreters, it might be confusing as to why AST level deals with only
the language's syntax. And some readers might not even understand what lexing,
parsing, and AST mean.

This documentation serves by no means as a crash course on compilers or language design.
And for details specifically related to Rust, the [Rustc Development Guide][rustc_dev_guide]
is a far better choice to peruse.

The [Syntax and AST][ast] chapter and the [High-Level IR][hir] chapter are
great introduction to the concepts mentioned in this chapter.

Some readers might also find the [introductory chapter][map_of_territory] of
Robert Nystrom's _Crafting Interpreters_ a helpful overview of compiled and
interpreted languages before jumping back to the Rustc guide.

[ast]: https://rustc-dev-guide.rust-lang.org/syntax-intro.html
[early_context]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/context/struct.EarlyContext.html
[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html
[hir]: https://rustc-dev-guide.rust-lang.org/hir.html
[late_context]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/context/struct.LateContext.html
[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html
[lexing_and_parsing]: https://rustc-dev-guide.rust-lang.org/overview.html#lexing-and-parsing
[rustc_dev_guide]: https://rustc-dev-guide.rust-lang.org/
[map_of_territory]: https://craftinginterpreters.com/a-map-of-the-territory.html