Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clippy Book Chapter Updates Reborn: Writing tests #10596

Merged
merged 4 commits into from
Sep 2, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- [Basics](development/basics.md)
- [Adding Lints](development/adding_lints.md)
- [Defining Lints](development/defining_lints.md)
- [Writing tests](development/writing_tests.md)
- [Lint Passes](development/lint_passes.md)
- [Type Checking](development/type_checking.md)
- [Method Checking](development/method_checking.md)
Expand Down
220 changes: 220 additions & 0 deletions book/src/development/writing_tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
# Testing

Developing lints for Clippy is a Test-Driven Development (TDD) process because
our first task before implementing any logic for a new lint is to write some test cases.

## Develop Lints with Tests

When we develop Clippy, we enter a complex and chaotic realm full of
programmatic issues, stylistic errors, illogical code and non-adherence to convention.
Tests are the first layer of order we can leverage to define when and where
we want a new lint to trigger or not.

Moreover, writing tests first help Clippy developers to find a balance for
the first iteration of and further enhancements for a lint.
With test cases on our side, we will not have to worry about over-engineering
a lint on its first version nor missing out some obvious edge cases of the lint.
This approach empowers us to iteratively enhance each lint.

## Clippy UI Tests

We use **UI tests** for testing in Clippy.
These UI tests check that the output of Clippy is exactly as we expect it to be.
Each test is just a plain Rust file that contains the code we want to check.

The output of Clippy is compared against a `.stderr` file.
Note that you don't have to create this file yourself.
We'll get to generating the `.stderr` files with the command [`cargo dev bless`](#cargo-dev-bless) (seen later on).
flip1995 marked this conversation as resolved.
Show resolved Hide resolved

### Write Test Cases

Let us now think about some tests for our imaginary `foo_functions` lint,
We start by opening the test file `tests/ui/foo_functions.rs` that was created by `cargo dev new_lint`.

Update the file with some examples to get started:

```rust
#![warn(clippy::foo_functions)]
// Impl methods
struct A;
impl A {
pub fn fo(&self) {}
pub fn foo(&self) {}
pub fn food(&self) {}
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
}

// Default trait methods
trait B {
fn fo(&self) {}
fn foo(&self) {}
fn food(&self) {}
}

// Plain functions
fn fo() {}
fn foo() {}
fn food() {}

fn main() {
// We also don't want to lint method calls
foo();
let a = A;
a.foo();
}
```

Without actual lint logic to emit the lint when we see a `foo` function name,
these tests are still quite meaningless.
However, we can now run the test with the following command:

```sh
$ TESTNAME=foo_functions cargo uitest
```

Clippy will compile and it will conclude with an `ok` for the tests:

```
...Clippy warnings and test outputs...
test compile_test ... ok
test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.48s
```

This is normal. After all, we wrote a bunch of Rust code but we haven't really
implemented any logic for Clippy to detect `foo` functions and emit a lint.

As we gradually implement our lint logic, we will keep running this UI test command.
Clippy will begin outputting information that allows us to check if the output is
turning into what we want it to be.

blyxyas marked this conversation as resolved.
Show resolved Hide resolved
### Example output

As our `foo_functions` lint is tested, the output would look something like this:

```
failures:

---- compile_test stdout ----
normalized stderr:
error: function called "foo"
--> $DIR/foo_functions.rs:6:12
|
LL | pub fn foo(&self) {}
| ^^^
|
= note: `-D clippy::foo-functions` implied by `-D warnings`

error: function called "foo"
--> $DIR/foo_functions.rs:13:8
|
LL | fn foo(&self) {}
| ^^^

error: function called "foo"
--> $DIR/foo_functions.rs:19:4
|
LL | fn foo() {}
| ^^^

error: aborting due to 3 previous errors
```

Note the *failures* label at the top of the fragment, we'll get rid of it (saving this output) in the next section.

> _Note:_ You can run multiple test files by specifying a comma separated list:
> `TESTNAME=foo_functions,bar_methods,baz_structs`.
### `cargo dev bless`
flip1995 marked this conversation as resolved.
Show resolved Hide resolved

Once we are satisfied with the output, we need to run this command to
generate or update the `.stderr` file for our lint:

```sh
$ TESTNAME=foo_functions cargo uitest
# (Output is as we want it to be)
$ cargo dev bless
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
```

This write the emitted lint suggestions and fixes to the `.stderr` file,
with the reason for the lint, suggested fixes, and line numbers, etc.

> _Note:_ we need to run `TESTNAME=foo_functions cargo uitest` every time before we run
> `cargo dev bless`.

flip1995 marked this conversation as resolved.
Show resolved Hide resolved
Running `TESTNAME=foo_functions cargo uitest` should pass then. When we
commit our lint, we need to commit the generated `.stderr` files, too.

In general, you should only commit files changed by `cargo dev bless` for the
specific lint you are creating/editing.

> _Note:_ If the generated `.stderr`, and `.fixed` files are empty,
> they should be removed.

## Cargo Lints

The process of testing is different for Cargo lints in that now we are
interested in the `Cargo.toml` manifest file.
In this case, we also need a minimal crate associated with that manifest.

Imagine we have a new example lint that is named `foo_categories`, we can run:

```sh
$ cargo dev new_lint --name=foo_categories --pass=late --category=cargo
```

After running `cargo dev new_lint` we will find by default two new crates,
each with its manifest file:

* `tests/ui-cargo/foo_categories/fail/Cargo.toml`: this file should cause the
new lint to raise an error.
* `tests/ui-cargo/foo_categories/pass/Cargo.toml`: this file should not trigger
the lint.

If you need more cases, you can copy one of those crates (under `foo_categories`) and rename it.

The process of generating the `.stderr` file is the same as for other lints
and prepending the `TESTNAME` variable to `cargo uitest` works for Cargo lints too.

Overall, you should see the following changes when you generate a new Cargo lint:

```sh
$ git status
On branch foo_categories
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: CHANGELOG.md
modified: clippy_lints/src/cargo/mod.rs
modified: clippy_lints/src/lib.rs
Untracked files:
(use "git add <file>..." to include in what will be committed)
clippy_lints/src/cargo/foo_categories.rs
tests/ui-cargo/foo_categories/
```

## Rustfix Tests

If the lint you are working on is making use of structured suggestions, the test
file should include a `// run-rustfix` comment at the top.

Structured suggestions tell a user how to fix or re-write certain code that has been linted, they are usually linted
with [`span_lint_and_sugg`](https://doc.rust-lang.org/beta/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_sugg.html).
blyxyas marked this conversation as resolved.
Show resolved Hide resolved

The `// run-rustfix` comment will additionally run [rustfix] for our test.
Rustfix will apply the suggestions from the lint to the test file code and
compare that to the contents of a `.fixed` file.

We'll talk about suggestions more in depth in a later chapter.
<!-- FIXME: (blyyas) Link to "Emitting lints" when that gets merged -->

Use `cargo dev bless` to automatically generate the `.fixed` file after running the tests.

[rustfix]: https://github.com/rust-lang/rustfix
## Testing Manually
flip1995 marked this conversation as resolved.
Show resolved Hide resolved

Manually testing against an example file can be useful if you have added some
`println!`s and the test suite output becomes unreadable.

To try Clippy with your local modifications, run from the working copy root.

```sh
$ cargo dev lint input.rs
```