]>
Commit | Line | Data |
---|---|---|
f20569fa XL |
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) | |
064997fb FG |
13 | - [Defining Our Lint](#defining-our-lint) |
14 | - [Standalone](#standalone) | |
15 | - [Specific Type](#specific-type) | |
16 | - [Tests Location](#tests-location) | |
f20569fa | 17 | - [Testing](#testing) |
94222f64 | 18 | - [Cargo lints](#cargo-lints) |
f20569fa | 19 | - [Rustfix tests](#rustfix-tests) |
f20569fa XL |
20 | - [Testing manually](#testing-manually) |
21 | - [Lint declaration](#lint-declaration) | |
3c0e092e | 22 | - [Lint registration](#lint-registration) |
f20569fa XL |
23 | - [Lint passes](#lint-passes) |
24 | - [Emitting a lint](#emitting-a-lint) | |
25 | - [Adding the lint logic](#adding-the-lint-logic) | |
cdc7bbd5 | 26 | - [Specifying the lint's minimum supported Rust version (MSRV)](#specifying-the-lints-minimum-supported-rust-version-msrv) |
f20569fa | 27 | - [Author lint](#author-lint) |
04454e1e | 28 | - [Print HIR lint](#print-hir-lint) |
f20569fa XL |
29 | - [Documentation](#documentation) |
30 | - [Running rustfmt](#running-rustfmt) | |
31 | - [Debugging](#debugging) | |
32 | - [PR Checklist](#pr-checklist) | |
33 | - [Adding configuration to a lint](#adding-configuration-to-a-lint) | |
923072b8 | 34 | - [Cheat Sheet](#cheat-sheet) |
f20569fa XL |
35 | |
36 | ## Setup | |
37 | ||
38 | See the [Basics](basics.md#get-the-code) documentation. | |
39 | ||
40 | ## Getting Started | |
41 | ||
42 | There is a bit of boilerplate code that needs to be set up when creating a new | |
064997fb | 43 | lint. Fortunately, you can use the Clippy dev tools to handle this for you. We |
f20569fa | 44 | are naming our new lint `foo_functions` (lints are generally written in snake |
064997fb FG |
45 | case), and we don't need type information, so it will have an early pass type |
46 | (more on this later). If you're unsure if the name you chose fits the lint, | |
47 | take a look at our [lint naming guidelines][lint_naming]. | |
48 | ||
49 | ## Defining Our Lint | |
50 | To get started, there are two ways to define our lint. | |
51 | ||
52 | ### Standalone | |
53 | Command: `cargo dev new_lint --name=foo_functions --pass=early --category=pedantic` | |
54 | (category will default to nursery if not provided) | |
55 | ||
56 | This command will create a new file: `clippy_lints/src/foo_functions.rs`, as well | |
57 | as [register the lint](#lint-registration). | |
58 | ||
59 | ### Specific Type | |
60 | Command: `cargo dev new_lint --name=foo_functions --type=functions --category=pedantic` | |
61 | ||
62 | This command will create a new file: `clippy_lints/src/{type}/foo_functions.rs`. | |
63 | ||
64 | Notice how this command has a `--type` flag instead of `--pass`. Unlike a standalone | |
65 | definition, this lint won't be registered in the traditional sense. Instead, you will | |
66 | call your lint from within the type's lint pass, found in `clippy_lints/src/{type}/mod.rs`. | |
67 | ||
68 | A "type" is just the name of a directory in `clippy_lints/src`, like `functions` in | |
69 | the example command. These are groupings of lints with common behaviors, so if your | |
70 | lint falls into one, it would be best to add it to that type. | |
71 | ||
72 | ### Tests Location | |
73 | Both commands will create a file: `tests/ui/foo_functions.rs`. For cargo lints, | |
74 | two project hierarchies (fail/pass) will be created by default under `tests/ui-cargo`. | |
f20569fa XL |
75 | |
76 | Next, we'll open up these files and add our lint! | |
77 | ||
78 | ## Testing | |
79 | ||
80 | Let's write some tests first that we can execute while we iterate on our lint. | |
81 | ||
82 | Clippy uses UI tests for testing. UI tests check that the output of Clippy is | |
83 | exactly as expected. Each test is just a plain Rust file that contains the code | |
84 | we want to check. The output of Clippy is compared against a `.stderr` file. | |
923072b8 FG |
85 | Note that you don't have to create this file yourself, we'll get to generating |
86 | the `.stderr` files further down. | |
f20569fa XL |
87 | |
88 | We start by opening the test file created at `tests/ui/foo_functions.rs`. | |
89 | ||
90 | Update the file with some examples to get started: | |
91 | ||
92 | ```rust | |
2b03887a | 93 | #![allow(unused)] |
f20569fa XL |
94 | #![warn(clippy::foo_functions)] |
95 | ||
96 | // Impl methods | |
97 | struct A; | |
98 | impl A { | |
99 | pub fn fo(&self) {} | |
100 | pub fn foo(&self) {} | |
101 | pub fn food(&self) {} | |
102 | } | |
103 | ||
104 | // Default trait methods | |
105 | trait B { | |
106 | fn fo(&self) {} | |
107 | fn foo(&self) {} | |
108 | fn food(&self) {} | |
109 | } | |
110 | ||
111 | // Plain functions | |
112 | fn fo() {} | |
113 | fn foo() {} | |
114 | fn food() {} | |
115 | ||
116 | fn main() { | |
117 | // We also don't want to lint method calls | |
118 | foo(); | |
119 | let a = A; | |
120 | a.foo(); | |
121 | } | |
122 | ``` | |
123 | ||
923072b8 FG |
124 | Now we can run the test with `TESTNAME=foo_functions cargo uitest`, currently |
125 | this test is meaningless though. | |
f20569fa | 126 | |
923072b8 FG |
127 | While we are working on implementing our lint, we can keep running the UI test. |
128 | That allows us to check if the output is turning into what we want. | |
f20569fa | 129 | |
923072b8 FG |
130 | Once we are satisfied with the output, we need to run `cargo dev bless` to |
131 | update the `.stderr` file for our lint. Please note that, we should run | |
132 | `TESTNAME=foo_functions cargo uitest` every time before running `cargo dev | |
133 | bless`. Running `TESTNAME=foo_functions cargo uitest` should pass then. When we | |
134 | commit our lint, we need to commit the generated `.stderr` files, too. In | |
135 | general, you should only commit files changed by `cargo dev bless` for the | |
f20569fa XL |
136 | specific lint you are creating/editing. Note that if the generated files are |
137 | empty, they should be removed. | |
138 | ||
923072b8 FG |
139 | > _Note:_ you can run multiple test files by specifying a comma separated list: |
140 | > `TESTNAME=foo_functions,test2,test3`. | |
f20569fa XL |
141 | |
142 | ### Cargo lints | |
143 | ||
923072b8 FG |
144 | For cargo lints, the process of testing differs in that we are interested in the |
145 | `Cargo.toml` manifest file. We also need a minimal crate associated with that | |
146 | manifest. | |
f20569fa | 147 | |
923072b8 FG |
148 | If our new lint is named e.g. `foo_categories`, after running `cargo dev |
149 | new_lint` we will find by default two new crates, each with its manifest file: | |
f20569fa | 150 | |
923072b8 FG |
151 | * `tests/ui-cargo/foo_categories/fail/Cargo.toml`: this file should cause the |
152 | new lint to raise an error. | |
153 | * `tests/ui-cargo/foo_categories/pass/Cargo.toml`: this file should not trigger | |
154 | the lint. | |
f20569fa | 155 | |
923072b8 FG |
156 | If you need more cases, you can copy one of those crates (under |
157 | `foo_categories`) and rename it. | |
f20569fa | 158 | |
923072b8 FG |
159 | The process of generating the `.stderr` file is the same, and prepending the |
160 | `TESTNAME` variable to `cargo uitest` works too. | |
f20569fa XL |
161 | |
162 | ## Rustfix tests | |
163 | ||
923072b8 FG |
164 | If the lint you are working on is making use of structured suggestions, the test |
165 | file should include a `// run-rustfix` comment at the top. This will | |
f20569fa | 166 | additionally run [rustfix] for that test. Rustfix will apply the suggestions |
923072b8 FG |
167 | from the lint to the code of the test file and compare that to the contents of a |
168 | `.fixed` file. | |
f20569fa | 169 | |
923072b8 FG |
170 | Use `cargo dev bless` to automatically generate the `.fixed` file after running |
171 | the tests. | |
f20569fa XL |
172 | |
173 | [rustfix]: https://github.com/rust-lang/rustfix | |
174 | ||
f20569fa XL |
175 | ## Testing manually |
176 | ||
177 | Manually testing against an example file can be useful if you have added some | |
178 | `println!`s and the test suite output becomes unreadable. To try Clippy with | |
179 | your local modifications, run | |
180 | ||
181 | ``` | |
a2a8927a | 182 | cargo dev lint input.rs |
f20569fa XL |
183 | ``` |
184 | ||
185 | from the working copy root. With tests in place, let's have a look at | |
186 | implementing our lint now. | |
187 | ||
188 | ## Lint declaration | |
189 | ||
923072b8 FG |
190 | Let's start by opening the new file created in the `clippy_lints` crate at |
191 | `clippy_lints/src/foo_functions.rs`. That's the crate where all the lint code | |
192 | is. This file has already imported some initial things we will need: | |
f20569fa XL |
193 | |
194 | ```rust | |
195 | use rustc_lint::{EarlyLintPass, EarlyContext}; | |
196 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
197 | use rustc_ast::ast::*; | |
198 | ``` | |
199 | ||
200 | The next step is to update the lint declaration. Lints are declared using the | |
201 | [`declare_clippy_lint!`][declare_clippy_lint] macro, and we just need to update | |
923072b8 FG |
202 | the auto-generated lint declaration to have a real description, something like |
203 | this: | |
f20569fa XL |
204 | |
205 | ```rust | |
206 | declare_clippy_lint! { | |
94222f64 | 207 | /// ### What it does |
f20569fa | 208 | /// |
94222f64 | 209 | /// ### Why is this bad? |
f20569fa | 210 | /// |
94222f64 | 211 | /// ### Example |
f20569fa XL |
212 | /// ```rust |
213 | /// // example code | |
214 | /// ``` | |
a2a8927a | 215 | #[clippy::version = "1.29.0"] |
f20569fa XL |
216 | pub FOO_FUNCTIONS, |
217 | pedantic, | |
218 | "function named `foo`, which is not a descriptive name" | |
219 | } | |
220 | ``` | |
221 | ||
222 | * The section of lines prefixed with `///` constitutes the lint documentation | |
923072b8 FG |
223 | section. This is the default documentation style and will be displayed [like |
224 | this][example_lint_page]. To render and open this documentation locally in a | |
225 | browser, run `cargo dev serve`. | |
226 | * The `#[clippy::version]` attribute will be rendered as part of the lint | |
227 | documentation. The value should be set to the current Rust version that the | |
228 | lint is developed in, it can be retrieved by running `rustc -vV` in the | |
229 | rust-clippy directory. The version is listed under *release*. (Use the version | |
230 | without the `-nightly`) suffix. | |
231 | * `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the [lint naming | |
232 | guidelines][lint_naming] here when naming your lint. In short, the name should | |
233 | state the thing that is being checked for and read well when used with | |
234 | `allow`/`warn`/`deny`. | |
235 | * `pedantic` sets the lint level to `Allow`. The exact mapping can be found | |
236 | [here][category_level_mapping] | |
f20569fa XL |
237 | * The last part should be a text that explains what exactly is wrong with the |
238 | code | |
239 | ||
923072b8 FG |
240 | The rest of this file contains an empty implementation for our lint pass, which |
241 | in this case is `EarlyLintPass` and should look like this: | |
f20569fa XL |
242 | |
243 | ```rust | |
244 | // clippy_lints/src/foo_functions.rs | |
245 | ||
246 | // .. imports and lint declaration .. | |
247 | ||
248 | declare_lint_pass!(FooFunctions => [FOO_FUNCTIONS]); | |
249 | ||
250 | impl EarlyLintPass for FooFunctions {} | |
251 | ``` | |
252 | ||
3c0e092e XL |
253 | [declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L60 |
254 | [example_lint_page]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure | |
255 | [lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints | |
256 | [category_level_mapping]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L110 | |
257 | ||
258 | ## Lint registration | |
259 | ||
260 | When using `cargo dev new_lint`, the lint is automatically registered and | |
261 | nothing more has to be done. | |
262 | ||
263 | When declaring a new lint by hand and `cargo dev update_lints` is used, the lint | |
264 | pass may have to be registered manually in the `register_plugins` function in | |
265 | `clippy_lints/src/lib.rs`: | |
f20569fa XL |
266 | |
267 | ```rust | |
3c0e092e | 268 | store.register_early_pass(|| Box::new(foo_functions::FooFunctions)); |
f20569fa XL |
269 | ``` |
270 | ||
271 | As one may expect, there is a corresponding `register_late_pass` method | |
272 | available as well. Without a call to one of `register_early_pass` or | |
273 | `register_late_pass`, the lint pass in question will not be run. | |
274 | ||
3c0e092e XL |
275 | One reason that `cargo dev update_lints` does not automate this step is that |
276 | multiple lints can use the same lint pass, so registering the lint pass may | |
277 | already be done when adding a new lint. Another reason that this step is not | |
278 | automated is that the order that the passes are registered determines the order | |
279 | the passes actually run, which in turn affects the order that any emitted lints | |
280 | are output in. | |
f20569fa XL |
281 | |
282 | ## Lint passes | |
283 | ||
284 | Writing a lint that only checks for the name of a function means that we only | |
285 | have to deal with the AST and don't have to deal with the type system at all. | |
286 | This is good, because it makes writing this particular lint less complicated. | |
287 | ||
288 | We have to make this decision with every new Clippy lint. It boils down to using | |
289 | either [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass]. | |
290 | ||
291 | In short, the `LateLintPass` has access to type information while the | |
292 | `EarlyLintPass` doesn't. If you don't need access to type information, use the | |
293 | `EarlyLintPass`. The `EarlyLintPass` is also faster. However linting speed | |
294 | hasn't really been a concern with Clippy so far. | |
295 | ||
296 | Since we don't need type information for checking the function name, we used | |
297 | `--pass=early` when running the new lint automation and all the imports were | |
298 | added accordingly. | |
299 | ||
300 | [early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html | |
301 | [late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html | |
302 | ||
303 | ## Emitting a lint | |
304 | ||
305 | With UI tests and the lint declaration in place, we can start working on the | |
306 | implementation of the lint logic. | |
307 | ||
308 | Let's start by implementing the `EarlyLintPass` for our `FooFunctions`: | |
309 | ||
310 | ```rust | |
311 | impl EarlyLintPass for FooFunctions { | |
312 | fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { | |
313 | // TODO: Emit lint here | |
314 | } | |
315 | } | |
316 | ``` | |
317 | ||
318 | We implement the [`check_fn`][check_fn] method from the | |
319 | [`EarlyLintPass`][early_lint_pass] trait. This gives us access to various | |
320 | information about the function that is currently being checked. More on that in | |
321 | the next section. Let's worry about the details later and emit our lint for | |
322 | *every* function definition first. | |
323 | ||
324 | Depending on how complex we want our lint message to be, we can choose from a | |
325 | variety of lint emission functions. They can all be found in | |
326 | [`clippy_utils/src/diagnostics.rs`][diagnostics]. | |
327 | ||
328 | `span_lint_and_help` seems most appropriate in this case. It allows us to | |
329 | provide an extra help message and we can't really suggest a better name | |
330 | automatically. This is how it looks: | |
331 | ||
332 | ```rust | |
333 | impl EarlyLintPass for FooFunctions { | |
334 | fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { | |
335 | span_lint_and_help( | |
336 | cx, | |
337 | FOO_FUNCTIONS, | |
338 | span, | |
339 | "function named `foo`", | |
340 | None, | |
341 | "consider using a more meaningful name" | |
342 | ); | |
343 | } | |
344 | } | |
345 | ``` | |
346 | ||
347 | Running our UI test should now produce output that contains the lint message. | |
348 | ||
349 | According to [the rustc-dev-guide], the text should be matter of fact and avoid | |
923072b8 FG |
350 | capitalization and periods, unless multiple sentences are needed. When code or |
351 | an identifier must appear in a message or label, it should be surrounded with | |
352 | single grave accents \`. | |
f20569fa XL |
353 | |
354 | [check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html#method.check_fn | |
355 | [diagnostics]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/diagnostics.rs | |
356 | [the rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/diagnostics.html | |
357 | ||
358 | ## Adding the lint logic | |
359 | ||
360 | Writing the logic for your lint will most likely be different from our example, | |
361 | so this section is kept rather short. | |
362 | ||
363 | Using the [`check_fn`][check_fn] method gives us access to [`FnKind`][fn_kind] | |
364 | that has the [`FnKind::Fn`] variant. It provides access to the name of the | |
365 | function/method via an [`Ident`][ident]. | |
366 | ||
367 | With that we can expand our `check_fn` method to: | |
368 | ||
369 | ```rust | |
370 | impl EarlyLintPass for FooFunctions { | |
371 | fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { | |
372 | if is_foo_fn(fn_kind) { | |
373 | span_lint_and_help( | |
374 | cx, | |
375 | FOO_FUNCTIONS, | |
376 | span, | |
377 | "function named `foo`", | |
378 | None, | |
379 | "consider using a more meaningful name" | |
380 | ); | |
381 | } | |
382 | } | |
383 | } | |
384 | ``` | |
385 | ||
386 | We separate the lint conditional from the lint emissions because it makes the | |
387 | code a bit easier to read. In some cases this separation would also allow to | |
388 | write some unit tests (as opposed to only UI tests) for the separate function. | |
389 | ||
390 | In our example, `is_foo_fn` looks like: | |
391 | ||
392 | ```rust | |
393 | // use statements, impl EarlyLintPass, check_fn, .. | |
394 | ||
395 | fn is_foo_fn(fn_kind: FnKind<'_>) -> bool { | |
396 | match fn_kind { | |
397 | FnKind::Fn(_, ident, ..) => { | |
398 | // check if `fn` name is `foo` | |
399 | ident.name.as_str() == "foo" | |
400 | } | |
401 | // ignore closures | |
402 | FnKind::Closure(..) => false | |
403 | } | |
404 | } | |
405 | ``` | |
406 | ||
407 | Now we should also run the full test suite with `cargo test`. At this point | |
923072b8 FG |
408 | running `cargo test` should produce the expected output. Remember to run `cargo |
409 | dev bless` to update the `.stderr` file. | |
f20569fa XL |
410 | |
411 | `cargo test` (as opposed to `cargo uitest`) will also ensure that our lint | |
412 | implementation is not violating any Clippy lints itself. | |
413 | ||
414 | That should be it for the lint implementation. Running `cargo test` should now | |
415 | pass. | |
416 | ||
417 | [fn_kind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html | |
418 | [`FnKind::Fn`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html#variant.Fn | |
419 | [ident]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/struct.Ident.html | |
420 | ||
cdc7bbd5 | 421 | ## Specifying the lint's minimum supported Rust version (MSRV) |
f20569fa | 422 | |
923072b8 FG |
423 | Sometimes a lint makes suggestions that require a certain version of Rust. For |
424 | example, the `manual_strip` lint suggests using `str::strip_prefix` and | |
425 | `str::strip_suffix` which is only available after Rust 1.45. In such cases, you | |
426 | need to ensure that the MSRV configured for the project is >= the MSRV of the | |
427 | required Rust feature. If multiple features are required, just use the one with | |
428 | a lower MSRV. | |
cdc7bbd5 | 429 | |
064997fb FG |
430 | First, add an MSRV alias for the required feature in [`clippy_utils::msrvs`]. |
431 | This can be accessed later as `msrvs::STR_STRIP_PREFIX`, for example. | |
f20569fa XL |
432 | |
433 | ```rust | |
cdc7bbd5 XL |
434 | msrv_aliases! { |
435 | .. | |
436 | 1,45,0 { STR_STRIP_PREFIX } | |
437 | } | |
f20569fa XL |
438 | ``` |
439 | ||
923072b8 FG |
440 | In order to access the project-configured MSRV, you need to have an `msrv` field |
441 | in the LintPass struct, and a constructor to initialize the field. The `msrv` | |
442 | value is passed to the constructor in `clippy_lints/lib.rs`. | |
f20569fa XL |
443 | |
444 | ```rust | |
445 | pub struct ManualStrip { | |
446 | msrv: Option<RustcVersion>, | |
447 | } | |
448 | ||
449 | impl ManualStrip { | |
450 | #[must_use] | |
451 | pub fn new(msrv: Option<RustcVersion>) -> Self { | |
452 | Self { msrv } | |
453 | } | |
454 | } | |
455 | ``` | |
456 | ||
cdc7bbd5 XL |
457 | The project's MSRV can then be matched against the feature MSRV in the LintPass |
458 | using the `meets_msrv` utility function. | |
f20569fa XL |
459 | |
460 | ``` rust | |
923072b8 | 461 | if !meets_msrv(self.msrv, msrvs::STR_STRIP_PREFIX) { |
f20569fa XL |
462 | return; |
463 | } | |
464 | ``` | |
465 | ||
cdc7bbd5 XL |
466 | The project's MSRV can also be specified as an inner attribute, which overrides |
467 | the value from `clippy.toml`. This can be accounted for using the | |
468 | `extract_msrv_attr!(LintContext)` macro and passing | |
469 | `LateContext`/`EarlyContext`. | |
f20569fa XL |
470 | |
471 | ```rust | |
472 | impl<'tcx> LateLintPass<'tcx> for ManualStrip { | |
473 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | |
474 | ... | |
475 | } | |
476 | extract_msrv_attr!(LateContext); | |
477 | } | |
478 | ``` | |
479 | ||
cdc7bbd5 | 480 | Once the `msrv` is added to the lint, a relevant test case should be added to |
2b03887a FG |
481 | the lint's test file, `tests/ui/manual_strip.rs` in this example. It should |
482 | have a case for the version below the MSRV and one with the same contents but | |
483 | for the MSRV version itself. | |
484 | ||
485 | ```rust | |
486 | #![feature(custom_inner_attributes)] | |
487 | ||
488 | ... | |
489 | ||
490 | fn msrv_1_44() { | |
491 | #![clippy::msrv = "1.44"] | |
492 | ||
493 | /* something that would trigger the lint */ | |
494 | } | |
495 | ||
496 | fn msrv_1_45() { | |
497 | #![clippy::msrv = "1.45"] | |
498 | ||
499 | /* something that would trigger the lint */ | |
500 | } | |
501 | ``` | |
cdc7bbd5 XL |
502 | |
503 | As a last step, the lint should be added to the lint documentation. This is done | |
504 | in `clippy_lints/src/utils/conf.rs`: | |
505 | ||
506 | ```rust | |
507 | define_Conf! { | |
508 | /// Lint: LIST, OF, LINTS, <THE_NEWLY_ADDED_LINT>. The minimum rust version that the project supports | |
17df50a5 | 509 | (msrv: Option<String> = None), |
cdc7bbd5 XL |
510 | ... |
511 | } | |
512 | ``` | |
f20569fa | 513 | |
064997fb FG |
514 | [`clippy_utils::msrvs`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/msrvs/index.html |
515 | ||
f20569fa XL |
516 | ## Author lint |
517 | ||
518 | If you have trouble implementing your lint, there is also the internal `author` | |
519 | lint to generate Clippy code that detects the offending pattern. It does not | |
520 | work for all of the Rust syntax, but can give a good starting point. | |
521 | ||
923072b8 FG |
522 | The quickest way to use it, is the [Rust playground: |
523 | play.rust-lang.org][author_example]. Put the code you want to lint into the | |
524 | editor and add the `#[clippy::author]` attribute above the item. Then run Clippy | |
525 | via `Tools -> Clippy` and you should see the generated code in the output below. | |
f20569fa XL |
526 | |
527 | [Here][author_example] is an example on the playground. | |
528 | ||
529 | If the command was executed successfully, you can copy the code over to where | |
530 | you are implementing your lint. | |
531 | ||
532 | [author_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=9a12cb60e5c6ad4e3003ac6d5e63cf55 | |
533 | ||
04454e1e FG |
534 | ## Print HIR lint |
535 | ||
923072b8 FG |
536 | To implement a lint, it's helpful to first understand the internal |
537 | representation that rustc uses. Clippy has the `#[clippy::dump]` attribute that | |
538 | prints the [_High-Level Intermediate Representation (HIR)_] of the item, | |
539 | statement, or expression that the attribute is attached to. To attach the | |
540 | attribute to expressions you often need to enable | |
541 | `#![feature(stmt_expr_attributes)]`. | |
04454e1e | 542 | |
923072b8 FG |
543 | [Here][print_hir_example] you can find an example, just select _Tools_ and run |
544 | _Clippy_. | |
04454e1e FG |
545 | |
546 | [_High-Level Intermediate Representation (HIR)_]: https://rustc-dev-guide.rust-lang.org/hir.html | |
547 | [print_hir_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=daf14db3a7f39ca467cd1b86c34b9afb | |
548 | ||
f20569fa XL |
549 | ## Documentation |
550 | ||
551 | The final thing before submitting our PR is to add some documentation to our | |
552 | lint declaration. | |
553 | ||
554 | Please document your lint with a doc comment akin to the following: | |
555 | ||
556 | ```rust | |
557 | declare_clippy_lint! { | |
94222f64 XL |
558 | /// ### What it does |
559 | /// Checks for ... (describe what the lint matches). | |
f20569fa | 560 | /// |
94222f64 XL |
561 | /// ### Why is this bad? |
562 | /// Supply the reason for linting the code. | |
f20569fa | 563 | /// |
94222f64 | 564 | /// ### Example |
f20569fa XL |
565 | /// |
566 | /// ```rust,ignore | |
923072b8 FG |
567 | /// // A short example of code that triggers the lint |
568 | /// ``` | |
f20569fa | 569 | /// |
923072b8 FG |
570 | /// Use instead: |
571 | /// ```rust,ignore | |
572 | /// // A short example of improved code that doesn't trigger the lint | |
f20569fa | 573 | /// ``` |
a2a8927a | 574 | #[clippy::version = "1.29.0"] |
f20569fa XL |
575 | pub FOO_FUNCTIONS, |
576 | pedantic, | |
577 | "function named `foo`, which is not a descriptive name" | |
578 | } | |
579 | ``` | |
580 | ||
581 | Once your lint is merged, this documentation will show up in the [lint | |
582 | list][lint_list]. | |
583 | ||
584 | [lint_list]: https://rust-lang.github.io/rust-clippy/master/index.html | |
585 | ||
586 | ## Running rustfmt | |
587 | ||
923072b8 FG |
588 | [Rustfmt] is a tool for formatting Rust code according to style guidelines. Your |
589 | code has to be formatted by `rustfmt` before a PR can be merged. Clippy uses | |
590 | nightly `rustfmt` in the CI. | |
f20569fa XL |
591 | |
592 | It can be installed via `rustup`: | |
593 | ||
594 | ```bash | |
595 | rustup component add rustfmt --toolchain=nightly | |
596 | ``` | |
597 | ||
598 | Use `cargo dev fmt` to format the whole codebase. Make sure that `rustfmt` is | |
599 | installed for the nightly toolchain. | |
600 | ||
601 | [Rustfmt]: https://github.com/rust-lang/rustfmt | |
602 | ||
603 | ## Debugging | |
604 | ||
605 | If you want to debug parts of your lint implementation, you can use the [`dbg!`] | |
606 | macro anywhere in your code. Running the tests should then include the debug | |
607 | output in the `stdout` part. | |
608 | ||
609 | [`dbg!`]: https://doc.rust-lang.org/std/macro.dbg.html | |
610 | ||
611 | ## PR Checklist | |
612 | ||
613 | Before submitting your PR make sure you followed all of the basic requirements: | |
614 | ||
615 | <!-- Sync this with `.github/PULL_REQUEST_TEMPLATE` --> | |
616 | ||
617 | - \[ ] Followed [lint naming conventions][lint_naming] | |
618 | - \[ ] Added passing UI tests (including committed `.stderr` file) | |
619 | - \[ ] `cargo test` passes locally | |
620 | - \[ ] Executed `cargo dev update_lints` | |
621 | - \[ ] Added lint documentation | |
622 | - \[ ] Run `cargo dev fmt` | |
623 | ||
624 | ## Adding configuration to a lint | |
625 | ||
923072b8 FG |
626 | Clippy supports the configuration of lints values using a `clippy.toml` file in |
627 | the workspace directory. Adding a configuration to a lint can be useful for | |
628 | thresholds or to constrain some behavior that can be seen as a false positive | |
629 | for some users. Adding a configuration is done in the following steps: | |
f20569fa | 630 | |
064997fb | 631 | 1. Adding a new configuration entry to [`clippy_lints::utils::conf`] like this: |
f20569fa | 632 | |
923072b8 FG |
633 | ```rust |
634 | /// Lint: LINT_NAME. | |
635 | /// | |
636 | /// <The configuration field doc comment> | |
637 | (configuration_ident: Type = DefaultValue), | |
638 | ``` | |
f20569fa | 639 | |
923072b8 FG |
640 | The doc comment is automatically added to the documentation of the listed |
641 | lints. The default value will be formatted using the `Debug` implementation | |
642 | of the type. | |
643 | 2. Adding the configuration value to the lint impl struct: | |
644 | 1. This first requires the definition of a lint impl struct. Lint impl | |
645 | structs are usually generated with the `declare_lint_pass!` macro. This | |
646 | struct needs to be defined manually to add some kind of metadata to it: | |
647 | ```rust | |
648 | // Generated struct definition | |
649 | declare_lint_pass!(StructName => [ | |
650 | LINT_NAME | |
651 | ]); | |
652 | ||
653 | // New manual definition struct | |
654 | #[derive(Copy, Clone)] | |
655 | pub struct StructName {} | |
656 | ||
657 | impl_lint_pass!(StructName => [ | |
658 | LINT_NAME | |
659 | ]); | |
660 | ``` | |
661 | ||
662 | 2. Next add the configuration value and a corresponding creation method like | |
663 | this: | |
664 | ```rust | |
665 | #[derive(Copy, Clone)] | |
666 | pub struct StructName { | |
667 | configuration_ident: Type, | |
668 | } | |
669 | ||
670 | // ... | |
671 | ||
672 | impl StructName { | |
673 | pub fn new(configuration_ident: Type) -> Self { | |
674 | Self { | |
675 | configuration_ident, | |
676 | } | |
677 | } | |
678 | } | |
679 | ``` | |
f20569fa XL |
680 | 3. Passing the configuration value to the lint impl struct: |
681 | ||
064997fb FG |
682 | First find the struct construction in the [`clippy_lints` lib file]. The |
683 | configuration value is now cloned or copied into a local value that is then | |
684 | passed to the impl struct like this: | |
923072b8 FG |
685 | |
686 | ```rust | |
687 | // Default generated registration: | |
688 | store.register_*_pass(|| box module::StructName); | |
f20569fa | 689 | |
923072b8 FG |
690 | // New registration with configuration value |
691 | let configuration_ident = conf.configuration_ident.clone(); | |
692 | store.register_*_pass(move || box module::StructName::new(configuration_ident)); | |
693 | ``` | |
f20569fa | 694 | |
923072b8 FG |
695 | Congratulations the work is almost done. The configuration value can now be |
696 | accessed in the linting code via `self.configuration_ident`. | |
f20569fa XL |
697 | |
698 | 4. Adding tests: | |
923072b8 | 699 | 1. The default configured value can be tested like any normal lint in |
064997fb FG |
700 | [`tests/ui`]. |
701 | 2. The configuration itself will be tested separately in [`tests/ui-toml`]. | |
702 | Simply add a new subfolder with a fitting name. This folder contains a | |
703 | `clippy.toml` file with the configuration value and a rust file that | |
704 | should be linted by Clippy. The test can otherwise be written as usual. | |
705 | ||
706 | [`clippy_lints::utils::conf`]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/conf.rs | |
707 | [`clippy_lints` lib file]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/lib.rs | |
708 | [`tests/ui`]: https://github.com/rust-lang/rust-clippy/blob/master/tests/ui | |
709 | [`tests/ui-toml`]: https://github.com/rust-lang/rust-clippy/blob/master/tests/ui-toml | |
f20569fa | 710 | |
923072b8 | 711 | ## Cheat Sheet |
f20569fa XL |
712 | |
713 | Here are some pointers to things you are likely going to need for every lint: | |
714 | ||
715 | * [Clippy utils][utils] - Various helper functions. Maybe the function you need | |
923072b8 FG |
716 | is already in here ([`is_type_diagnostic_item`], [`implements_trait`], |
717 | [`snippet`], etc) | |
f20569fa | 718 | * [Clippy diagnostics][diagnostics] |
923072b8 FG |
719 | * [Let chains][let-chains] |
720 | * [`from_expansion`][from_expansion] and | |
721 | [`in_external_macro`][in_external_macro] | |
f20569fa XL |
722 | * [`Span`][span] |
723 | * [`Applicability`][applicability] | |
923072b8 FG |
724 | * [Common tools for writing lints](common_tools_writing_lints.md) helps with |
725 | common operations | |
726 | * [The rustc-dev-guide][rustc-dev-guide] explains a lot of internal compiler | |
727 | concepts | |
f20569fa XL |
728 | * [The nightly rustc docs][nightly_docs] which has been linked to throughout |
729 | this guide | |
730 | ||
731 | For `EarlyLintPass` lints: | |
732 | ||
733 | * [`EarlyLintPass`][early_lint_pass] | |
734 | * [`rustc_ast::ast`][ast] | |
735 | ||
736 | For `LateLintPass` lints: | |
737 | ||
738 | * [`LateLintPass`][late_lint_pass] | |
739 | * [`Ty::TyKind`][ty] | |
740 | ||
741 | While most of Clippy's lint utils are documented, most of rustc's internals lack | |
742 | documentation currently. This is unfortunate, but in most cases you can probably | |
743 | get away with copying things from existing similar lints. If you are stuck, | |
744 | don't hesitate to ask on [Zulip] or in the issue/PR. | |
745 | ||
a2a8927a XL |
746 | [utils]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/index.html |
747 | [`is_type_diagnostic_item`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.is_type_diagnostic_item.html | |
748 | [`implements_trait`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.implements_trait.html | |
749 | [`snippet`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/source/fn.snippet.html | |
923072b8 | 750 | [let-chains]: https://github.com/rust-lang/rust/pull/94927 |
f20569fa XL |
751 | [from_expansion]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html#method.from_expansion |
752 | [in_external_macro]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/lint/fn.in_external_macro.html | |
753 | [span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html | |
754 | [applicability]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html | |
755 | [rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/ | |
756 | [nightly_docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ | |
757 | [ast]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/index.html | |
758 | [ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/sty/index.html | |
759 | [Zulip]: https://rust-lang.zulipchat.com/#narrow/stream/clippy |