1 # Contributing to Clippy
3 Hello fellow Rustacean! Great to see your interest in compiler internals and lints!
5 **First**: if you're unsure or afraid of _anything_, just ask or submit the issue or pull request anyway. You won't be
6 yelled at for giving it your best effort. The worst that can happen is that you'll be politely asked to change
7 something. We appreciate any sort of contributions, and don't want a wall of rules to get in the way of that.
9 Clippy welcomes contributions from everyone. There are many ways to contribute to Clippy and the following document
10 explains how you can contribute and how to get started. If you have any questions about contributing or need help with
11 anything, feel free to ask questions on issues or visit the `#clippy` on [Zulip].
13 All contributors are expected to follow the [Rust Code of Conduct].
15 - [Contributing to Clippy](#contributing-to-clippy)
16 - [Getting started](#getting-started)
17 - [High level approach](#high-level-approach)
18 - [Finding something to fix/improve](#finding-something-to-fiximprove)
19 - [Writing code](#writing-code)
20 - [Getting code-completion for rustc internals to work](#getting-code-completion-for-rustc-internals-to-work)
21 - [How Clippy works](#how-clippy-works)
22 - [Syncing changes between Clippy and `rust-lang/rust`](#syncing-changes-between-clippy-and-rust-langrust)
23 - [Patching git-subtree to work with big repos](#patching-git-subtree-to-work-with-big-repos)
24 - [Performing the sync from `rust-lang/rust` to Clippy](#performing-the-sync-from-rust-langrust-to-clippy)
25 - [Performing the sync from Clippy to `rust-lang/rust`](#performing-the-sync-from-clippy-to-rust-langrust)
26 - [Defining remotes](#defining-remotes)
27 - [Issue and PR triage](#issue-and-pr-triage)
28 - [Bors and Homu](#bors-and-homu)
29 - [Contributions](#contributions)
31 [Zulip]: https://rust-lang.zulipchat.com/#narrow/stream/clippy
32 [Rust Code of Conduct]: https://www.rust-lang.org/policies/code-of-conduct
36 **Note: If this is your first time contributing to Clippy, you should
37 first read the [Basics docs](doc/basics.md).**
39 ### High level approach
41 1. Find something to fix/improve
42 2. Change code (likely some file in `clippy_lints/src/`)
43 3. Follow the instructions in the [Basics docs](doc/basics.md) to get set up
44 4. Run `cargo test` in the root directory and wiggle code until it passes
45 5. Open a PR (also can be done after 2. if you run into problems)
47 ### Finding something to fix/improve
49 All issues on Clippy are mentored, if you want help simply ask @Manishearth, @flip1995, @phansch
50 or @llogiq directly by mentioning them in the issue or over on [Zulip]. This list may be out of date.
51 All currently active mentors can be found [here](https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust-clippy.json#L3)
53 Some issues are easier than others. The [`good-first-issue`] label can be used to find the easy
54 issues. You can use `@rustbot claim` to assign the issue to yourself.
56 There are also some abandoned PRs, marked with [`S-inactive-closed`].
57 Pretty often these PRs are nearly completed and just need some extra steps
58 (formatting, addressing review comments, ...) to be merged. If you want to
59 complete such a PR, please leave a comment in the PR and open a new one based
62 Issues marked [`T-AST`] involve simple matching of the syntax tree structure,
63 and are generally easier than [`T-middle`] issues, which involve types
66 [`T-AST`] issues will generally need you to match against a predefined syntax structure.
67 To figure out how this syntax structure is encoded in the AST, it is recommended to run
68 `rustc -Z ast-json` on an example of the structure and compare with the [nodes in the AST docs].
69 Usually the lint will end up to be a nested series of matches and ifs, [like so][deep-nesting].
70 But we can make it nest-less by using [if_chain] macro, [like this][nest-less].
72 [`E-medium`] issues are generally pretty easy too, though it's recommended you work on an [`good-first-issue`]
73 first. Sometimes they are only somewhat involved code wise, but not difficult per-se.
74 Note that [`E-medium`] issues may require some knowledge of Clippy internals or some
75 debugging to find the actual problem behind the issue.
77 [`T-middle`] issues can be more involved and require verifying types. The [`ty`] module contains a
78 lot of methods that are useful, though one of the most useful would be `expr_ty` (gives the type of
79 an AST expression). `match_def_path()` in Clippy's `utils` module can also be useful.
81 [`good-first-issue`]: https://github.com/rust-lang/rust-clippy/labels/good-first-issue
82 [`S-inactive-closed`]: https://github.com/rust-lang/rust-clippy/pulls?q=is%3Aclosed+label%3AS-inactive-closed
83 [`T-AST`]: https://github.com/rust-lang/rust-clippy/labels/T-AST
84 [`T-middle`]: https://github.com/rust-lang/rust-clippy/labels/T-middle
85 [`E-medium`]: https://github.com/rust-lang/rust-clippy/labels/E-medium
86 [`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty
87 [nodes in the AST docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/
88 [deep-nesting]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/mem_forget.rs#L29-L43
89 [if_chain]: https://docs.rs/if_chain/*/if_chain
90 [nest-less]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/bit_mask.rs#L124-L150
94 Have a look at the [docs for writing lints][adding_lints] for more details.
96 If you want to add a new lint or change existing ones apart from bugfixing, it's
97 also a good idea to give the [stability guarantees][rfc_stability] and
98 [lint categories][rfc_lint_cats] sections of the [Clippy 1.0 RFC][clippy_rfc] a
101 [adding_lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md
102 [clippy_rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md
103 [rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees
104 [rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories
106 ## Getting code-completion for rustc internals to work
108 Unfortunately, [`rust-analyzer`][ra_homepage] does not (yet?) understand how Clippy uses compiler-internals
109 using `extern crate` and it also needs to be able to read the source files of the rustc-compiler which are not
110 available via a `rustup` component at the time of writing.
111 To work around this, you need to have a copy of the [rustc-repo][rustc_repo] available which can be obtained via
112 `git clone https://github.com/rust-lang/rust/`.
113 Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies
114 which rust-analyzer will be able to understand.
115 Run `cargo dev ra_setup --repo-path <repo-path>` where `<repo-path>` is an absolute path to the rustc repo
117 The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to
118 Clippys `Cargo.toml`s and should allow rust-analyzer to understand most of the types that Clippy uses.
119 Just make sure to remove the dependencies again before finally making a pull request!
121 [ra_homepage]: https://rust-analyzer.github.io/
122 [rustc_repo]: https://github.com/rust-lang/rust/
126 [`clippy_lints/src/lib.rs`][lint_crate_entry] imports all the different lint modules and registers in the [`LintStore`].
127 For example, the [`else_if_without_else`][else_if_without_else] lint is registered like this:
130 // ./clippy_lints/src/lib.rs
133 pub mod else_if_without_else;
136 pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) {
138 store.register_early_pass(|| box else_if_without_else::ElseIfWithoutElse);
141 store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
143 LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE),
149 The [`rustc_lint::LintStore`][`LintStore`] provides two methods to register lints:
150 [register_early_pass][reg_early_pass] and [register_late_pass][reg_late_pass]. Both take an object
151 that implements an [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass] respectively. This is done in
152 every single lint. It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `cargo dev
153 update_lints`. When you are writing your own lint, you can use that script to save you some time.
156 // ./clippy_lints/src/else_if_without_else.rs
158 use rustc_lint::{EarlyLintPass, EarlyContext};
162 pub struct ElseIfWithoutElse;
166 impl EarlyLintPass for ElseIfWithoutElse {
167 // ... the functions needed, to make the lint work
171 The difference between `EarlyLintPass` and `LateLintPass` is that the methods of the `EarlyLintPass` trait only provide
172 AST information. The methods of the `LateLintPass` trait are executed after type checking and contain type information
173 via the `LateContext` parameter.
175 That's why the `else_if_without_else` example uses the `register_early_pass` function. Because the
176 [actual lint logic][else_if_without_else] does not depend on any type information.
178 [lint_crate_entry]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/lib.rs
179 [else_if_without_else]: https://github.com/rust-lang/rust-clippy/blob/4253aa7137cb7378acc96133c787e49a345c2b3c/clippy_lints/src/else_if_without_else.rs
180 [`LintStore`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html
181 [reg_early_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html#method.register_early_pass
182 [reg_late_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html#method.register_late_pass
183 [early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html
184 [late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html
186 ## Syncing changes between Clippy and [`rust-lang/rust`]
188 Clippy currently gets built with a pinned nightly version.
190 In the `rust-lang/rust` repository, where rustc resides, there's a copy of Clippy
191 that compiler hackers modify from time to time to adapt to changes in the unstable
194 We need to sync these changes back to this repository periodically, and the changes
195 made to this repository in the meantime also need to be synced to the `rust-lang/rust` repository.
197 To avoid flooding the `rust-lang/rust` PR queue, this two-way sync process is done
198 in a bi-weekly basis if there's no urgent changes. This is done starting on the day of
199 the Rust stable release and then every other week. That way we guarantee that we keep
200 this repo up to date with the latest compiler API, and every feature in Clippy is available
201 for 2 weeks in nightly, before it can get to beta. For reference, the first sync
202 following this cadence was performed the 2020-08-27.
204 This process is described in detail in the following sections. For general information
205 about `subtree`s in the Rust repository see [Rust's `CONTRIBUTING.md`][subtree].
207 ### Patching git-subtree to work with big repos
209 Currently there's a bug in `git-subtree` that prevents it from working properly
210 with the [`rust-lang/rust`] repo. There's an open PR to fix that, but it's stale.
211 Before continuing with the following steps, we need to manually apply that fix to
212 our local copy of `git-subtree`.
214 You can get the patched version of `git-subtree` from [here][gitgitgadget-pr].
215 Put this file under `/usr/lib/git-core` (taking a backup of the previous file)
216 and make sure it has the proper permissions:
219 sudo cp --backup /path/to/patched/git-subtree.sh /usr/lib/git-core/git-subtree
220 sudo chmod --reference=/usr/lib/git-core/git-subtree~ /usr/lib/git-core/git-subtree
221 sudo chown --reference=/usr/lib/git-core/git-subtree~ /usr/lib/git-core/git-subtree
224 _Note:_ The first time running `git subtree push` a cache has to be built. This
225 involves going through the complete Clippy history once. For this you have to
226 increase the stack limit though, which you can do with `ulimit -s 60000`.
227 Make sure to run the `ulimit` command from the same session you call git subtree.
229 _Note:_ If you are a Debian user, `dash` is the shell used by default for scripts instead of `sh`.
230 This shell has a hardcoded recursion limit set to 1000. In order to make this process work,
231 you need to force the script to run `bash` instead. You can do this by editing the first
232 line of the `git-subtree` script and changing `sh` to `bash`.
234 ### Performing the sync from [`rust-lang/rust`] to Clippy
236 Here is a TL;DR version of the sync process (all of the following commands have
237 to be run inside the `rust` directory):
239 1. Clone the [`rust-lang/rust`] repository or make sure it is up to date.
240 2. Checkout the commit from the latest available nightly. You can get it using `rustup check`.
241 3. Sync the changes to the rust-copy of Clippy to your Clippy fork:
243 # Make sure to change `your-github-name` to your github name in the following command
244 git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust
247 _Note:_ This will directly push to the remote repository. You can also push
248 to your local copy by replacing the remote address with `/path/to/rust-clippy`
251 _Note:_ Most of the time you have to create a merge commit in the
252 `rust-clippy` repo (this has to be done in the Clippy repo, not in the
253 rust-copy of Clippy):
255 git fetch origin && git fetch upstream
256 git checkout sync-from-rust
257 git merge upstream/master
259 4. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to
260 accelerate the process ping the `@rust-lang/clippy` team in your PR and/or
261 ~~annoy~~ ask them in the [Zulip] stream.)
263 ### Performing the sync from Clippy to [`rust-lang/rust`]
265 All of the following commands have to be run inside the `rust` directory.
267 1. Make sure Clippy itself is up-to-date by following the steps outlined in the previous
268 section if necessary.
270 2. Sync the `rust-lang/rust-clippy` master to the rust-copy of Clippy:
272 git checkout -b sync-from-clippy
273 git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy master
275 3. Open a PR to [`rust-lang/rust`]
279 You may want to define remotes, so you don't have to type out the remote
280 addresses on every sync. You can do this with the following commands (these
281 commands still have to be run inside the `rust` directory):
284 # Set clippy-upstream remote for pulls
285 $ git remote add clippy-upstream https://github.com/rust-lang/rust-clippy
286 # Make sure to not push to the upstream repo
287 $ git remote set-url --push clippy-upstream DISABLED
288 # Set clippy-origin remote to your fork for pushes
289 $ git remote add clippy-origin git@github.com:your-github-name/rust-clippy
291 $ git remote add clippy-local /path/to/rust-clippy
294 You can then sync with the remote names from above, e.g.:
297 $ git subtree push -P src/tools/clippy clippy-local sync-from-rust
300 [gitgitgadget-pr]: https://github.com/gitgitgadget/git/pull/493
301 [subtree]: https://rustc-dev-guide.rust-lang.org/contributing.html#external-dependencies-subtree
302 [`rust-lang/rust`]: https://github.com/rust-lang/rust
304 ## Issue and PR triage
306 Clippy is following the [Rust triage procedure][triage] for issues and pull
309 However, we are a smaller project with all contributors being volunteers
310 currently. Between writing new lints, fixing issues, reviewing pull requests and
311 responding to issues there may not always be enough time to stay on top of it
314 Our highest priority is fixing [crashes][l-crash] and [bugs][l-bug], for example
315 an ICE in a popular crate that many other crates depend on. We don't
316 want Clippy to crash on your code and we want it to be as reliable as the
317 suggestions from Rust compiler errors.
319 We have prioritization labels and a sync-blocker label, which are described below.
320 - [P-low][p-low]: Requires attention (fix/response/evaluation) by a team member but isn't urgent.
321 - [P-medium][p-medium]: Should be addressed by a team member until the next sync.
322 - [P-high][p-high]: Should be immediately addressed and will require an out-of-cycle sync or a backport.
323 - [L-sync-blocker][l-sync-blocker]: An issue that "blocks" a sync.
324 Or rather: before the sync this should be addressed,
325 e.g. by removing a lint again, so it doesn't hit beta/stable.
329 We use a bot powered by [Homu][homu] to help automate testing and landing of pull
330 requests in Clippy. The bot's username is @bors.
332 You can find the Clippy bors queue [here][homu_queue].
334 If you have @bors permissions, you can find an overview of the available
335 commands [here][homu_instructions].
337 [triage]: https://forge.rust-lang.org/release/triage-procedure.html
338 [l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash
339 [l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug
340 [p-low]: https://github.com/rust-lang/rust-clippy/labels/P-low
341 [p-medium]: https://github.com/rust-lang/rust-clippy/labels/P-medium
342 [p-high]: https://github.com/rust-lang/rust-clippy/labels/P-high
343 [l-sync-blocker]: https://github.com/rust-lang/rust-clippy/labels/L-sync-blocker
344 [homu]: https://github.com/rust-lang/homu
345 [homu_instructions]: https://bors.rust-lang.org/
346 [homu_queue]: https://bors.rust-lang.org/queue/clippy
350 Contributions to Clippy should be made in the form of GitHub pull requests. Each pull request will
351 be reviewed by a core contributor (someone with permission to land patches) and either landed in the
352 main tree or given feedback for changes that would be required.
354 All code in this repository is under the [Apache-2.0] or the [MIT] license.
356 <!-- adapted from https://github.com/servo/servo/blob/master/CONTRIBUTING.md -->
358 [Apache-2.0]: https://www.apache.org/licenses/LICENSE-2.0
359 [MIT]: https://opensource.org/licenses/MIT