-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Update Documentation to Focus on LateLintPass
#9426
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
Changes from all commits
7612603
7910894
3290f5e
faf52c9
3e12616
5948c6a
ea136e5
daa84dc
3be525b
851fd7e
99fe34b
2f32508
02330c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
# Adding Documentation for New Lint | ||
|
||
Assuming that our dear reader has read all the previous chapters | ||
on development in Clippy, the final thing to do when authoring a | ||
new lint or updating an existing lint is to write or edit the | ||
lint decalaration's documentation. | ||
|
||
You might have come across [Clippy lints list][lint_list] and | ||
read about a few lints that you want to use, restrict, or understand. | ||
|
||
These lints's documentation is generated by using the `declare_clippy_lint` | ||
macro and by writing the documentation inside: | ||
Comment on lines
+8
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would write this something like
|
||
|
||
```rust | ||
declare_clippy_lint! { | ||
/// ### What it does | ||
/// Checks for ... (describe what the lint matches). | ||
/// | ||
/// ### Why is this bad? | ||
/// Supply the reason for linting the code. | ||
/// | ||
/// ### Example | ||
/// | ||
/// ```rust,ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and below, I would remove the |
||
/// // A short example of code that triggers the lint | ||
/// ``` | ||
/// | ||
/// Use instead: | ||
/// ```rust,ignore | ||
/// // A short example of improved code that doesn't trigger the lint | ||
/// ``` | ||
#[clippy::version = "1.64.0"] | ||
pub FOO_FUNCTIONS, | ||
pedantic, | ||
"function named `foo`, which is not a descriptive name" | ||
} | ||
``` | ||
|
||
Make sure you put some love into writing a good documentation so that | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we could also add something about the special magic Besides this, another awesome chapter! |
||
other Clippy users can understand the awesome lint that you just created. | ||
|
||
Once the PR to your lint is merged, this documentation will show up in the | ||
[lint list][lint_list]. Pat yourself on the shoulder and be proud that | ||
you have made Rust that extra little bit more awesome for the world! | ||
|
||
[lint_list]: https://rust-lang.github.io/rust-clippy/master/index.html |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,189 @@ | ||
# Define New Lints | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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 | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
A wise software engineer Phil Karlton once said: | ||
> There are only two hard things in Computer Science: cache invalidation and naming things. | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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`. | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Add and Register the Lint | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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). | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Overall, you should notice that the following files are modified or created: | ||
|
||
```sh | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$ 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. | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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. | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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 | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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. | ||
nahuakang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be any story telling in here. Just the code snippet how the declaration/documentation looks like and a short explanation for each section in the documentation.