Skip to content

Commit befa9b5

Browse files
committed
Expand on lint implementation section, wrap lines
1 parent 0ad89b4 commit befa9b5

File tree

1 file changed

+65
-23
lines changed

1 file changed

+65
-23
lines changed

doc/adding_lints.md

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ use rustc::lint::{LintArray, LintPass};
8888
use rustc::{declare_tool_lint, lint_array};
8989
```
9090

91-
The next step is to provide a lint declaration. Lints are declared using the [`declare_clippy_lint!`][declare_clippy_lint] macro:
91+
The next step is to provide a lint declaration. Lints are declared using the
92+
[`declare_clippy_lint!`][declare_clippy_lint] macro:
9293

9394
```rust
9495
declare_clippy_lint! {
@@ -107,7 +108,8 @@ state the thing that is being checked for and read well when used with
107108
* The last part should be a text that explains what exactly is wrong with the
108109
code
109110

110-
With our lint declaration done, we will now make sure that our lint declaration is assigned to a lint pass:
111+
With our lint declaration done, we will now make sure that our lint declaration
112+
is assigned to a lint pass:
111113

112114
```rust
113115
// clippy_lints/src/foo_functions.rs
@@ -130,31 +132,43 @@ impl LintPass for FooFunctionsPass {
130132
}
131133
```
132134

133-
Don't worry about the `name` method here. As long as it includes the name of the lint pass it should be fine.
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.
134137

135138
Next you should run `util/dev update_lints` to register the lint in various
136139
places, mainly in `clippy_lints/src/lib.rs`.
137140

138-
While `update_lints` automates some things, it doesn't automate everything. We will have to register our lint pass manually in the `register_plugins` function in `clippy_lints/src/lib.rs`:
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`:
139144

140145
```rust
141146
reg.register_early_lint_pass(box foo_functions::FooFunctionsPass);
142147
```
143148

144-
Without that, running the UI tests would produce an error like `unknown clippy lint: clippy::foo_functions`.
145-
The next decision we have to make is which lint pass our lint is going to need.
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.
146152

147153
### Lint passes
148154

149155
Writing a lint that just checks for the name of a function means that we just
150156
have to deal with the AST and don't have to deal with the type system at all.
151157
This is good, because it makes writing this particular lint less complicated.
152158

153-
We have to make this decision with every new Clippy lint. It boils down to using either `EarlyLintPass` or `LateLintPass`. This is a result of Rust's compilation process. You can read more about it [in the rustc guide][compilation_stages].
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].
154163

155-
In short, the `LateLintPass` has access to type information while the `EarlyLintPass` doesn't. If you don't need access to type information, use the `EarlyLintPass`. The `EarlyLintPass` is also faster. However linting speed hasn't really been a concern with Clippy so far.
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.
156168

157-
Since we don't need type information for checking the function name, we are going to use the `EarlyLintPass`. It has to be imported as well, changing our imports to:
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:
158172

159173
```rust
160174
use rustc::lint::{LintArray, LintPass, EarlyLintPass, EarlyContext};
@@ -165,20 +179,25 @@ use rustc::{declare_tool_lint, lint_array};
165179

166180
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.
167181

168-
Let's start by emitting a lint for every function definition.
182+
Let's start by implementing the `EarlyLintPass` for our `FooFunctionsPass`:
169183

170184
```rust
171-
impl EarlyLintPass for Pass {
185+
impl EarlyLintPass for FooFunctionsPass {
172186
fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) {
173187
// TODO: Emit lint here
174188
}
175189
}
176190
```
177191

178-
We implement the [`check_fn`][check_fn] method from the [`EarlyLintPass`][early_lint_pass] trait. This gives us access to various information about the function that is currently being checked. More on that in the next section. Let's worry about the details later and emit our lint for *every* function definition first.
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.
179197

180-
Depending on how complex we want our lint message to be, we can choose from a variety of lint emission functions.
181-
They can all be found in [`clippy_lints/src/utils/diagnostics.rs`][diagnostics].
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].
182201

183202

184203
```rust
@@ -203,12 +222,33 @@ so this section is kept rather short.
203222
Using the [`check_fn`][check_fn] method gives us access to [`FnKind`][fn_kind]
204223
that has two relevant variants for us `FnKind::ItemFn` and `FnKind::Method`.
205224
Both provide access to the name of the function/method via an [`Ident`][ident].
206-
and delegate our check to it's own function, passing through only the data we
207-
need.
208225

209-
In our example, the implementation would look like:
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:
210248

211249
```rust
250+
// use statements, impl EarlyLintPass, check_fn, ..
251+
212252
fn is_foo_fn(fn_kind: FnKind<'_>) -> bool {
213253
match fn_kind {
214254
FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) => {
@@ -219,12 +259,12 @@ fn is_foo_fn(fn_kind: FnKind<'_>) -> bool {
219259
}
220260
```
221261

222-
223262
Now you'll want to also run the full test suite with `cargo test`. Apart from
224263
running all other UI tests, this ensures that our lint implementation is not
225-
violating any Clippy lints itself. If you are still following the example,
226-
you'll see that the `FooFunctionsPass` violates a Clippy lint. So we are going
227-
to rename that struct to just `Pass`:
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`:
228268

229269
```rust
230270
#[derive(Copy, Clone)]
@@ -233,6 +273,8 @@ pub struct Pass;
233273
impl LintPass for Pass { /* .. */ }
234274
```
235275

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.
236278

237279
### Documentation
238280

@@ -255,9 +297,9 @@ Please document your lint with a doc comment akin to the following:
255297
/// Insert a short example of code that triggers the lint
256298
///
257299
/// // Good
258-
/// Insert a short example of improved code that doesnt trigger the lint
300+
/// Insert a short example of improved code that doesn't trigger the lint
259301
/// ```
260-
declare_clippy_lint! // ...
302+
declare_clippy_lint! { /* ... */ }
261303
```
262304

263305
Once your lint is merged, this documentation will show up in the [lint

0 commit comments

Comments
 (0)