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