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