doc/adding_lints.md

   1 ## Adding a new lint
   2
   3 You are probably here because you want to add a new lint to Clippy. If this is
   4 the first time you're contributing to Clippy, this document guides you through
   5 creating an example lint from scratch.
   6
   7 To get started, we will create a lint that detects functions called `foo`,
   8 because that's clearly a non-descriptive name.
   9
  10 - [Adding a new lint](#adding-a-new-lint)
  11   - [Setup](#setup)
  12   - [Getting Started](#getting-started)
  13   - [Testing](#testing)
  14   - [Rustfix tests](#rustfix-tests)
  15   - [Edition 2018 tests](#edition-2018-tests)
  16   - [Testing manually](#testing-manually)
  17   - [Lint declaration](#lint-declaration)
  18   - [Lint passes](#lint-passes)
  19   - [Emitting a lint](#emitting-a-lint)
  20   - [Adding the lint logic](#adding-the-lint-logic)
  21   - [Author lint](#author-lint)
  22   - [Documentation](#documentation)
  23   - [Running rustfmt](#running-rustfmt)
  24   - [Debugging](#debugging)
  25   - [PR Checklist](#pr-checklist)
  26   - [Cheatsheet](#cheatsheet)
  27
  28 ### Setup
  29
  30 When working on Clippy, you will need the current git master version of rustc,
  31 which can change rapidly. Make sure you're working near rust-clippy's master,
  32 and use the `setup-toolchain.sh` script to configure the appropriate toolchain
  33 for the Clippy directory.
  34
  35 ### Getting Started
  36
  37 There is a bit of boilerplate code that needs to be set up when creating a new
  38 lint. Fortunately, you can use the clippy dev tools to handle this for you. We
  39 are naming our new lint `foo_functions` (lints are generally written in snake
  40 case), and we don't need type information so it will have an early pass type
  41 (more on this later on). To get started on this lint you can run
  42 `cargo dev new_lint --name=foo_functions --pass=early --category=pedantic`
  43 (category will default to nursery if not provided). This command will create
  44 two files: `tests/ui/foo_functions.rs` and `clippy_lints/src/foo_functions.rs`,
  45 as well as run `cargo dev update_lints` to register the new lint. Next, we'll
  46 open up these files and add our lint!
  47
  48 ### Testing
  49
  50 Let's write some tests first that we can execute while we iterate on our lint.
  51
  52 Clippy uses UI tests for testing. UI tests check that the output of Clippy is
  53 exactly as expected. Each test is just a plain Rust file that contains the code
  54 we want to check. The output of Clippy is compared against a `.stderr` file.
  55 Note that you don't have to create this file yourself, we'll get to
  56 generating the `.stderr` files further down.
  57
  58 We start by opening the test file created at `tests/ui/foo_functions.rs`.
  59
  60 Update the file with some examples to get started:
  61
  62 ```rust
  63 #![warn(clippy::foo_functions)]
  64
  65 // Impl methods
  66 struct A;
  67 impl A {
  68     pub fn fo(&self) {}
  69     pub fn foo(&self) {}
  70     pub fn food(&self) {}
  71 }
  72
  73 // Default trait methods
  74 trait B {
  75     fn fo(&self) {}
  76     fn foo(&self) {}
  77     fn food(&self) {}
  78 }
  79
  80 // Plain functions
  81 fn fo() {}
  82 fn foo() {}
  83 fn food() {}
  84
  85 fn main() {
  86     // We also don't want to lint method calls
  87     foo();
  88     let a = A;
  89     a.foo();
  90 }
  91
  92 ```
  93
  94 Now we can run the test with `TESTNAME=foo_functions cargo uitest`.
  95 Currently this test will fail. If you go through the output you will see that we
  96 are told that `clippy::foo_functions` is an unknown lint, which is expected.
  97
  98 While we are working on implementing our lint, we can keep running the UI
  99 test. That allows us to check if the output is turning into what we want.
 100
 101 Once we are satisfied with the output, we need to run
 102 `tests/ui/update-all-references.sh` to update the `.stderr` file for our lint.
 103 Please note that, we should run `TESTNAME=foo_functions cargo uitest`
 104 every time before running `tests/ui/update-all-references.sh`.
 105 Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit
 106 our lint, we need to commit the generated `.stderr` files, too.
 107
 108 ### Rustfix tests
 109
 110 If the lint you are working on is making use of structured suggestions, the
 111 test file should include a `// run-rustfix` comment at the top. This will
 112 additionally run [rustfix](https://github.com/rust-lang-nursery/rustfix) for
 113 that test. Rustfix will apply the suggestions from the lint to the code of the
 114 test file and compare that to the contents of a `.fixed` file.
 115
 116 Use `tests/ui/update-all-references.sh` to automatically generate the
 117 `.fixed` file after running the tests.
 118
 119 ### Edition 2018 tests
 120
 121 Some features require the 2018 edition to work (e.g. `async_await`), but
 122 compile-test tests run on the 2015 edition by default. To change this behavior
 123 add `// compile-flags: --edition 2018` at the top of the test file.
 124
 125 ### Testing manually
 126
 127 Manually testing against an example file can be useful if you have added some
 128 `println!`s and the test suite output becomes unreadable. To try Clippy with
 129 your local modifications, run `env CLIPPY_TESTS=true cargo run --bin
 130 clippy-driver -- -L ./target/debug input.rs` from the working copy root.
 131
 132 With tests in place, let's have a look at implementing our lint now.
 133
 134 ### Lint declaration
 135
 136 Let's start by opening the new file created in the `clippy_lints` crate
 137 at `clippy_lints/src/foo_functions.rs`. That's the crate where all the
 138 lint code is. This file has already imported some initial things we will need:
 139
 140 ```rust
 141 use rustc_lint::{EarlyLintPass, EarlyContext};
 142 use rustc_session::{declare_lint_pass, declare_tool_lint};
 143 use syntax::ast::*;
 144 ```
 145
 146 The next step is to update the lint declaration. Lints are declared using the
 147 [`declare_clippy_lint!`][declare_clippy_lint] macro, and we just need to update
 148 the auto-generated lint declaration to have a real description, something like this:
 149
 150 ```rust
 151 declare_clippy_lint! {
 152     /// **What it does:**
 153     ///
 154     /// **Why is this bad?**
 155     ///
 156     /// **Known problems:** None.
 157     ///
 158     /// **Example:**
 159     ///
 160     /// ```rust
 161     /// // example code
 162     /// ```
 163     pub FOO_FUNCTIONS,
 164     pedantic,
 165     "function named `foo`, which is not a descriptive name"
 166 }
 167 ```
 168
 169 * The section of lines prefixed with `///` constitutes the lint documentation
 170 section. This is the default documentation style and will be displayed at
 171 https://rust-lang.github.io/rust-clippy/master/index.html.
 172 * `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the [lint naming
 173 guidelines][lint_naming] here when naming your lint. In short, the name should
 174 state the thing that is being checked for and read well when used with
 175 `allow`/`warn`/`deny`.
 176 * `pedantic` sets the lint level to `Allow`.
 177   The exact mapping can be found [here][category_level_mapping]
 178 * The last part should be a text that explains what exactly is wrong with the
 179   code
 180
 181 The rest of this file contains an empty implementation for our lint pass,
 182 which in this case is `EarlyLintPass` and should look like this:
 183
 184 ```rust
 185 // clippy_lints/src/foo_functions.rs
 186
 187 // .. imports and lint declaration ..
 188
 189 declare_lint_pass!(FooFunctions => [FOO_FUNCTIONS]);
 190
 191 impl EarlyLintPass for FooFunctions {}
 192 ```
 193
 194 Don't worry about the `name` method here. As long as it includes the name of the
 195 lint pass it should be fine.
 196
 197 The new lint automation runs `update_lints`, which automates some things, but it
 198 doesn't automate everything. We will have to register our lint pass manually in
 199 the `register_plugins` function in `clippy_lints/src/lib.rs`:
 200
 201 ```rust
 202 reg.register_early_lint_pass(box foo_functions::FooFunctions);
 203 ```
 204
 205 This should fix the `unknown clippy lint: clippy::foo_functions` error that we
 206 saw when we executed our tests the first time. The next decision we have to make
 207 is which lint pass our lint is going to need.
 208
 209 ### Lint passes
 210
 211 Writing a lint that only checks for the name of a function means that we only
 212 have to deal with the AST and don't have to deal with the type system at all.
 213 This is good, because it makes writing this particular lint less complicated.
 214
 215 We have to make this decision with every new Clippy lint. It boils down to using
 216 either [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass].
 217
 218 In short, the `LateLintPass` has access to type information while the
 219 `EarlyLintPass` doesn't. If you don't need access to type information, use the
 220 `EarlyLintPass`. The `EarlyLintPass` is also faster. However linting speed
 221 hasn't really been a concern with Clippy so far.
 222
 223 Since we don't need type information for checking the function name, we used
 224 `--pass=early` when running the new lint automation and all the imports were
 225 added accordingly.
 226
 227 ### Emitting a lint
 228
 229 With UI tests and the lint declaration in place, we can start working on the
 230 implementation of the lint logic.
 231
 232 Let's start by implementing the `EarlyLintPass` for our `FooFunctions`:
 233
 234 ```rust
 235 impl EarlyLintPass for FooFunctions {
 236     fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) {
 237         // TODO: Emit lint here
 238     }
 239 }
 240 ```
 241
 242 We implement the [`check_fn`][check_fn] method from the
 243 [`EarlyLintPass`][early_lint_pass] trait. This gives us access to various
 244 information about the function that is currently being checked. More on that in
 245 the next section. Let's worry about the details later and emit our lint for
 246 *every* function definition first.
 247
 248 Depending on how complex we want our lint message to be, we can choose from a
 249 variety of lint emission functions. They can all be found in
 250 [`clippy_lints/src/utils/diagnostics.rs`][diagnostics].
 251
 252 `span_lint_and_help` seems most appropriate in this case. It allows us to
 253 provide an extra help message and we can't really suggest a better name
 254 automatically. This is how it looks:
 255
 256 ```rust
 257 impl EarlyLintPass for FooFunctions {
 258     fn check_fn(&mut self, cx: &EarlyContext<'_>, _: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) {
 259         span_lint_and_help(
 260             cx,
 261             FOO_FUNCTIONS,
 262             span,
 263             "function named `foo`",
 264             "consider using a more meaningful name"
 265         );
 266     }
 267 }
 268 ```
 269
 270 Running our UI test should now produce output that contains the lint message.
 271
 272 ### Adding the lint logic
 273
 274 Writing the logic for your lint will most likely be different from our example,
 275 so this section is kept rather short.
 276
 277 Using the [`check_fn`][check_fn] method gives us access to [`FnKind`][fn_kind]
 278 that has two relevant variants for us `FnKind::ItemFn` and `FnKind::Method`.
 279 Both provide access to the name of the function/method via an [`Ident`][ident].
 280
 281 With that we can expand our `check_fn` method to:
 282
 283 ```rust
 284 impl EarlyLintPass for FooFunctions {
 285     fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) {
 286         if is_foo_fn(fn_kind) {
 287             span_lint_and_help(
 288                 cx,
 289                 FOO_FUNCTIONS,
 290                 span,
 291                 "function named `foo`",
 292                 "consider using a more meaningful name"
 293             );
 294         }
 295     }
 296 }
 297 ```
 298
 299 We separate the lint conditional from the lint emissions because it makes the
 300 code a bit easier to read. In some cases this separation would also allow to
 301 write some unit tests (as opposed to only UI tests) for the separate function.
 302
 303 In our example, `is_foo_fn` looks like:
 304
 305 ```rust
 306 // use statements, impl EarlyLintPass, check_fn, ..
 307
 308 fn is_foo_fn(fn_kind: FnKind<'_>) -> bool {
 309     match fn_kind {
 310         FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) => {
 311             ident.name == "foo"
 312         },
 313         FnKind::Closure(..) => false
 314     }
 315 }
 316 ```
 317
 318 Now we should also run the full test suite with `cargo test`. At this point
 319 running `cargo test` should produce the expected output. Remember to run
 320 `tests/ui/update-all-references.sh` to update the `.stderr` file.
 321
 322 `cargo test` (as opposed to `cargo uitest`) will also ensure that our lint
 323 implementation is not violating any Clippy lints itself.
 324
 325 That should be it for the lint implementation. Running `cargo test` should now
 326 pass.
 327
 328 ### Author lint
 329
 330 If you have trouble implementing your lint, there is also the internal `author`
 331 lint to generate Clippy code that detects the offending pattern. It does not
 332 work for all of the Rust syntax, but can give a good starting point.
 333
 334 The quickest way to use it, is the [Rust playground: play.rust-lang.org][Play].
 335 Put the code you want to lint into the editor and add the `#[clippy::author]`
 336 attribute above the item. Then run Clippy via `Tools -> Clippy` and you should
 337 see the generated code in the output below.
 338
 339 [Here][author_example] is an example on the playground.
 340
 341 If the command was executed successfully, you can copy the code over to where
 342 you are implementing your lint.
 343
 344 ### Documentation
 345
 346 The final thing before submitting our PR is to add some documentation to our
 347 lint declaration.
 348
 349 Please document your lint with a doc comment akin to the following:
 350
 351 ```rust
 352 declare_clippy_lint! {
 353     /// **What it does:** Checks for ... (describe what the lint matches).
 354     ///
 355     /// **Why is this bad?** Supply the reason for linting the code.
 356     ///
 357     /// **Known problems:** None. (Or describe where it could go wrong.)
 358     ///
 359     /// **Example:**
 360     ///
 361     /// ```rust,ignore
 362     /// // Bad
 363     /// Insert a short example of code that triggers the lint
 364     ///
 365     /// // Good
 366     /// Insert a short example of improved code that doesn't trigger the lint
 367     /// ```
 368     pub FOO_FUNCTIONS,
 369     pedantic,
 370     "function named `foo`, which is not a descriptive name"
 371 }
 372 ```
 373
 374 Once your lint is merged, this documentation will show up in the [lint
 375 list][lint_list].
 376
 377 ### Running rustfmt
 378
 379 [Rustfmt](https://github.com/rust-lang/rustfmt) is a tool for formatting Rust
 380 code according to style guidelines. Your code has to be formatted by `rustfmt`
 381 before a PR can be merged. Clippy uses nightly `rustfmt` in the CI.
 382
 383 It can be installed via `rustup`:
 384
 385 ```bash
 386 rustup component add rustfmt --toolchain=nightly
 387 ```
 388
 389 Use `cargo dev fmt` to format the whole codebase. Make sure that `rustfmt` is
 390 installed for the nightly toolchain.
 391
 392 ### Debugging
 393
 394 If you want to debug parts of your lint implementation, you can use the `dbg!`
 395 macro anywhere in your code. Running the tests should then include the debug
 396 output in the `stdout` part.
 397
 398 ### PR Checklist
 399
 400 Before submitting your PR make sure you followed all of the basic requirements:
 401
 402 <!-- Sync this with `.github/PULL_REQUEST_TEMPLATE` -->
 403
 404 - [ ] Followed [lint naming conventions][lint_naming]
 405 - [ ] Added passing UI tests (including committed `.stderr` file)
 406 - [ ] `cargo test` passes locally
 407 - [ ] Executed `cargo dev update_lints`
 408 - [ ] Added lint documentation
 409 - [ ] Run `cargo dev fmt`
 410
 411 ### Cheatsheet
 412
 413 Here are some pointers to things you are likely going to need for every lint:
 414
 415 * [Clippy utils][utils] - Various helper functions. Maybe the function you need
 416   is already in here (`implements_trait`, `match_path`, `snippet`, etc)
 417 * [Clippy diagnostics][diagnostics]
 418 * [The `if_chain` macro][if_chain]
 419 * [`from_expansion`][from_expansion] and [`in_external_macro`][in_external_macro]
 420 * [`Span`][span]
 421 * [`Applicability`][applicability]
 422 * [The rustc guide][rustc_guide] explains a lot of internal compiler concepts
 423 * [The nightly rustc docs][nightly_docs] which has been linked to throughout
 424   this guide
 425
 426 For `EarlyLintPass` lints:
 427
 428 * [`EarlyLintPass`][early_lint_pass]
 429 * [`syntax::ast`][ast]
 430
 431 For `LateLintPass` lints:
 432
 433 * [`LateLintPass`][late_lint_pass]
 434 * [`Ty::TyKind`][ty]
 435
 436
 437 While most of Clippy's lint utils are documented, most of rustc's internals lack
 438 documentation currently. This is unfortunate, but in most cases you can probably
 439 get away with copying things from existing similar lints. If you are stuck,
 440 don't hesitate to ask on Discord, IRC or in the issue/PR.
 441
 442 [lint_list]: https://rust-lang.github.io/rust-clippy/master/index.html
 443 [lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
 444 [category_level_mapping]: https://github.com/rust-lang/rust-clippy/blob/bd23cb89ec0ea63403a17d3fc5e50c88e38dd54f/clippy_lints/src/lib.rs#L43
 445 [declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/a71acac1da7eaf667ab90a1d65d10e5cc4b80191/clippy_lints/src/lib.rs#L39
 446 [compilation_stages]: https://rust-lang.github.io/rustc-guide/high-level-overview.html#the-main-stages-of-compilation
 447 [check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.EarlyLintPass.html#method.check_fn
 448 [early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.EarlyLintPass.html
 449 [late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.LateLintPass.html
 450 [fn_kind]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/visit/enum.FnKind.html
 451 [diagnostics]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/diagnostics.rs
 452 [utils]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/mod.rs
 453 [ident]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/source_map/symbol/struct.Ident.html
 454 [span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html
 455 [applicability]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html
 456 [if_chain]: https://docs.rs/if_chain/*/if_chain/
 457 [ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/sty/index.html
 458 [ast]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast/index.html
 459 [from_expansion]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html#method.from_expansion
 460 [in_external_macro]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/fn.in_external_macro.html
 461 [play]: https://play.rust-lang.org
 462 [author_example]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f093b986e80ad62f3b67a1f24f5e66e2
 463 [rustc_guide]: https://rust-lang.github.io/rustc-guide/
 464 [nightly_docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/