Skip to content

Commit

Permalink
Auto merge of #3824 - phansch:adding_lints, r=phansch
Browse files Browse the repository at this point in the history 
Add lint writing documentation

[Rendered](https://github.com/phansch/rust-clippy/blob/adding_lints/doc/adding_lints.md)

This adds a new documentation page that explains how to write Clippy
lints. It guides the reader through creating a `foo` function lint.

I plan to iterate a bit more on the prose of some sections, but I think the
general structure is fine now, so I'm looking forward to feedback =)

One thing I'm not sure about: I felt like this is too big for CONTRIBUTING.md
so I put it into a new `doc/` directory. I can imagine having more
documentation in the future, so we might even want to create a book using
mdbook instead? Or should everything go into CONTRIBUTING.md?

Further things left to do:

- [x] Link from CONTRIBUTING.md
- [x] Remove things covered in this guide from CONTRIBUTING.md
- [x] Section about `clippy::author` attribute
- [x] Run `remark-lint` on CI over the `doc` directory and fix things
  • Loading branch information
@bors
bors committed Mar 9, 2019
2 parents ccfbfb8 + 34685a5 commit 920112d
Show file tree
Hide file tree
Showing 4 changed files with 455 additions and 117 deletions.
117 changes: 3 additions & 114 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ All contributors are expected to follow the [Rust Code of Conduct](http://www.ru
* [Getting started](#getting-started)
* [Finding something to fix/improve](#finding-something-to-fiximprove)
* [Writing code](#writing-code)
* [Author lint](#author-lint)
* [Documentation](#documentation)
* [Running test suite](#running-test-suite)
* [Running rustfmt](#running-rustfmt)
* [Testing manually](#testing-manually)
* [How Clippy works](#how-clippy-works)
* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
* [Issue and PR Triage](#issue-and-pr-triage)
Expand Down Expand Up @@ -73,121 +68,15 @@ an AST expression). `match_def_path()` in Clippy's `utils` module can also be us

## Writing code

Clippy depends on the current git master version of rustc, which can change rapidly. Make sure you're
working near rust-clippy's master, and use the `setup-toolchain.sh` script to configure the appropriate
toolchain for this directory.

[Llogiq's blog post on lints](https://llogiq.github.io/2015/06/04/workflows.html) is a nice primer
to lint-writing, though it does get into advanced stuff. Most lints consist of an implementation of
`LintPass` with one or more of its default methods overridden. See the existing lints for examples
of this.
Have a look at the [docs for writing lints](doc/adding_lints.md) for more details. [Llogiq's blog post on lints](https://llogiq.github.io/2015/06/04/workflows.html) is also a nice primer
to lint-writing, though it does get into advanced stuff and may be a bit
outdated.

If you want to add a new lint or change existing ones apart from bugfixing, it's
also a good idea to give the [stability guarantees][rfc_stability] and
[lint categories][rfc_lint_cats] sections of the [Clippy 1.0 RFC][clippy_rfc] a
quick read.

### Author lint

There is also the internal `author` lint to generate Clippy code that detects the offending pattern. It does not work for all of the Rust syntax, but can give a good starting point.

First, create a new UI test file in the `tests/ui/` directory with the pattern you want to match:

```rust
// ./tests/ui/my_lint.rs
fn main() {
#[clippy::author]
let arr: [i32; 1] = [7]; // Replace line with the code you want to match
}
```

Now you run `TESTNAME=ui/my_lint cargo uitest` to produce
a `.stdout` file with the generated code:

```rust
// ./tests/ui/my_lint.stdout

if_chain! {
if let ExprKind::Array(ref elements) = stmt.node;
if elements.len() == 1;
if let ExprKind::Lit(ref lit) = elements[0].node;
if let LitKind::Int(7, _) = lit.node;
then {
// report your lint here
}
}
```

If the command was executed successfully, you can copy the code over to where you are implementing your lint.

### Documentation

Please document your lint with a doc comment akin to the following:

```rust
/// **What it does:** Checks for ... (describe what the lint matches).
///
/// **Why is this bad?** Supply the reason for linting the code.
///
/// **Known problems:** None. (Or describe where it could go wrong.)
///
/// **Example:**
///
/// ```rust
/// // Bad
/// Insert a short example of code that triggers the lint
///
/// // Good
/// Insert a short example of improved code that doesn't trigger the lint
/// ```
```

Once your lint is merged it will show up in the [lint list](https://rust-lang.github.io/rust-clippy/master/index.html)

### Running test suite

Use `cargo test` to run the whole testsuite.

If you don't want to wait for all tests to finish, you can also execute a single test file by using `TESTNAME` to specify the test to run:

```bash
TESTNAME=ui/empty_line_after_outer_attr cargo uitest
```

Clippy uses UI tests. UI tests check that the output of the compiler is exactly as expected.
Of course there's little sense in writing the output yourself or copying it around.
Therefore you should use `tests/ui/update-all-references.sh` (after running
`cargo test`) and check whether the output looks as you expect with `git diff`. Commit all
`*.stderr` files, too.

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. This will
additionally run [rustfix](https://github.com/rust-lang-nursery/rustfix) for
that test. Rustfix will apply the suggestions from the lint to the code of the
test file and compare that to the contents of a `.fixed` file.

Use `tests/ui/update-all-references.sh` to automatically generate the
`.fixed` file after running `cargo test`.

### Running rustfmt

[Rustfmt](https://github.com/rust-lang/rustfmt) is a tool for formatting Rust code according
to style guidelines. The code has to be formatted by `rustfmt` before a PR will be merged.

It can be installed via `rustup`:
```bash
rustup component add rustfmt
```

Use `cargo fmt --all` to format the whole codebase.

### Testing manually

Manually testing against an example file is useful if you have added some
`println!`s and test suite output becomes unreadable. To try Clippy with your
local modifications, run `env CLIPPY_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug input.rs`
from the working copy root.

## How Clippy works

Clippy is a [rustc compiler plugin][compiler_plugin]. The main entry point is at [`src/lib.rs`][main_entry]. In there, the lint registration is delegated to the [`clippy_lints`][lint_crate] crate.
Expand Down
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ script:
- cargo clippy
# if you want the build job to fail when encountering warnings, use
- cargo clippy -- -D warnings
# in order to also check tests and none-default crate features, use
# in order to also check tests and non-default crate features, use
- cargo clippy --all-targets --all-features -- -D warnings
- cargo test
# etc.
```

It might happen that Clippy is not available for a certain nightly release.
If you are on nightly, It might happen that Clippy is not available for a certain nightly release.
In this case you can try to conditionally install Clippy from the git repo.

```yaml
Expand Down
2 changes: 1 addition & 1 deletion ci/base-tests.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ echo "Running clippy base tests"

PATH=$PATH:./node_modules/.bin
if [ "$TRAVIS_OS_NAME" == "linux" ]; then
remark -f *.md > /dev/null
remark -f *.md -f doc/*.md > /dev/null
fi
# build clippy in debug mode and run tests
cargo build --features debugging
Expand Down
Loading

0 comments on commit 920112d

Please sign in to comment.