Clean-up adding_lints.md
This commit is contained in:
Yuki Okushi
parent
8cdd75ca29
commit
459fb53f8d
1 changed files with 49 additions and 50 deletions
|
|
@ -1,4 +1,4 @@
|
|||
## Adding a new lint
|
||||
# Adding a new lint
|
||||
|
||||
You are probably here because you want to add a new lint to Clippy. If this is
|
||||
the first time you're contributing to Clippy, this document guides you through
|
||||
|
|
@ -25,14 +25,14 @@ because that's clearly a non-descriptive name.
|
|||
- [PR Checklist](#pr-checklist)
|
||||
- [Cheatsheet](#cheatsheet)
|
||||
|
||||
### Setup
|
||||
## Setup
|
||||
|
||||
When working on Clippy, you will need the current git master version of rustc,
|
||||
which can change rapidly. Make sure you're working near rust-clippy's master,
|
||||
and use the `setup-toolchain.sh` script to configure the appropriate toolchain
|
||||
for the Clippy directory.
|
||||
|
||||
### Getting Started
|
||||
## Getting Started
|
||||
|
||||
There is a bit of boilerplate code that needs to be set up when creating a new
|
||||
lint. Fortunately, you can use the clippy dev tools to handle this for you. We
|
||||
|
|
@ -45,7 +45,7 @@ two files: `tests/ui/foo_functions.rs` and `clippy_lints/src/foo_functions.rs`,
|
|||
as well as run `cargo dev update_lints` to register the new lint. Next, we'll
|
||||
open up these files and add our lint!
|
||||
|
||||
### Testing
|
||||
## Testing
|
||||
|
||||
Let's write some tests first that we can execute while we iterate on our lint.
|
||||
|
||||
|
|
@ -88,12 +88,10 @@ fn main() {
|
|||
let a = A;
|
||||
a.foo();
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
Now we can run the test with `TESTNAME=foo_functions cargo uitest`.
|
||||
Currently this test will fail. If you go through the output you will see that we
|
||||
are told that `clippy::foo_functions` is an unknown lint, which is expected.
|
||||
Now we can run the test with `TESTNAME=foo_functions cargo uitest`,
|
||||
currently this test is meaningless though.
|
||||
|
||||
While we are working on implementing our lint, we can keep running the UI
|
||||
test. That allows us to check if the output is turning into what we want.
|
||||
|
|
@ -105,7 +103,7 @@ every time before running `tests/ui/update-all-references.sh`.
|
|||
Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit
|
||||
our lint, we need to commit the generated `.stderr` files, too.
|
||||
|
||||
### Rustfix tests
|
||||
## Rustfix tests
|
||||
|
||||
If the lint you are working on is making use of structured suggestions, the
|
||||
test file should include a `// run-rustfix` comment at the top. This will
|
||||
|
|
@ -116,13 +114,13 @@ test file and compare that to the contents of a `.fixed` file.
|
|||
Use `tests/ui/update-all-references.sh` to automatically generate the
|
||||
`.fixed` file after running the tests.
|
||||
|
||||
### Edition 2018 tests
|
||||
## Edition 2018 tests
|
||||
|
||||
Some features require the 2018 edition to work (e.g. `async_await`), but
|
||||
compile-test tests run on the 2015 edition by default. To change this behavior
|
||||
add `// compile-flags: --edition 2018` at the top of the test file.
|
||||
add `// edition:2018` at the top of the test file (note that it's space-sensitive).
|
||||
|
||||
### Testing manually
|
||||
## Testing manually
|
||||
|
||||
Manually testing against an example file can be useful if you have added some
|
||||
`println!`s and the test suite output becomes unreadable. To try Clippy with
|
||||
|
|
@ -131,7 +129,7 @@ clippy-driver -- -L ./target/debug input.rs` from the working copy root.
|
|||
|
||||
With tests in place, let's have a look at implementing our lint now.
|
||||
|
||||
### Lint declaration
|
||||
## Lint declaration
|
||||
|
||||
Let's start by opening the new file created in the `clippy_lints` crate
|
||||
at `clippy_lints/src/foo_functions.rs`. That's the crate where all the
|
||||
|
|
@ -140,7 +138,7 @@ lint code is. This file has already imported some initial things we will need:
|
|||
```rust
|
||||
use rustc_lint::{EarlyLintPass, EarlyContext};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use syntax::ast::*;
|
||||
use rustc_ast::ast::*;
|
||||
```
|
||||
|
||||
The next step is to update the lint declaration. Lints are declared using the
|
||||
|
|
@ -167,8 +165,8 @@ declare_clippy_lint! {
|
|||
```
|
||||
|
||||
* The section of lines prefixed with `///` constitutes the lint documentation
|
||||
section. This is the default documentation style and will be displayed at
|
||||
https://rust-lang.github.io/rust-clippy/master/index.html.
|
||||
section. This is the default documentation style and will be displayed
|
||||
[like this][example_lint_page].
|
||||
* `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the [lint naming
|
||||
guidelines][lint_naming] here when naming your lint. In short, the name should
|
||||
state the thing that is being checked for and read well when used with
|
||||
|
|
@ -199,14 +197,12 @@ automate everything. We will have to register our lint pass manually in the
|
|||
`register_plugins` function in `clippy_lints/src/lib.rs`:
|
||||
|
||||
```rust
|
||||
store.register_early_pass(box foo_functions::FooFunctions);
|
||||
store.register_early_pass(|| box foo_functions::FooFunctions);
|
||||
```
|
||||
|
||||
This should fix the `unknown clippy lint: clippy::foo_functions` error that we
|
||||
saw when we executed our tests the first time. The next decision we have to make
|
||||
is which lint pass our lint is going to need.
|
||||
[example lint page]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
|
||||
|
||||
### Lint passes
|
||||
## Lint passes
|
||||
|
||||
Writing a lint that only checks for the name of a function means that we only
|
||||
have to deal with the AST and don't have to deal with the type system at all.
|
||||
|
|
@ -224,7 +220,7 @@ Since we don't need type information for checking the function name, we used
|
|||
`--pass=early` when running the new lint automation and all the imports were
|
||||
added accordingly.
|
||||
|
||||
### Emitting a lint
|
||||
## Emitting a lint
|
||||
|
||||
With UI tests and the lint declaration in place, we can start working on the
|
||||
implementation of the lint logic.
|
||||
|
|
@ -233,7 +229,7 @@ Let's start by implementing the `EarlyLintPass` for our `FooFunctions`:
|
|||
|
||||
```rust
|
||||
impl EarlyLintPass for FooFunctions {
|
||||
fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) {
|
||||
fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) {
|
||||
// TODO: Emit lint here
|
||||
}
|
||||
}
|
||||
|
|
@ -255,7 +251,7 @@ automatically. This is how it looks:
|
|||
|
||||
```rust
|
||||
impl EarlyLintPass for FooFunctions {
|
||||
fn check_fn(&mut self, cx: &EarlyContext<'_>, _: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) {
|
||||
fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) {
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
FOO_FUNCTIONS,
|
||||
|
|
@ -269,20 +265,20 @@ impl EarlyLintPass for FooFunctions {
|
|||
|
||||
Running our UI test should now produce output that contains the lint message.
|
||||
|
||||
### Adding the lint logic
|
||||
## Adding the lint logic
|
||||
|
||||
Writing the logic for your lint will most likely be different from our example,
|
||||
so this section is kept rather short.
|
||||
|
||||
Using the [`check_fn`][check_fn] method gives us access to [`FnKind`][fn_kind]
|
||||
that has two relevant variants for us `FnKind::ItemFn` and `FnKind::Method`.
|
||||
Both provide access to the name of the function/method via an [`Ident`][ident].
|
||||
that has the `FnKind::Fn` variant. It provides access to the name of the
|
||||
function/method via an [`Ident`][ident].
|
||||
|
||||
With that we can expand our `check_fn` method to:
|
||||
|
||||
```rust
|
||||
impl EarlyLintPass for FooFunctions {
|
||||
fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) {
|
||||
fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) {
|
||||
if is_foo_fn(fn_kind) {
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
|
|
@ -307,9 +303,11 @@ In our example, `is_foo_fn` looks like:
|
|||
|
||||
fn is_foo_fn(fn_kind: FnKind<'_>) -> bool {
|
||||
match fn_kind {
|
||||
FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) => {
|
||||
ident.name == "foo"
|
||||
},
|
||||
FnKind::Fn(_, ident, ..) => {
|
||||
// check if `fn` name is `foo`
|
||||
ident.name.as_str() == "foo"
|
||||
}
|
||||
// ignore closures
|
||||
FnKind::Closure(..) => false
|
||||
}
|
||||
}
|
||||
|
|
@ -325,13 +323,14 @@ implementation is not violating any Clippy lints itself.
|
|||
That should be it for the lint implementation. Running `cargo test` should now
|
||||
pass.
|
||||
|
||||
### Author lint
|
||||
## Author lint
|
||||
|
||||
If you have trouble implementing your lint, there is also the internal `author`
|
||||
lint to generate Clippy code that detects the offending pattern. It does not
|
||||
work for all of the Rust syntax, but can give a good starting point.
|
||||
|
||||
The quickest way to use it, is the [Rust playground: play.rust-lang.org][Play].
|
||||
The quickest way to use it, is the
|
||||
[Rust playground: play.rust-lang.org][author_example].
|
||||
Put the code you want to lint into the editor and add the `#[clippy::author]`
|
||||
attribute above the item. Then run Clippy via `Tools -> Clippy` and you should
|
||||
see the generated code in the output below.
|
||||
|
|
@ -341,7 +340,7 @@ see the generated code in the output below.
|
|||
If the command was executed successfully, you can copy the code over to where
|
||||
you are implementing your lint.
|
||||
|
||||
### Documentation
|
||||
## Documentation
|
||||
|
||||
The final thing before submitting our PR is to add some documentation to our
|
||||
lint declaration.
|
||||
|
|
@ -374,7 +373,7 @@ declare_clippy_lint! {
|
|||
Once your lint is merged, this documentation will show up in the [lint
|
||||
list][lint_list].
|
||||
|
||||
### Running rustfmt
|
||||
## Running rustfmt
|
||||
|
||||
[Rustfmt](https://github.com/rust-lang/rustfmt) is a tool for formatting Rust
|
||||
code according to style guidelines. Your code has to be formatted by `rustfmt`
|
||||
|
|
@ -389,13 +388,13 @@ rustup component add rustfmt --toolchain=nightly
|
|||
Use `cargo dev fmt` to format the whole codebase. Make sure that `rustfmt` is
|
||||
installed for the nightly toolchain.
|
||||
|
||||
### Debugging
|
||||
## Debugging
|
||||
|
||||
If you want to debug parts of your lint implementation, you can use the `dbg!`
|
||||
If you want to debug parts of your lint implementation, you can use the [`dbg!`]
|
||||
macro anywhere in your code. Running the tests should then include the debug
|
||||
output in the `stdout` part.
|
||||
|
||||
### PR Checklist
|
||||
## PR Checklist
|
||||
|
||||
Before submitting your PR make sure you followed all of the basic requirements:
|
||||
|
||||
|
|
@ -408,7 +407,7 @@ Before submitting your PR make sure you followed all of the basic requirements:
|
|||
- [ ] Added lint documentation
|
||||
- [ ] Run `cargo dev fmt`
|
||||
|
||||
### Cheatsheet
|
||||
## Cheatsheet
|
||||
|
||||
Here are some pointers to things you are likely going to need for every lint:
|
||||
|
||||
|
|
@ -426,38 +425,38 @@ Here are some pointers to things you are likely going to need for every lint:
|
|||
For `EarlyLintPass` lints:
|
||||
|
||||
* [`EarlyLintPass`][early_lint_pass]
|
||||
* [`syntax::ast`][ast]
|
||||
* [`rustc_ast::ast`][ast]
|
||||
|
||||
For `LateLintPass` lints:
|
||||
|
||||
* [`LateLintPass`][late_lint_pass]
|
||||
* [`Ty::TyKind`][ty]
|
||||
|
||||
|
||||
While most of Clippy's lint utils are documented, most of rustc's internals lack
|
||||
documentation currently. This is unfortunate, but in most cases you can probably
|
||||
get away with copying things from existing similar lints. If you are stuck,
|
||||
don't hesitate to ask on Discord, IRC or in the issue/PR.
|
||||
don't hesitate to ask on [Discord] or in the issue/PR.
|
||||
|
||||
[lint_list]: https://rust-lang.github.io/rust-clippy/master/index.html
|
||||
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
|
||||
[category_level_mapping]: https://github.com/rust-lang/rust-clippy/blob/bd23cb89ec0ea63403a17d3fc5e50c88e38dd54f/clippy_lints/src/lib.rs#L43
|
||||
[declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/a71acac1da7eaf667ab90a1d65d10e5cc4b80191/clippy_lints/src/lib.rs#L39
|
||||
[check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.EarlyLintPass.html#method.check_fn
|
||||
[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.EarlyLintPass.html
|
||||
[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.LateLintPass.html
|
||||
[fn_kind]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/visit/enum.FnKind.html
|
||||
[check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html#method.check_fn
|
||||
[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html
|
||||
[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html
|
||||
[fn_kind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html
|
||||
[diagnostics]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/diagnostics.rs
|
||||
[utils]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/mod.rs
|
||||
[ident]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/source_map/symbol/struct.Ident.html
|
||||
[ident]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/struct.Ident.html
|
||||
[span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html
|
||||
[applicability]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html
|
||||
[if_chain]: https://docs.rs/if_chain/*/if_chain/
|
||||
[ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/sty/index.html
|
||||
[ast]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast/index.html
|
||||
[ast]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/index.html
|
||||
[from_expansion]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html#method.from_expansion
|
||||
[in_external_macro]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/fn.in_external_macro.html
|
||||
[play]: https://play.rust-lang.org
|
||||
[author_example]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f093b986e80ad62f3b67a1f24f5e66e2
|
||||
[author_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=9a12cb60e5c6ad4e3003ac6d5e63cf55
|
||||
[rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/
|
||||
[nightly_docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/
|
||||
[Discord]: https://discord.gg/rust-lang
|
||||
[`dbg`!]: https://doc.rust-lang.org/std/macro.dbg.html
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue