-# Contributing to rust-clippy
+# Contributing to Clippy
Hello fellow Rustacean! Great to see your interest in compiler internals and lints!
+**First**: if you're unsure or afraid of _anything_, just ask or submit the issue or pull request anyway. You won't be
+yelled at for giving it your best effort. The worst that can happen is that you'll be politely asked to change
+something. We appreciate any sort of contributions, and don't want a wall of rules to get in the way of that.
+
+Clippy welcomes contributions from everyone. There are many ways to contribute to Clippy and the following document
+explains how you can contribute and how to get started. If you have any questions about contributing or need help with
+anything, feel free to ask questions on issues or visit the `#clippy` on [Discord].
+
+All contributors are expected to follow the [Rust Code of Conduct].
+
+* [Getting started](#getting-started)
+ * [Finding something to fix/improve](#finding-something-to-fiximprove)
+* [Writing code](#writing-code)
+* [How Clippy works](#how-clippy-works)
+* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
+* [Issue and PR Triage](#issue-and-pr-triage)
+* [Bors and Homu](#bors-and-homu)
+* [Contributions](#contributions)
+
+[Discord]: https://discord.gg/rust-lang
+[Rust Code of Conduct]: https://www.rust-lang.org/policies/code-of-conduct
+
## Getting started
High level approach:
1. Find something to fix/improve
2. Change code (likely some file in `clippy_lints/src/`)
-3. Run `cargo test` in the root directory and wiggle code until it passes
-4. Open a PR (also can be done between 2. and 3. if you run into problems)
+3. Follow the instructions in the [docs for writing lints](doc/adding_lints.md) such as running the `setup-toolchain.sh` script
+4. Run `cargo test` in the root directory and wiggle code until it passes
+5. Open a PR (also can be done after 2. if you run into problems)
### Finding something to fix/improve
All issues on Clippy are mentored, if you want help with a bug just ask @Manishearth, @llogiq, @mcarton or @oli-obk.
-Some issues are easier than others. The [`good first issue`](https://github.com/rust-lang-nursery/rust-clippy/labels/good%20first%20issue)
-label can be used to find the easy issues. If you want to work on an issue, please leave a comment
-so that we can assign it to you!
+Some issues are easier than others. The [`good first issue`] label can be used to find the easy issues.
+If you want to work on an issue, please leave a comment so that we can assign it to you!
-Issues marked [`T-AST`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-AST) involve simple
-matching of the syntax tree structure, and are generally easier than
-[`T-middle`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-middle) issues, which involve types
+There are also some abandoned PRs, marked with [`S-inactive-closed`].
+Pretty often these PRs are nearly completed and just need some extra steps
+(formatting, addressing review comments, ...) to be merged. If you want to
+complete such a PR, please leave a comment in the PR and open a new one based
+on it.
+
+Issues marked [`T-AST`] involve simple matching of the syntax tree structure,
+and are generally easier than [`T-middle`] issues, which involve types
and resolved paths.
-[`T-AST`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-AST) issues will generally need you to match against a predefined syntax structure. To figure out
-how this syntax structure is encoded in the AST, it is recommended to run `rustc -Z ast-json` on an
-example of the structure and compare with the
-[nodes in the AST docs](http://manishearth.github.io/rust-internals-docs/syntax/ast/). Usually
-the lint will end up to be a nested series of matches and ifs,
-[like so](https://github.com/rust-lang-nursery/rust-clippy/blob/de5ccdfab68a5e37689f3c950ed1532ba9d652a0/src/misc.rs#L34).
+[`T-AST`] issues will generally need you to match against a predefined syntax structure.
+To figure out how this syntax structure is encoded in the AST, it is recommended to run
+`rustc -Z ast-json` on an example of the structure and compare with the [nodes in the AST docs].
+Usually the lint will end up to be a nested series of matches and ifs, [like so][deep-nesting].
+But we can make it nest-less by using [if_chain] macro, [like this][nest-less].
-[`E-medium`](https://github.com/rust-lang-nursery/rust-clippy/labels/E-medium) issues are generally
-pretty easy too, though it's recommended you work on an E-easy issue first. They are mostly classified
-as `E-medium`, since they might be somewhat involved code wise, but not difficult per-se.
+[`E-medium`] issues are generally pretty easy too, though it's recommended you work on an E-easy issue first.
+They are mostly classified as [`E-medium`], since they might be somewhat involved code wise,
+but not difficult per-se.
-[`T-middle`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-middle) issues can
-be more involved and require verifying types. The
-[`ty`](http://manishearth.github.io/rust-internals-docs/rustc/ty) module contains a
+[`T-middle`] issues can be more involved and require verifying types. The [`ty`] module contains a
lot of methods that are useful, though one of the most useful would be `expr_ty` (gives the type of
an AST expression). `match_def_path()` in Clippy's `utils` module can also be useful.
-### Writing code
+[`good first issue`]: https://github.com/rust-lang/rust-clippy/labels/good%20first%20issue
+[`S-inactive-closed`]: https://github.com/rust-lang/rust-clippy/pulls?q=is%3Aclosed+label%3AS-inactive-closed
+[`T-AST`]: https://github.com/rust-lang/rust-clippy/labels/T-AST
+[`T-middle`]: https://github.com/rust-lang/rust-clippy/labels/T-middle
+[`E-medium`]: https://github.com/rust-lang/rust-clippy/labels/E-medium
+[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty
+[nodes in the AST docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/
+[deep-nesting]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/mem_forget.rs#L29-L43
+[if_chain]: https://docs.rs/if_chain/*/if_chain
+[nest-less]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/bit_mask.rs#L124-L150
-Compiling clippy from scratch can take almost a minute or more depending on your machine.
-However, since Rust 1.24.0 incremental compilation is enabled by default and compile times for small changes should be quick.
+## Writing code
-[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][adding_lints] for more details. [Llogiq's blog post on lints]
+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
+[adding_lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md
+[Llogiq's blog post on lints]: https://llogiq.github.io/2015/06/04/workflows.html
+[clippy_rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md
+[rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees
+[rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories
-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.
+## How Clippy works
-Create a new UI test with the pattern you want to match:
+[`clippy_lints/src/lib.rs`][lint_crate_entry] imports all the different lint modules and registers in the [`LintStore`].
+For example, the [`else_if_without_else`][else_if_without_else] lint is registered like this:
```rust
-// ./tests/ui/my_lint.rs
-
-// The custom_attribute needs to be enabled for the author lint to work
-#![feature(plugin, custom_attribute)]
-
-fn main() {
- #[clippy(author)]
- let arr: [i32; 1] = [7]; // Replace line with the code you want to match
+// ./clippy_lints/src/lib.rs
+
+// ...
+pub mod else_if_without_else;
+// ...
+
+pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) {
+ // ...
+ store.register_early_pass(|| box else_if_without_else::ElseIfWithoutElse);
+ // ...
+
+ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
+ // ...
+ LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE),
+ // ...
+ ]);
}
```
-Now you run `TESTNAME=ui/my_lint cargo test --test compile-test` to produce
-the file with the generated code:
+The [`rustc_lint::LintStore`][`LintStore`] provides two methods to register lints:
+[register_early_pass][reg_early_pass] and [register_late_pass][reg_late_pass]. Both take an object
+that implements an [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass] respectively. This is done in
+every single lint. It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `cargo dev
+update_lints`. When you are writing your own lint, you can use that script to save you some time.
```rust
-// ./tests/ui/my_lint.stdout
-
-if_chain! {
- if let Expr_::ExprArray(ref elements) = stmt.node;
- if elements.len() == 1;
- if let Expr_::ExprLit(ref lit) = elements[0].node;
- if let LitKind::Int(7, _) = lit.node;
- then {
- // report your lint here
- }
+// ./clippy_lints/src/else_if_without_else.rs
+
+use rustc_lint::{EarlyLintPass, EarlyContext};
+
+// ...
+
+pub struct ElseIfWithoutElse;
+
+// ...
+
+impl EarlyLintPass for ElseIfWithoutElse {
+ // ... the functions needed, to make the lint work
}
```
-#### Documentation
+The difference between `EarlyLintPass` and `LateLintPass` is that the methods of the `EarlyLintPass` trait only provide
+AST information. The methods of the `LateLintPass` trait are executed after type checking and contain type information
+via the `LateContext` parameter.
-Please document your lint with a doc comment akin to the following:
+That's why the `else_if_without_else` example uses the `register_early_pass` function. Because the
+[actual lint logic][else_if_without_else] does not depend on any type information.
-```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
-/// ```
-```
+[lint_crate_entry]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/lib.rs
+[else_if_without_else]: https://github.com/rust-lang/rust-clippy/blob/4253aa7137cb7378acc96133c787e49a345c2b3c/clippy_lints/src/else_if_without_else.rs
+[`LintStore`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html
+[reg_early_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html#method.register_early_pass
+[reg_late_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html#method.register_late_pass
+[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html
+[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html
+
+## Fixing build failures caused by Rust
+
+Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of
+the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures
+caused by Rust updates, can be a good way to learn about Rust internals.
-Once your lint is merged it will show up in the [lint list](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
+In order to find out why Clippy does not work properly with a new Rust commit, you can use the [rust-toolstate commit
+history][toolstate_commit_history]. You will then have to look for the last commit that contains
+`test-pass -> build-fail` or `test-pass -> test-fail` for the `clippy-driver` component.
+[Here][toolstate_commit] is an example.
-### Running test suite
+The commit message contains a link to the PR. The PRs are usually small enough to discover the breaking API change and
+if they are bigger, they likely include some discussion that may help you to fix Clippy.
-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 can simply run `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.
+To check if Clippy is available for a specific target platform, you can check
+the [rustup component history][rustup_component_history].
-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:
+If you decide to make Clippy work again with a Rust commit that breaks it,
+you probably want to install the latest Rust from master locally and run Clippy
+using that version of Rust.
+
+You can set up the master toolchain by running `./setup-toolchain.sh`. That script will install
+[rustup-toolchain-install-master][rtim] and master toolchain, then run `rustup override set master`.
+
+After fixing the build failure on this repository, we can submit a pull request
+to [`rust-lang/rust`] to fix the toolstate.
+
+To submit a pull request, you should follow these steps:
```bash
-TESTNAME=ui/empty_line_after_outer_attr cargo test --test compile-test
+# Assuming you already cloned the rust-lang/rust repo and you're in the correct directory
+git submodule update --remote src/tools/clippy
+cargo update -p clippy
+git add -u
+git commit -m "Update Clippy"
+./x.py test -i --stage 1 src/tools/clippy # This is optional and should succeed anyway
+# Open a PR in rust-lang/rust
```
-### Testing manually
+[rustup_component_history]: https://rust-lang.github.io/rustup-components-history
+[toolstate_commit_history]: https://github.com/rust-lang-nursery/rust-toolstate/commits/master
+[toolstate_commit]: https://github.com/rust-lang-nursery/rust-toolstate/commit/aad74d8294e198a7cf8ac81a91aebb7f3bbcf727
+[rtim]: https://github.com/kennytm/rustup-toolchain-install-master
+[`rust-lang/rust`]: https://github.com/rust-lang/rust
-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 `cargo run --bin clippy-driver -- -L ./target/debug input.rs` from the
-working copy root. Your test file, here `input.rs`, needs to have clippy
-enabled as a plugin:
+## Issue and PR triage
-```rust
-#![feature(plugin)]
-#![plugin(clippy)]
-```
+Clippy is following the [Rust triage procedure][triage] for issues and pull
+requests.
-## Contributions
+However, we are a smaller project with all contributors being volunteers
+currently. Between writing new lints, fixing issues, reviewing pull requests and
+responding to issues there may not always be enough time to stay on top of it
+all.
-Clippy welcomes contributions from everyone.
+Our highest priority is fixing [crashes][l-crash] and [bugs][l-bug]. We don't
+want Clippy to crash on your code and we want it to be as reliable as the
+suggestions from Rust compiler errors.
-Contributions to Clippy should be made in the form of GitHub pull requests. Each pull request will
-be reviewed by a core contributor (someone with permission to land patches) and either landed in the
-main tree or given feedback for changes that would be required.
+## Bors and Homu
-All code in this repository is under the [Mozilla Public License, 2.0](https://www.mozilla.org/MPL/2.0/)
+We use a bot powered by [Homu][homu] to help automate testing and landing of pull
+requests in Clippy. The bot's username is @bors.
-## Conduct
+You can find the Clippy bors queue [here][homu_queue].
-We follow the [Rust Code of Conduct](http://www.rust-lang.org/conduct.html).
+If you have @bors permissions, you can find an overview of the available
+commands [here][homu_instructions].
+[triage]: https://forge.rust-lang.org/triage-procedure.html
+[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A
+[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A
+[homu]: https://github.com/rust-lang/homu
+[homu_instructions]: https://buildbot2.rust-lang.org/homu/
+[homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy
+
+## Contributions
+
+Contributions to Clippy should be made in the form of GitHub pull requests. Each pull request will
+be reviewed by a core contributor (someone with permission to land patches) and either landed in the
+main tree or given feedback for changes that would be required.
+
+All code in this repository is under the [Apache-2.0] or the [MIT] license.
<!-- adapted from https://github.com/servo/servo/blob/master/CONTRIBUTING.md -->
+
+[Apache-2.0]: https://www.apache.org/licenses/LICENSE-2.0
+[MIT]: https://opensource.org/licenses/MIT
projects / rust.git / blobdiff