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