diff --git a/.gitignore b/.gitignore index 3e50c45a9b63e..503ae3c509039 100644 --- a/.gitignore +++ b/.gitignore @@ -39,3 +39,6 @@ helper.txt *.iml .vscode .idea + +# mdbook generated output +/book/book diff --git a/book/README.md b/book/README.md new file mode 100644 index 0000000000000..b652194d0d13b --- /dev/null +++ b/book/README.md @@ -0,0 +1,4 @@ +# Clippy Book + +This is the source for the Clippy Book. See the +[book](src/infrastructure/book.md) for more information. diff --git a/book/book.toml b/book/book.toml new file mode 100644 index 0000000000000..93b6641f7e1e7 --- /dev/null +++ b/book/book.toml @@ -0,0 +1,28 @@ +[book] +authors = ["The Rust Clippy Developers"] +language = "en" +multilingual = false +src = "src" +title = "Clippy Documentation" + +[rust] +edition = "2018" + +[output.html] +edit-url-template = "https://github.com/rust-lang/rust-clippy/edit/master/book/{path}" +git-repository-url = "https://github.com/rust-lang/rust-clippy/tree/master/book" +mathjax-support = true +site-url = "/rust-clippy/" + +[output.html.playground] +editable = true +line-numbers = true + +[output.html.search] +boost-hierarchy = 2 +boost-paragraph = 1 +boost-title = 2 +expand = true +heading-split-level = 2 +limit-results = 20 +use-boolean-and = true diff --git a/book/src/README.md b/book/src/README.md new file mode 100644 index 0000000000000..de1f70d7e9640 --- /dev/null +++ b/book/src/README.md @@ -0,0 +1,34 @@ +# Clippy + +[![Clippy Test](https://github.com/rust-lang/rust-clippy/workflows/Clippy%20Test/badge.svg?branch=auto&event=push)](https://github.com/rust-lang/rust-clippy/actions?query=workflow%3A%22Clippy+Test%22+event%3Apush+branch%3Aauto) +[![License: MIT OR Apache-2.0](https://img.shields.io/crates/l/clippy.svg)](#license) + +A collection of lints to catch common mistakes and improve your +[Rust](https://github.com/rust-lang/rust) code. + +[There are over 500 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) + +Lints are divided into categories, each with a default [lint +level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how +much Clippy is supposed to ~~annoy~~ help you by changing the lint level by +category. + +| Category | Description | Default level | +| --------------------- | ----------------------------------------------------------------------------------- | ------------- | +| `clippy::all` | all lints that are on by default (correctness, suspicious, style, complexity, perf) | **warn/deny** | +| `clippy::correctness` | code that is outright wrong or useless | **deny** | +| `clippy::suspicious` | code that is most likely wrong or useless | **warn** | +| `clippy::complexity` | code that does something simple but in a complex way | **warn** | +| `clippy::perf` | code that can be written to run faster | **warn** | +| `clippy::style` | code that should be written in a more idiomatic way | **warn** | +| `clippy::pedantic` | lints which are rather strict or might have false positives | allow | +| `clippy::nursery` | new lints that are still under development | allow | +| `clippy::cargo` | lints for the cargo manifest | allow | | allow | + +More to come, please [file an +issue](https://github.com/rust-lang/rust-clippy/issues) if you have ideas! + +The [lint list](https://rust-lang.github.io/rust-clippy/master/index.html) also +contains "restriction lints", which are for things which are usually not +considered "bad", but may be useful to turn on in specific cases. These should +be used very selectively, if at all. diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md new file mode 100644 index 0000000000000..470760b6d1688 --- /dev/null +++ b/book/src/SUMMARY.md @@ -0,0 +1,26 @@ +# Summary + +[Introduction](README.md) + +- [Installation and Usage](installation_and_usage.md) +- [Configuration](configuration.md) +- [Clippy's Lints](lints/README.md) + - [Correctness]() + - [Suspicious]() + - [Style]() + - [Complexity]() + - [Perf]() + - [Pendantic]() + - [Nursery]() + - [Cargo]() +- [Development](development/README.md) + - [Basics](development/basics.md) + - [Adding Lints](development/adding_lints.md) + - [Common Tools](development/common_tools_writing_lints.md) +- [Infrastructure](infrastructure/README.md) + - [Backporting Changes](infrastructure/backport.md) + - [Updating the Changelog](infrastructure/changelog_update.md) + - [Release a New Version](infrastructure/release.md) + - [The Clippy Book](infrastructure/book.md) +- [Roadmap](roadmap/README.md) + - [2021](roadmap/2021.md) diff --git a/book/src/configuration.md b/book/src/configuration.md new file mode 100644 index 0000000000000..6e295ac3181dd --- /dev/null +++ b/book/src/configuration.md @@ -0,0 +1,92 @@ +# Configuring Clippy + +> **Note:** The configuration file is unstable and may be deprecated in the future. + +Some lints can be configured in a TOML file named `clippy.toml` or `.clippy.toml`. It contains a +basic `variable = value` mapping eg. + +```toml +avoid-breaking-exported-api = false +blacklisted-names = ["toto", "tata", "titi"] +cognitive-complexity-threshold = 30 +``` + +See the [list of lints](https://rust-lang.github.io/rust-clippy/master/index.html) for more information about which +lints can be configured and the meaning of the variables. + +To deactivate the "for further information visit *lint-link*" message you can define the `CLIPPY_DISABLE_DOCS_LINKS` +environment variable. + +### Allowing/denying lints + +You can add options to your code to `allow`/`warn`/`deny` Clippy lints: + +* the whole set of `Warn` lints using the `clippy` lint group (`#![deny(clippy::all)]`) + +* all lints using both the `clippy` and `clippy::pedantic` lint groups (`#![deny(clippy::all)]`, + `#![deny(clippy::pedantic)]`). Note that `clippy::pedantic` contains some very aggressive lints prone to false + positives. + +* only some lints (`#![deny(clippy::single_match, clippy::box_vec)]`, etc.) + +* `allow`/`warn`/`deny` can be limited to a single function or module using `#[allow(...)]`, etc. + +Note: `allow` means to suppress the lint for your code. With `warn` the lint will only emit a warning, while with `deny` +the lint will emit an error, when triggering for your code. An error causes clippy to exit with an error code, so is +useful in scripts like CI/CD. + +If you do not want to include your lint levels in your code, you can globally enable/disable lints by passing extra +flags to Clippy during the run: + +To allow `lint_name`, run + +```terminal +cargo clippy -- -A clippy::lint_name +``` + +And to warn on `lint_name`, run + +```terminal +cargo clippy -- -W clippy::lint_name +``` + +This also works with lint groups. For example you can run Clippy with warnings for all lints enabled: + +```terminal +cargo clippy -- -W clippy::pedantic +``` + +If you care only about a single lint, you can allow all others and then explicitly warn on the lint(s) you are +interested in: + +```terminal +cargo clippy -- -A clippy::all -W clippy::useless_format -W clippy::... +``` + +### Specifying the minimum supported Rust version + +Projects that intend to support old versions of Rust can disable lints pertaining to newer features by specifying the +minimum supported Rust version (MSRV) in the clippy configuration file. + +```toml +msrv = "1.30.0" +``` + +The MSRV can also be specified as an inner attribute, like below. + +```rust +#![feature(custom_inner_attributes)] +#![clippy::msrv = "1.30.0"] + +fn main() { + ... +} +``` + +You can also omit the patch version when specifying the MSRV, so `msrv = 1.30` +is equivalent to `msrv = 1.30.0`. + +Note: `custom_inner_attributes` is an unstable feature so it has to be enabled explicitly. + +Lints that recognize this configuration option can be +found [here](https://rust-lang.github.io/rust-clippy/master/index.html#msrv) diff --git a/book/src/development/README.md b/book/src/development/README.md new file mode 100644 index 0000000000000..09d6aad2c538e --- /dev/null +++ b/book/src/development/README.md @@ -0,0 +1 @@ +# Clippy Development diff --git a/book/src/development/adding_lints.md b/book/src/development/adding_lints.md new file mode 100644 index 0000000000000..5a06afedbf4c2 --- /dev/null +++ b/book/src/development/adding_lints.md @@ -0,0 +1,670 @@ +# Adding a new lint + +You are probably here because you want to add a new lint to Clippy. If this is +the first time you're contributing to Clippy, this document guides you through +creating an example lint from scratch. + +To get started, we will create a lint that detects functions called `foo`, +because that's clearly a non-descriptive name. + +- [Adding a new lint](#adding-a-new-lint) + - [Setup](#setup) + - [Getting Started](#getting-started) + - [Testing](#testing) + - [Rustfix tests](#rustfix-tests) + - [Edition 2018 tests](#edition-2018-tests) + - [Testing manually](#testing-manually) + - [Lint declaration](#lint-declaration) + - [Lint passes](#lint-passes) + - [Emitting a lint](#emitting-a-lint) + - [Adding the lint logic](#adding-the-lint-logic) + - [Specifying the lint's minimum supported Rust version (MSRV)](#specifying-the-lints-minimum-supported-rust-version-msrv) + - [Author lint](#author-lint) + - [Documentation](#documentation) + - [Running rustfmt](#running-rustfmt) + - [Debugging](#debugging) + - [PR Checklist](#pr-checklist) + - [Adding configuration to a lint](#adding-configuration-to-a-lint) + - [Cheatsheet](#cheatsheet) + +## Setup + +See the [Basics](basics.md#get-the-code) documentation. + +## Getting Started + +There is a bit of boilerplate code that needs to be set up when creating a new +lint. Fortunately, you can use the clippy dev tools to handle this for you. We +are naming our new lint `foo_functions` (lints are generally written in snake +case), and we don't need type information so it will have an early pass type +(more on this later on). If you're not sure if the name you chose fits the lint, +take a look at our [lint naming guidelines][lint_naming]. To get started on this +lint you can run `cargo dev new_lint --name=foo_functions --pass=early +--category=pedantic` (category will default to nursery if not provided). This +command will create two files: `tests/ui/foo_functions.rs` and +`clippy_lints/src/foo_functions.rs`, as well as run `cargo dev update_lints` to +register the new lint. For cargo lints, two project hierarchies (fail/pass) will +be created by default under `tests/ui-cargo`. + +Next, we'll open up these files and add our lint! + +## Testing + +Let's write some tests first that we can execute while we iterate on our lint. + +Clippy uses UI tests for testing. UI tests check that the output of Clippy is +exactly as expected. 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 further down. + +We start by opening the test file created at `tests/ui/foo_functions.rs`. + +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) {} +} + +// 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(); +} +``` + +Now we can run the test with `TESTNAME=foo_functions cargo uitest`, +currently this test is meaningless though. + +While we are working on implementing our lint, we can keep running the UI +test. That allows us to check if the output is turning into what we want. + +Once we are satisfied with the output, we need to run +`cargo dev bless` to update the `.stderr` file for our lint. +Please note that, we should run `TESTNAME=foo_functions cargo uitest` +every time before running `cargo dev bless`. +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 that if the generated files are +empty, they should be removed. + +Note that you can run multiple test files by specifying a comma separated list: +`TESTNAME=foo_functions,test2,test3`. + +### Cargo lints + +For cargo lints, the process of testing differs in that we are interested in +the `Cargo.toml` manifest file. We also need a minimal crate associated +with that manifest. + +If our new lint is named e.g. `foo_categories`, 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, and prepending the `TESTNAME` +variable to `cargo uitest` works too. + +## 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. This will +additionally run [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 `cargo dev bless` to automatically generate the +`.fixed` file after running the tests. + +[rustfix]: https://github.com/rust-lang/rustfix + +## Edition 2018 tests + +Some features require the 2018 edition to work (e.g. `async_await`), but +compile-test tests run on the 2015 edition by default. To change this behavior +add `// edition:2018` at the top of the test file (note that it's space-sensitive). + +## Testing manually + +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 + +``` +env __CLIPPY_INTERNAL_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug input.rs +``` + +from the working copy root. With tests in place, let's have a look at +implementing our lint now. + +## Lint declaration + +Let's start by opening the new file created in the `clippy_lints` crate +at `clippy_lints/src/foo_functions.rs`. That's the crate where all the +lint code is. This file has already imported some initial things we will need: + +```rust +use rustc_lint::{EarlyLintPass, EarlyContext}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_ast::ast::*; +``` + +The next step is to update the lint declaration. Lints are declared using the +[`declare_clippy_lint!`][declare_clippy_lint] macro, and we just need to update +the auto-generated lint declaration to have a real description, something like this: + +```rust +declare_clippy_lint! { + /// **What it does:** + /// + /// **Why is this bad?** + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// // example code + /// ``` + pub FOO_FUNCTIONS, + pedantic, + "function named `foo`, which is not a descriptive name" +} +``` + +* The section of lines prefixed with `///` constitutes the lint documentation + section. This is the default documentation style and will be displayed + [like this][example_lint_page]. To render and open this documentation locally + in a browser, run `cargo dev serve`. +* `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the + [lint naming guidelines][lint_naming] here when naming your lint. + In short, the name should state the thing that is being checked for and + read well when used with `allow`/`warn`/`deny`. +* `pedantic` sets the lint level to `Allow`. + The exact mapping can be found [here][category_level_mapping] +* The last part should be a text that explains what exactly is wrong with the + code + +The rest of this file contains an empty implementation for our lint pass, +which in this case is `EarlyLintPass` and should look like this: + +```rust +// clippy_lints/src/foo_functions.rs + +// .. imports and lint declaration .. + +declare_lint_pass!(FooFunctions => [FOO_FUNCTIONS]); + +impl EarlyLintPass for FooFunctions {} +``` + +Normally after declaring the lint, we have to run `cargo dev update_lints`, +which updates some files, so Clippy knows about the new lint. Since we used +`cargo dev new_lint ...` to generate the lint declaration, this was done +automatically. While `update_lints` automates most of the 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`: + +```rust +store.register_early_pass(|| box foo_functions::FooFunctions); +``` + +As one may expect, there is a corresponding `register_late_pass` method +available as well. Without a call to one of `register_early_pass` or +`register_late_pass`, the lint pass in question will not be run. + +One reason that `cargo dev` does not automate this step is that multiple lints +can use the same lint pass, so registering the lint pass may already be done +when adding a new lint. Another reason that this step is not automated is that +the order that the passes are registered determines the order the passes +actually run, which in turn affects the order that any emitted lints are output +in. + +[declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L60 +[example_lint_page]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure +[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints +[category_level_mapping]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L110 + +## Lint passes + +Writing a lint that only checks for the name of a function means that we only +have to deal with the AST and don't have to deal with the type system at all. +This is good, because it makes writing this particular lint less complicated. + +We have to make this decision with every new Clippy lint. It boils down to using +either [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass]. + +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. + +Since we don't need type information for checking the function name, we used +`--pass=early` when running the new lint automation and all the imports were +added accordingly. + +[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 + +## Emitting a lint + +With UI tests and the lint declaration in place, we can start working on the +implementation of the lint logic. + +Let's start by implementing the `EarlyLintPass` for our `FooFunctions`: + +```rust +impl EarlyLintPass for FooFunctions { + fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { + // TODO: Emit lint here + } +} +``` + +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. + +Depending on how complex we want our lint message to be, we can choose from a +variety of lint emission functions. They can all be found in +[`clippy_utils/src/diagnostics.rs`][diagnostics]. + +`span_lint_and_help` seems most appropriate in this case. It allows us to +provide an extra help message and we can't really suggest a better name +automatically. This is how it looks: + +```rust +impl EarlyLintPass for FooFunctions { + fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { + span_lint_and_help( + cx, + FOO_FUNCTIONS, + span, + "function named `foo`", + None, + "consider using a more meaningful name" + ); + } +} +``` + +Running our UI test should now produce output that contains the lint message. + +According to [the rustc-dev-guide], the text should be matter of fact and avoid +capitalization and periods, unless multiple sentences are needed. +When code or an identifier must appear in a message or label, it should be +surrounded with single grave accents \`. + +[check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html#method.check_fn +[diagnostics]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/diagnostics.rs +[the rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/diagnostics.html + +## Adding the lint logic + +Writing the logic for your lint will most likely be different from our example, +so this section is kept rather short. + +Using the [`check_fn`][check_fn] method gives us access to [`FnKind`][fn_kind] +that has the [`FnKind::Fn`] variant. It provides access to the name of the +function/method via an [`Ident`][ident]. + +With that we can expand our `check_fn` method to: + +```rust +impl EarlyLintPass for FooFunctions { + fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { + if is_foo_fn(fn_kind) { + span_lint_and_help( + cx, + FOO_FUNCTIONS, + span, + "function named `foo`", + None, + "consider using a more meaningful name" + ); + } + } +} +``` + +We separate the lint conditional from the lint emissions because it makes the +code a bit easier to read. In some cases this separation would also allow to +write some unit tests (as opposed to only UI tests) for the separate function. + +In our example, `is_foo_fn` looks like: + +```rust +// use statements, impl EarlyLintPass, check_fn, .. + +fn is_foo_fn(fn_kind: FnKind<'_>) -> bool { + match fn_kind { + FnKind::Fn(_, ident, ..) => { + // check if `fn` name is `foo` + ident.name.as_str() == "foo" + } + // ignore closures + FnKind::Closure(..) => false + } +} +``` + +Now we should also run the full test suite with `cargo test`. At this point +running `cargo test` should produce the expected output. Remember to run +`cargo dev bless` to update the `.stderr` file. + +`cargo test` (as opposed to `cargo uitest`) will also ensure that our lint +implementation is not violating any Clippy lints itself. + +That should be it for the lint implementation. Running `cargo test` should now +pass. + +[fn_kind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html +[`FnKind::Fn`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html#variant.Fn +[ident]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/struct.Ident.html + +## Specifying the lint's minimum supported Rust version (MSRV) + +Sometimes a lint makes suggestions that require a certain version of Rust. For example, the `manual_strip` lint suggests +using `str::strip_prefix` and `str::strip_suffix` which is only available after Rust 1.45. In such cases, you need to +ensure that the MSRV configured for the project is >= the MSRV of the required Rust feature. If multiple features are +required, just use the one with a lower MSRV. + +First, add an MSRV alias for the required feature in [`clippy_utils::msrvs`](/clippy_utils/src/msrvs.rs). This can be +accessed later as `msrvs::STR_STRIP_PREFIX`, for example. + +```rust +msrv_aliases! { + .. + 1,45,0 { STR_STRIP_PREFIX } +} +``` + +In order to access the project-configured MSRV, you need to have an `msrv` field in the LintPass struct, and a +constructor to initialize the field. The `msrv` value is passed to the constructor in `clippy_lints/lib.rs`. + +```rust +pub struct ManualStrip { + msrv: Option, +} + +impl ManualStrip { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} +``` + +The project's MSRV can then be matched against the feature MSRV in the LintPass +using the `meets_msrv` utility function. + +``` rust +if !meets_msrv(self.msrv.as_ref(), &msrvs::STR_STRIP_PREFIX) { + return; +} +``` + +The project's MSRV can also be specified as an inner attribute, which overrides +the value from `clippy.toml`. This can be accounted for using the +`extract_msrv_attr!(LintContext)` macro and passing +`LateContext`/`EarlyContext`. + +```rust +impl<'tcx> LateLintPass<'tcx> for ManualStrip { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + ... + } + extract_msrv_attr!(LateContext); +} +``` + +Once the `msrv` is added to the lint, a relevant test case should be added to +`tests/ui/min_rust_version_attr.rs` which verifies that the lint isn't emitted +if the project's MSRV is lower. + +As a last step, the lint should be added to the lint documentation. This is done +in `clippy_lints/src/utils/conf.rs`: + +```rust +define_Conf! { + /// Lint: LIST, OF, LINTS, . The minimum rust version that the project supports + (msrv: Option = None), + ... +} +``` + +## Author lint + +If you have trouble implementing your 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. + +The quickest way to use it, is the +[Rust playground: play.rust-lang.org][author_example]. +Put the code you want to lint into the editor and add the `#[clippy::author]` +attribute above the item. Then run Clippy via `Tools -> Clippy` and you should +see the generated code in the output below. + +[Here][author_example] is an example on the playground. + +If the command was executed successfully, you can copy the code over to where +you are implementing your lint. + +[author_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=9a12cb60e5c6ad4e3003ac6d5e63cf55 + +## Documentation + +The final thing before submitting our PR is to add some documentation to our +lint declaration. + +Please document your lint with a doc comment akin to the following: + +```rust +declare_clippy_lint! { + /// **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,ignore + /// // 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 + /// ``` + pub FOO_FUNCTIONS, + pedantic, + "function named `foo`, which is not a descriptive name" +} +``` + +Once your lint is merged, this documentation will show up in the [lint +list][lint_list]. + +[lint_list]: https://rust-lang.github.io/rust-clippy/master/index.html + +## Running rustfmt + +[Rustfmt] is a tool for formatting Rust code according to style guidelines. +Your code has to be formatted by `rustfmt` before a PR can be merged. +Clippy uses nightly `rustfmt` in the CI. + +It can be installed via `rustup`: + +```bash +rustup component add rustfmt --toolchain=nightly +``` + +Use `cargo dev fmt` to format the whole codebase. Make sure that `rustfmt` is +installed for the nightly toolchain. + +[Rustfmt]: https://github.com/rust-lang/rustfmt + +## Debugging + +If you want to debug parts of your lint implementation, you can use the [`dbg!`] +macro anywhere in your code. Running the tests should then include the debug +output in the `stdout` part. + +[`dbg!`]: https://doc.rust-lang.org/std/macro.dbg.html + +## PR Checklist + +Before submitting your PR make sure you followed all of the basic requirements: + + + +- \[ ] Followed [lint naming conventions][lint_naming] +- \[ ] Added passing UI tests (including committed `.stderr` file) +- \[ ] `cargo test` passes locally +- \[ ] Executed `cargo dev update_lints` +- \[ ] Added lint documentation +- \[ ] Run `cargo dev fmt` + +## Adding configuration to a lint + +Clippy supports the configuration of lints values using a `clippy.toml` file in the workspace +directory. Adding a configuration to a lint can be useful for thresholds or to constrain some +behavior that can be seen as a false positive for some users. Adding a configuration is done +in the following steps: + +1. Adding a new configuration entry to [clippy_utils::conf](/clippy_utils/src/conf.rs) + like this: + ```rust + /// Lint: LINT_NAME. + (configuration_ident: Type = DefaultValue), + ``` + The configuration value and identifier should usually be the same. The doc comment will be + automatically added to the lint documentation. +2. Adding the configuration value to the lint impl struct: + 1. This first requires the definition of a lint impl struct. Lint impl structs are usually + generated with the `declare_lint_pass!` macro. This struct needs to be defined manually + to add some kind of metadata to it: + ```rust + // Generated struct definition + declare_lint_pass!(StructName => [ + LINT_NAME + ]); + + // New manual definition struct + #[derive(Copy, Clone)] + pub struct StructName {} + + impl_lint_pass!(StructName => [ + LINT_NAME + ]); + ``` + + 2. Next add the configuration value and a corresponding creation method like this: + ```rust + #[derive(Copy, Clone)] + pub struct StructName { + configuration_ident: Type, + } + + // ... + + impl StructName { + pub fn new(configuration_ident: Type) -> Self { + Self { + configuration_ident, + } + } + } + ``` +3. Passing the configuration value to the lint impl struct: + + First find the struct construction in the [clippy_lints lib file](/clippy_lints/src/lib.rs). + The configuration value is now cloned or copied into a local value that is then passed to the + impl struct like this: + ```rust + // Default generated registration: + store.register_*_pass(|| box module::StructName); + + // New registration with configuration value + let configuration_ident = conf.configuration_ident.clone(); + store.register_*_pass(move || box module::StructName::new(configuration_ident)); + ``` + + Congratulations the work is almost done. The configuration value can now be accessed + in the linting code via `self.configuration_ident`. + +4. Adding tests: + 1. The default configured value can be tested like any normal lint in [`tests/ui`](/tests/ui). + 2. The configuration itself will be tested separately in [`tests/ui-toml`](/tests/ui-toml). + Simply add a new subfolder with a fitting name. This folder contains a `clippy.toml` file + with the configuration value and a rust file that should be linted by Clippy. The test can + otherwise be written as usual. + +## Cheatsheet + +Here are some pointers to things you are likely going to need for every lint: + +* [Clippy utils][utils] - Various helper functions. Maybe the function you need + is already in here (`implements_trait`, `match_def_path`, `snippet`, etc) +* [Clippy diagnostics][diagnostics] +* [The `if_chain` macro][if_chain] +* [`from_expansion`][from_expansion] and [`in_external_macro`][in_external_macro] +* [`Span`][span] +* [`Applicability`][applicability] +* [Common tools for writing lints](common_tools_writing_lints.md) helps with common operations +* [The rustc-dev-guide][rustc-dev-guide] explains a lot of internal compiler concepts +* [The nightly rustc docs][nightly_docs] which has been linked to throughout + this guide + +For `EarlyLintPass` lints: + +* [`EarlyLintPass`][early_lint_pass] +* [`rustc_ast::ast`][ast] + +For `LateLintPass` lints: + +* [`LateLintPass`][late_lint_pass] +* [`Ty::TyKind`][ty] + +While most of Clippy's lint utils are documented, most of rustc's internals lack +documentation currently. This is unfortunate, but in most cases you can probably +get away with copying things from existing similar lints. If you are stuck, +don't hesitate to ask on [Zulip] or in the issue/PR. + +[utils]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/lib.rs +[if_chain]: https://docs.rs/if_chain/*/if_chain/ +[from_expansion]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html#method.from_expansion +[in_external_macro]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/lint/fn.in_external_macro.html +[span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html +[applicability]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html +[rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/ +[nightly_docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ +[ast]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/index.html +[ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/sty/index.html +[Zulip]: https://rust-lang.zulipchat.com/#narrow/stream/clippy diff --git a/book/src/development/basics.md b/book/src/development/basics.md new file mode 100644 index 0000000000000..aaf31158f58a8 --- /dev/null +++ b/book/src/development/basics.md @@ -0,0 +1,172 @@ +# Basics for hacking on Clippy + +This document explains the basics for hacking on Clippy. Besides others, this +includes how to build and test Clippy. For a more in depth description on +the codebase take a look at [Adding Lints] or [Common Tools]. + +[Adding Lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md +[Common Tools]: https://github.com/rust-lang/rust-clippy/blob/master/doc/common_tools_writing_lints.md + +- [Basics for hacking on Clippy](#basics-for-hacking-on-clippy) + - [Get the Code](#get-the-code) + - [Building and Testing](#building-and-testing) + - [`cargo dev`](#cargo-dev) + - [lintcheck](#lintcheck) + - [PR](#pr) + - [Common Abbreviations](#common-abbreviations) + - [Install from source](#install-from-source) + +## Get the Code + +First, make sure you have checked out the latest version of Clippy. If this is +your first time working on Clippy, create a fork of the repository and clone it +afterwards with the following command: + +```bash +git clone git@github.com:/rust-clippy +``` + +If you've already cloned Clippy in the past, update it to the latest version: + +```bash +# If the upstream remote has not been added yet +git remote add upstream https://github.com/rust-lang/rust-clippy +# upstream has to be the remote of the rust-lang/rust-clippy repo +git fetch upstream +# make sure that you are on the master branch +git checkout master +# rebase your master branch on the upstream master +git rebase upstream/master +# push to the master branch of your fork +git push +``` + +## Building and Testing + +You can build and test Clippy like every other Rust project: + +```bash +cargo build # builds Clippy +cargo test # tests Clippy +``` + +Since Clippy's test suite is pretty big, there are some commands that only run a +subset of Clippy's tests: + +```bash +# only run UI tests +cargo uitest +# only run UI tests starting with `test_` +TESTNAME="test_" cargo uitest +# only run dogfood tests +cargo test --test dogfood +``` + +If the output of a [UI test] differs from the expected output, you can update the +reference file with: + +```bash +cargo dev bless +``` + +For example, this is necessary, if you fix a typo in an error message of a lint +or if you modify a test file to add a test case. + +_Note:_ This command may update more files than you intended. In that case only +commit the files you wanted to update. + +[UI test]: https://rustc-dev-guide.rust-lang.org/tests/adding.html#guide-to-the-ui-tests + +## `cargo dev` + +Clippy has some dev tools to make working on Clippy more convenient. These tools +can be accessed through the `cargo dev` command. Available tools are listed +below. To get more information about these commands, just call them with +`--help`. + +```bash +# formats the whole Clippy codebase and all tests +cargo dev fmt +# register or update lint names/groups/... +cargo dev update_lints +# create a new lint and register it +cargo dev new_lint +# (experimental) Setup Clippy to work with IntelliJ-Rust +cargo dev ide_setup +``` + +## lintcheck +`cargo lintcheck` will build and run clippy on a fixed set of crates and generate a log of the results. +You can `git diff` the updated log against its previous version and +see what impact your lint made on a small set of crates. +If you add a new lint, please audit the resulting warnings and make sure +there are no false positives and that the suggestions are valid. + +Refer to the tools [README] for more details. + +[README]: https://github.com/rust-lang/rust-clippy/blob/master/lintcheck/README.md +## PR + +We follow a rustc no merge-commit policy. +See . + +## Common Abbreviations + +| Abbreviation | Meaning | +| ------------ | -------------------------------------- | +| UB | Undefined Behavior | +| FP | False Positive | +| FN | False Negative | +| ICE | Internal Compiler Error | +| AST | Abstract Syntax Tree | +| MIR | Mid-Level Intermediate Representation | +| HIR | High-Level Intermediate Representation | +| TCX | Type context | + +This is a concise list of abbreviations that can come up during Clippy development. An extensive +general list can be found in the [rustc-dev-guide glossary][glossary]. Always feel free to ask if +an abbreviation or meaning is unclear to you. + +## Install from source + +If you are hacking on Clippy and want to install it from source, do the following: + +First, take note of the toolchain [override](https://rust-lang.github.io/rustup/overrides.html) in `/rust-toolchain`. +We will use this override to install Clippy into the right toolchain. + +> Tip: You can view the active toolchain for the current directory with `rustup show active-toolchain`. + +From the Clippy project root, run the following command to build the Clippy binaries and copy them into the +toolchain directory. This will override the currently installed Clippy component. + +```terminal +cargo build --release --bin cargo-clippy --bin clippy-driver -Zunstable-options --out-dir "$(rustc --print=sysroot)/bin" +``` + +Now you may run `cargo clippy` in any project, using the toolchain where you just installed Clippy. + +```terminal +cd my-project +cargo +nightly-2021-07-01 clippy +``` + +...or `clippy-driver` + +```terminal +clippy-driver +nightly-2021-07-01 +``` + +If you need to restore the default Clippy installation, run the following (from the Clippy project root). + +```terminal +rustup component remove clippy +rustup component add clippy +``` + +> **DO NOT** install using `cargo install --path . --force` since this will overwrite rustup +> [proxies](https://rust-lang.github.io/rustup/concepts/proxies.html). That is, `~/.cargo/bin/cargo-clippy` and +> `~/.cargo/bin/clippy-driver` should be hard or soft links to `~/.cargo/bin/rustup`. You can repair these by running +> `rustup update`. + + +[glossary]: https://rustc-dev-guide.rust-lang.org/appendix/glossary.html diff --git a/book/src/development/common_tools_writing_lints.md b/book/src/development/common_tools_writing_lints.md new file mode 100644 index 0000000000000..0a85f65001101 --- /dev/null +++ b/book/src/development/common_tools_writing_lints.md @@ -0,0 +1,203 @@ +# Common tools for writing lints + +You may need following tooltips to catch up with common operations. + +- [Common tools for writing lints](#common-tools-for-writing-lints) + - [Retrieving the type of an expression](#retrieving-the-type-of-an-expression) + - [Checking if an expression is calling a specific method](#checking-if-an-expr-is-calling-a-specific-method) + - [Checking if a type implements a specific trait](#checking-if-a-type-implements-a-specific-trait) + - [Checking if a type defines a specific method](#checking-if-a-type-defines-a-specific-method) + - [Dealing with macros](#dealing-with-macros) + +Useful Rustc dev guide links: +- [Stages of compilation](https://rustc-dev-guide.rust-lang.org/compiler-src.html#the-main-stages-of-compilation) +- [Type checking](https://rustc-dev-guide.rust-lang.org/type-checking.html) +- [Ty module](https://rustc-dev-guide.rust-lang.org/ty.html) + +# Retrieving the type of an expression + +Sometimes you may want to retrieve the type `Ty` of an expression `Expr`, for example to answer following questions: + +- which type does this expression correspond to (using its [`TyKind`][TyKind])? +- is it a sized type? +- is it a primitive type? +- does it implement a trait? + +This operation is performed using the [`expr_ty()`][expr_ty] method from the [`TypeckResults`][TypeckResults] struct, +that gives you access to the underlying structure [`TyS`][TyS]. + +Example of use: +```rust +impl LateLintPass<'_> for MyStructLint { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + // Get type of `expr` + let ty = cx.typeck_results().expr_ty(expr); + // Match its kind to enter its type + match ty.kind { + ty::Adt(adt_def, _) if adt_def.is_struct() => println!("Our `expr` is a struct!"), + _ => () + } + } +} +``` + +Similarly in [`TypeckResults`][TypeckResults] methods, you have the [`pat_ty()`][pat_ty] method +to retrieve a type from a pattern. + +Two noticeable items here: +- `cx` is the lint context [`LateContext`][LateContext]. The two most useful + data structures in this context are `tcx` and the `TypeckResults` returned by + `LateContext::typeck_results`, allowing us to jump to type definitions and + other compilation stages such as HIR. +- `typeck_results`'s return value is [`TypeckResults`][TypeckResults] and is + created by type checking step, it includes useful information such as types + of expressions, ways to resolve methods and so on. + +# Checking if an expr is calling a specific method + +Starting with an `expr`, you can check whether it is calling a specific method `some_method`: + +```rust +impl LateLintPass<'_> for MyStructLint { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + if_chain! { + // Check our expr is calling a method + if let hir::ExprKind::MethodCall(path, _, _args, _) = &expr.kind; + // Check the name of this method is `some_method` + if path.ident.name == sym!(some_method); + then { + // ... + } + } + } +} +``` + +# Checking if a type implements a specific trait + +There are two ways to do this, depending if the target trait is part of lang items. + +```rust +use clippy_utils::{implements_trait, match_trait_method, paths}; + +impl LateLintPass<'_> for MyStructLint { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + // 1. Using expression and Clippy's convenient method + // we use `match_trait_method` function from Clippy's toolbox + if match_trait_method(cx, expr, &paths::INTO) { + // `expr` implements `Into` trait + } + + // 2. Using type context `TyCtxt` + let ty = cx.typeck_results().expr_ty(expr); + if cx.tcx.lang_items() + // we are looking for the `DefId` of `Drop` trait in lang items + .drop_trait() + // then we use it with our type `ty` by calling `implements_trait` from Clippy's utils + .map_or(false, |id| implements_trait(cx, ty, id, &[])) { + // `expr` implements `Drop` trait + } + } +} +``` + +> Prefer using lang items, if the target trait is available there. + +A list of defined paths for Clippy can be found in [paths.rs][paths] + +We access lang items through the type context `tcx`. `tcx` is of type [`TyCtxt`][TyCtxt] and is defined in the `rustc_middle` crate. + +# Checking if a type defines a specific method + +To check if our type defines a method called `some_method`: + +```rust +use clippy_utils::{is_type_diagnostic_item, return_ty}; + +impl<'tcx> LateLintPass<'tcx> for MyTypeImpl { + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) { + if_chain! { + // Check if item is a method/function + if let ImplItemKind::Fn(ref signature, _) = impl_item.kind; + // Check the method is named `some_method` + if impl_item.ident.name == sym!(some_method); + // We can also check it has a parameter `self` + if signature.decl.implicit_self.has_implicit_self(); + // We can go further and even check if its return type is `String` + if is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(string_type)); + then { + // ... + } + } + } +} +``` + +# Dealing with macros + +There are several helpers in [`clippy_utils`][utils] to deal with macros: + +- `in_macro()`: detect if the given span is expanded by a macro + +You may want to use this for example to not start linting in any macro. + +```rust +macro_rules! foo { + ($param:expr) => { + match $param { + "bar" => println!("whatever"), + _ => () + } + }; +} + +foo!("bar"); + +// if we lint the `match` of `foo` call and test its span +assert_eq!(in_macro(match_span), true); +``` + +- `in_external_macro()`: detect if the given span is from an external macro, defined in a foreign crate + +You may want to use it for example to not start linting in macros from other crates + +```rust +#[macro_use] +extern crate a_crate_with_macros; + +// `foo` is defined in `a_crate_with_macros` +foo!("bar"); + +// if we lint the `match` of `foo` call and test its span +assert_eq!(in_external_macro(cx.sess(), match_span), true); +``` + +- `differing_macro_contexts()`: returns true if the two given spans are not from the same context + +```rust +macro_rules! m { + ($a:expr, $b:expr) => { + if $a.is_some() { + $b; + } + } +} + +let x: Option = Some(42); +m!(x, x.unwrap()); + +// These spans are not from the same context +// x.is_some() is from inside the macro +// x.unwrap() is from outside the macro +assert_eq!(differing_macro_contexts(x_is_some_span, x_unwrap_span), true); +``` + +[TyS]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TyS.html +[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.TyKind.html +[TypeckResults]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html +[expr_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.expr_ty +[LateContext]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LateContext.html +[TyCtxt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html +[pat_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TypeckResults.html#method.pat_ty +[paths]: ../clippy_utils/src/paths.rs +[utils]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/lib.rs diff --git a/book/src/infrastructure/README.md b/book/src/infrastructure/README.md new file mode 100644 index 0000000000000..2b4e5f6a6efe0 --- /dev/null +++ b/book/src/infrastructure/README.md @@ -0,0 +1 @@ +# Infrastructure diff --git a/book/src/infrastructure/backport.md b/book/src/infrastructure/backport.md new file mode 100644 index 0000000000000..15f3d1f080604 --- /dev/null +++ b/book/src/infrastructure/backport.md @@ -0,0 +1,71 @@ +# Backport Changes + +Sometimes it is necessary to backport changes to the beta release of Clippy. +Backports in Clippy are rare and should be approved by the Clippy team. For +example, a backport is done, if a crucial ICE was fixed or a lint is broken to a +point, that it has to be disabled, before landing on stable. + +Backports are done to the `beta` branch of Clippy. Backports to stable Clippy +releases basically don't exist, since this would require a Rust point release, +which is almost never justifiable for a Clippy fix. + + +## Backport the changes + +Backports are done on the beta branch of the Clippy repository. + +```bash +# Assuming the current directory corresponds to the Clippy repository +$ git checkout beta +$ git checkout -b backport +$ git cherry-pick # `` is the commit hash of the commit(s), that should be backported +$ git push origin backport +``` + +Now you should test that the backport passes all the tests in the Rust +repository. You can do this with: + +```bash +# Assuming the current directory corresponds to the Rust repository +$ git checkout beta +$ git subtree pull -p src/tools/clippy https://github.com//rust-clippy backport +$ ./x.py test src/tools/clippy +``` + +Should the test fail, you can fix Clippy directly in the Rust repository. This +has to be first applied to the Clippy beta branch and then again synced to the +Rust repository, though. The easiest way to do this is: + +```bash +# In the Rust repository +$ git diff --patch --relative=src/tools/clippy > clippy.patch +# In the Clippy repository +$ git apply /path/to/clippy.patch +$ git add -u +$ git commit -m "Fix rustup fallout" +$ git push origin backport +``` + +After this, you can open a PR to the `beta` branch of the Clippy repository. + + +## Update Clippy in the Rust Repository + +This step must be done, **after** the PR of the previous step was merged. + +After the backport landed in the Clippy repository, the branch has to be synced +back to the beta branch of the Rust repository. + +```bash +# Assuming the current directory corresponds to the Rust repository +$ git checkout beta +$ git checkout -b clippy_backport +$ git subtree pull -p src/tools/clippy https://github.com/rust-lang/rust-clippy beta +$ git push origin clippy_backport +``` + +Make sure to test the backport in the Rust repository before opening a PR. This +is done with `./x.py test src/tools/clippy`. If that passes all tests, open a PR +to the `beta` branch of the Rust repository. In this PR you should tag the +Clippy team member, that agreed to the backport or the `@rust-lang/clippy` team. +Make sure to add `[beta]` to the title of the PR. diff --git a/book/src/infrastructure/book.md b/book/src/infrastructure/book.md new file mode 100644 index 0000000000000..056d54b679243 --- /dev/null +++ b/book/src/infrastructure/book.md @@ -0,0 +1,38 @@ +# The Clippy Book + +This document explains how to make additions and changes to the Clippy book, the guide to Clippy that you're reading +right now. The Clippy book is formatted with [Markdown](https://www.markdownguide.org) and generated +by [mdbook](https://github.com/rust-lang/mdBook). + +- [Get mdbook](#get-mdbook) +- [Make changes](#make-changes) + +## Get mdbook + +While not strictly necessary since the book source is simply Markdown text files, having mdbook locally will allow you +to build, test and serve the book locally to view changes before you commit them to the repository. You likely already +have +`cargo` installed, so the easiest option is to simply: + +```shell +cargo install mdbook +``` + +See the mdbook [installation](https://github.com/rust-lang/mdBook#installation) instructions for other options. + +## Make changes + +The book's [src](https://github.com/joshrotenberg/rust-clippy/tree/clippy_guide/book/src) directory contains all of the +markdown files used to generate the book. If you want to see your changes in real time, you can use the mdbook `serve` +command to run a web server locally that will automatically update changes as they are made. From the top level of +your `rust-clippy` +directory: + +```shell +mdbook serve book --open +``` + +Then navigate to `http://localhost:3000` to see the generated book. While the server is running, changes you make will +automatically be updated. + +For more information, see the mdbook [guide](https://rust-lang.github.io/mdBook/). diff --git a/book/src/infrastructure/changelog_update.md b/book/src/infrastructure/changelog_update.md new file mode 100644 index 0000000000000..115848c48044c --- /dev/null +++ b/book/src/infrastructure/changelog_update.md @@ -0,0 +1,97 @@ +# Changelog Update + +If you want to help with updating the [changelog][changelog], you're in the right place. + +## When to update + +Typos and other small fixes/additions are _always_ welcome. + +Special care needs to be taken when it comes to updating the changelog for a new +Rust release. For that purpose, the changelog is ideally updated during the week +before an upcoming stable release. You can find the release dates on the [Rust +Forge][forge]. + +Most of the time we only need to update the changelog for minor Rust releases. It's +been very rare that Clippy changes were included in a patch release. + +## Changelog update walkthrough + +### 1. Finding the relevant Clippy commits + +Each Rust release ships with its own version of Clippy. The Clippy subtree can +be found in the `tools` directory of the Rust repository. + +Depending on the current time and what exactly you want to update, the following +bullet points might be helpful: + +* When writing the release notes for the **upcoming stable release** you need to check + out the Clippy commit of the current Rust `beta` branch. [Link][rust_beta_tools] +* When writing the release notes for the **upcoming beta release**, you need to check + out the Clippy commit of the current Rust `master`. [Link][rust_master_tools] +* When writing the (forgotten) release notes for a **past stable release**, you + need to check out the Rust release tag of the stable release. + [Link][rust_stable_tools] + +Usually you want to wirte the changelog of the **upcoming stable release**. Make +sure though, that `beta` was already branched in the Rust repository. + +To find the commit hash, issue the following command when in a `rust-lang/rust` checkout: +``` +git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g" +``` + +### 2. Fetching the PRs between those commits + +Once you've got the correct commit range, run + + util/fetch_prs_between.sh commit1 commit2 > changes.txt + +and open that file in your editor of choice. + +When updating the changelog it's also a good idea to make sure that `commit1` is +already correct in the current changelog. + +### 3. Authoring the final changelog + +The above script should have dumped all the relevant PRs to the file you +specified. It should have filtered out most of the irrelevant PRs +already, but it's a good idea to do a manual cleanup pass where you look for +more irrelevant PRs. If you're not sure about some PRs, just leave them in for +the review and ask for feedback. + +With the PRs filtered, you can start to take each PR and move the +`changelog: ` content to `CHANGELOG.md`. Adapt the wording as you see fit but +try to keep it somewhat coherent. + +The order should roughly be: + +1. New lints +2. Moves or deprecations of lints +3. Changes that expand what code existing lints cover +4. False positive fixes +5. Suggestion fixes/improvements +6. ICE fixes +7. Documentation improvements +8. Others + +As section headers, we use: + +``` +### New Lints +### Moves and Deprecations +### Enhancements +### False Positive Fixes +### Suggestion Fixes/Improvements +### ICE Fixes +### Documentation Improvements +### Others +``` + +Please also be sure to update the Beta/Unreleased sections at the top with the +relevant commit ranges. + +[changelog]: https://github.com/rust-lang/rust-clippy/blob/master/CHANGELOG.md +[forge]: https://forge.rust-lang.org/ +[rust_master_tools]: https://github.com/rust-lang/rust/tree/master/src/tools/clippy +[rust_beta_tools]: https://github.com/rust-lang/rust/tree/beta/src/tools/clippy +[rust_stable_tools]: https://github.com/rust-lang/rust/releases diff --git a/book/src/infrastructure/release.md b/book/src/infrastructure/release.md new file mode 100644 index 0000000000000..afe3033c288cf --- /dev/null +++ b/book/src/infrastructure/release.md @@ -0,0 +1,124 @@ +# Release a new Clippy Version + +_NOTE: This document is probably only relevant to you, if you're a member of the +Clippy team._ + +Clippy is released together with stable Rust releases. The dates for these +releases can be found at the [Rust Forge]. This document explains the necessary +steps to create a Clippy release. + +1. [Remerge the `beta` branch](#remerge-the-beta-branch) +2. [Update the `beta` branch](#update-the-beta-branch) +3. [Find the Clippy commit](#find-the-clippy-commit) +4. [Tag the stable commit](#tag-the-stable-commit) +5. [Update `CHANGELOG.md`](#update-changelogmd) + +_NOTE: This document is for stable Rust releases, not for point releases. For +point releases, step 1. and 2. should be enough._ + +[Rust Forge]: https://forge.rust-lang.org/ + + +## Remerge the `beta` branch + +This step is only necessary, if since the last release something was backported +to the beta Rust release. The remerge is then necessary, to make sure that the +Clippy commit, that was used by the now stable Rust release, persists in the +tree of the Clippy repository. + +To find out if this step is necessary run + +```bash +# Assumes that the local master branch is up-to-date +$ git fetch upstream +$ git branch master --contains upstream/beta +``` + +If this command outputs `master`, this step is **not** necessary. + +```bash +# Assuming `HEAD` is the current `master` branch of rust-lang/rust-clippy +$ git checkout -b backport_remerge +$ git merge upstream/beta +$ git diff # This diff has to be empty, otherwise something with the remerge failed +$ git push origin backport_remerge # This can be pushed to your fork +``` + +After this, open a PR to the master branch. In this PR, the commit hash of the +`HEAD` of the `beta` branch must exists. In addition to that, no files should +be changed by this PR. + + +## Update the `beta` branch + +This step must be done **after** the PR of the previous step was merged. + +First, the Clippy commit of the `beta` branch of the Rust repository has to be +determined. + +```bash +# Assuming the current directory corresponds to the Rust repository +$ git checkout beta +$ BETA_SHA=$(git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g") +``` + +After finding the Clippy commit, the `beta` branch in the Clippy repository can +be updated. + +```bash +# Assuming the current directory corresponds to the Clippy repository +$ git checkout beta +$ git reset --hard $BETA_SHA +$ git push upstream beta +``` + + +## Find the Clippy commit + +The first step is to tag the Clippy commit, that is included in the stable Rust +release. This commit can be found in the Rust repository. + +```bash +# Assuming the current directory corresponds to the Rust repository +$ git fetch upstream # `upstream` is the `rust-lang/rust` remote +$ git checkout 1.XX.0 # XX should be exchanged with the corresponding version +$ SHA=$(git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g") +``` + + +## Tag the stable commit + +After finding the Clippy commit, it can be tagged with the release number. + +```bash +# Assuming the current directory corresponds to the Clippy repository +$ git checkout $SHA +$ git tag rust-1.XX.0 # XX should be exchanged with the corresponding version +$ git push upstream rust-1.XX.0 # `upstream` is the `rust-lang/rust-clippy` remote +``` + +After this, the release should be available on the Clippy [release page]. + +[release page]: https://github.com/rust-lang/rust-clippy/releases + +## Update the `stable` branch + +At this step you should have already checked out the commit of the `rust-1.XX.0` +tag. Updating the stable branch from here is as easy as: + +```bash +# Assuming the current directory corresponds to the Clippy repository and the +# commit of the just created rust-1.XX.0 tag is checked out. +$ git push upstream rust-1.XX.0:stable # `upstream` is the `rust-lang/rust-clippy` remote +``` + +_NOTE: Usually there are no stable backports for Clippy, so this update should +be possible without force pushing or anything like this. If there should have +happened a stable backport, make sure to re-merge those changes just as with the +`beta` branch._ + +## Update `CHANGELOG.md` + +For this see the document on [how to update the changelog]. + +[how to update the changelog]: https://github.com/rust-lang/rust-clippy/blob/master/doc/changelog_update.md diff --git a/book/src/installation_and_usage.md b/book/src/installation_and_usage.md new file mode 100644 index 0000000000000..190c8ed5342ab --- /dev/null +++ b/book/src/installation_and_usage.md @@ -0,0 +1,108 @@ +# Installation and Usage + +Below are instructions on how to use Clippy as a subcommand, compiled from source +or in Travis CI. Note that Clippy is installed as a +[component](https://rust-lang.github.io/rustup/concepts/components.html?highlight=clippy#components) as part of the +[rustup](https://rust-lang.github.io/rustup/installation/index.html) installation. + +### As a cargo subcommand (`cargo clippy`) + +One way to use Clippy is by installing Clippy through rustup as a cargo +subcommand. + +#### Step 1: Install rustup + +You can install [rustup](https://rustup.rs/) on supported platforms. This will help +us install Clippy and its dependencies. + +If you already have rustup installed, update to ensure you have the latest +rustup and compiler: + +```terminal +rustup update +``` + +#### Step 2: Install Clippy + +Once you have rustup and the latest stable release (at least Rust 1.29) installed, run the following command: + +```terminal +rustup component add clippy +``` +If it says that it can't find the `clippy` component, please run `rustup self update`. + +#### Step 3: Run Clippy + +Now you can run Clippy by invoking the following command: + +```terminal +cargo clippy +``` + +#### Automatically applying Clippy suggestions + +Clippy can automatically apply some lint suggestions. +Note that this is still experimental and only supported on the nightly channel: + +```terminal +cargo clippy --fix +``` + +#### Workspaces + +All the usual workspace options should work with Clippy. For example the following command +will run Clippy on the `example` crate: + +```terminal +cargo clippy -p example +``` + +As with `cargo check`, this includes dependencies that are members of the workspace, like path dependencies. +If you want to run Clippy **only** on the given crate, use the `--no-deps` option like this: + +```terminal +cargo clippy -p example --no-deps +``` + +### As a rustc replacement (`clippy-driver`) + +Clippy can also be used in projects that do not use cargo. To do so, you will need to replace +your `rustc` compilation commands with `clippy-driver`. For example, if your project runs: + +```terminal +rustc --edition 2018 -Cpanic=abort foo.rs +``` + +Then, to enable Clippy, you will need to call: + +```terminal +clippy-driver --edition 2018 -Cpanic=abort foo.rs +``` + +Note that `rustc` will still run, i.e. it will still emit the output files it normally does. + +### Travis CI + +You can add Clippy to Travis CI in the same way you use it locally: + +```yml +language: rust +rust: + - stable + - beta +before_script: + - rustup component add clippy +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 non-default crate features, use + - cargo clippy --all-targets --all-features -- -D warnings + - cargo test + # etc. +``` + +Note that adding `-D warnings` will cause your build to fail if **any** warnings are found in your code. +That includes warnings found by rustc (e.g. `dead_code`, etc.). If you want to avoid this and only cause +an error for Clippy warnings, use `#![deny(clippy::all)]` in your code or `-D clippy::all` on the command +line. (You can swap `clippy::all` with the specific lint category you are targeting.) diff --git a/book/src/lints/README.md b/book/src/lints/README.md new file mode 100644 index 0000000000000..a2777e8f4d583 --- /dev/null +++ b/book/src/lints/README.md @@ -0,0 +1 @@ +# Clippy's Lints diff --git a/book/src/lints/cargo.md b/book/src/lints/cargo.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/complexity.md b/book/src/lints/complexity.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/correctness.md b/book/src/lints/correctness.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/deprecated.md b/book/src/lints/deprecated.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/nursery.md b/book/src/lints/nursery.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/pedantic.md b/book/src/lints/pedantic.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/perf.md b/book/src/lints/perf.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/restriction.md b/book/src/lints/restriction.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/lints/style.md b/book/src/lints/style.md new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/book/src/roadmap/2021.md b/book/src/roadmap/2021.md new file mode 100644 index 0000000000000..fe8b080f56f2b --- /dev/null +++ b/book/src/roadmap/2021.md @@ -0,0 +1,235 @@ +# Roadmap 2021 + +# Summary + +This Roadmap lays out the plans for Clippy in 2021: + +- Improving usability and reliability +- Improving experience of contributors and maintainers +- Develop and specify processes + +Members of the Clippy team will be assigned tasks from one or more of these +topics. The team member is then responsible to complete the assigned tasks. This +can either be done by implementing them or by providing mentorship to interested +contributors. + +# Motivation + +With the ongoing growth of the Rust language and with that of the whole +ecosystem, also Clippy gets more and more users and contributors. This is good +for the project, but also brings challenges along. Some of these challenges are: + +- More issues about reliability or usability are popping up +- Traffic is hard to handle for a small team +- Bigger projects don't get completed due to the lack of processes and/or time + of the team members + +Additionally, according to the [Rust Roadmap 2021], clear processes should be +defined by every team and unified across teams. This Roadmap is the first step +towards this. + +[Rust Roadmap 2021]: https://github.com/rust-lang/rfcs/pull/3037 + +# Explanation + +This section will explain the things that should be done in 2021. It is +important to note, that this document focuses on the "What?", not the "How?". +The later will be addressed in follow-up tracking issue, with an assigned team +member. + +The following is split up in two major sections. The first section covers the +user facing plans, the second section the internal plans. + +## User Facing + +Clippy should be as pleasant to use and configure as possible. This section +covers plans that should be implemented to improve the situation of Clippy in +this regard. + +### Usability + +In the following, plans to improve the usability are covered. + +#### No Output After `cargo check` + +Currently when `cargo clippy` is run after `cargo check`, it does not produce +any output. This is especially problematic since `rust-analyzer` is on the rise +and it uses `cargo check` for checking code. A fix is already implemented, but +it still has to be pushed over the finish line. This also includes the +stabilization of the `cargo clippy --fix` command or the support of multi-span +suggestions in `rustfix`. + +- [#4612](https://github.com/rust-lang/rust-clippy/issues/4612) + +#### `lints.toml` Configuration + +This is something that comes up every now and then: a reusable configuration +file, where lint levels can be defined. Discussions about this often lead to +nothing specific or to "we need an RFC for this". And this is exactly what needs +to be done. Get together with the cargo team and write an RFC and implement such +a configuration file somehow and somewhere. + +- [#3164](https://github.com/rust-lang/rust-clippy/issues/3164) +- [cargo#5034](https://github.com/rust-lang/cargo/issues/5034) +- [IRLO](https://internals.rust-lang.org/t/proposal-cargo-lint-configuration/9135/8) + +#### Lint Groups + +There are more and more issues about managing lints in Clippy popping up. Lints +are hard to implement with a guarantee of no/few false positives (FPs). One way +to address this might be to introduce more lint groups to give users the ability +to better manage lints, or improve the process of classifying lints, so that +disabling lints due to FPs becomes rare. It is important to note, that Clippy +lints are less conservative than `rustc` lints, which won't change in the +future. + +- [#5537](https://github.com/rust-lang/rust-clippy/issues/5537) +- [#6366](https://github.com/rust-lang/rust-clippy/issues/6366) + +### Reliability + +In the following, plans to improve the reliability are covered. + +#### False Positive Rate + +In the worst case, new lints are only available in nightly for 2 weeks, before +hitting beta and ultimately stable. This and the fact that fewer people use +nightly Rust nowadays makes it more probable that a lint with many FPs hits +stable. This leads to annoyed users, that will disable these new lints in the +best case and to more annoyed users, that will stop using Clippy in the worst. +A process should be developed and implemented to prevent this from happening. + +- [#6429](https://github.com/rust-lang/rust-clippy/issues/6429) + +## Internal + +(The end of) 2020 has shown, that Clippy has to think about the available +resources, especially regarding management and maintenance of the project. This +section address issues affecting team members and contributors. + +### Management + +In 2020 Clippy achieved over 1000 open issues with regularly between 25-35 open +PRs. This is simultaneously a win and a loss. More issues and PRs means more +people are interested in Clippy and in contributing to it. On the other hand, it +means for team members more work and for contributors longer wait times for +reviews. The following will describe plans how to improve the situation for both +team members and contributors. + +#### Clear Expectations for Team Members + +According to the [Rust Roadmap 2021], a document specifying what it means to be +a member of the team should be produced. This should not put more pressure on +the team members, but rather help them and interested folks to know what the +expectations are. With this it should also be easier to recruit new team members +and may encourage people to get in touch, if they're interested to join. + +#### Scaling up the Team + +More people means less work for each individual. Together with the document +about expectations for team members, a document defining the process of how to +join the team should be produced. This can also increase the stability of the +team, in case of current members dropping out (temporarily). There can also be +different roles in the team, like people triaging vs. people reviewing. + +#### Regular Meetings + +Other teams have regular meetings. Clippy is big enough that it might be worth +to also do them. Especially if more people join the team, this can be important +for sync-ups. Besides the asynchronous communication, that works well for +working on separate lints, a meeting adds a synchronous alternative at a known +time. This is especially helpful if there are bigger things that need to be +discussed (like the projects in this roadmap). For starters bi-weekly meetings +before Rust syncs might make sense. + +#### Triaging + +To get a handle on the influx of open issues, a process for triaging issues and +PRs should be developed. Officially, Clippy follows the Rust triage process, but +currently no one enforces it. This can be improved by sharing triage teams +across projects or by implementing dashboards / tools which simplify triaging. + +### Development + +Improving the developer and contributor experience is something the Clippy team +works on regularly. Though, some things might need special attention and +planing. These topics are listed in the following. + +#### Process for New and Existing Lints + +As already mentioned above, classifying new lints gets quite hard, because the +probability of a buggy lint getting into stable is quite high. A process should +be implemented on how to classify lints. In addition, a test system should be +developed to find out which lints are currently problematic in real world code +to fix or disable them. + +- [#6429 (comment)](https://github.com/rust-lang/rust-clippy/issues/6429#issuecomment-741056379) +- [#6429 (comment)](https://github.com/rust-lang/rust-clippy/issues/6429#issuecomment-741153345) + +#### Processes + +Related to the point before, a process for suggesting and discussing major +changes should be implemented. It's also not clearly defined when a lint should +be enabled or disabled by default. This can also be improved by the test system +mentioned above. + +#### Dev-Tools + +There's already `cargo dev` which makes Clippy development easier and more +pleasant. This can still be expanded, so that it covers more areas of the +development process. + +- [#5394](https://github.com/rust-lang/rust-clippy/issues/5394) + +#### Contributor Guide + +Similar to a Clippy Book, which describes how to use Clippy, a book about how to +contribute to Clippy might be helpful for new and existing contributors. There's +already the `doc` directory in the Clippy repo, this can be turned into a +`mdbook`. + +#### `rustc` integration + +Recently Clippy was integrated with `git subtree` into the `rust-lang/rust` +repository. This made syncing between the two repositories easier. A +`#[non_exhaustive]` list of things that still can be improved is: + +1. Use the same `rustfmt` version and configuration as `rustc`. +2. Make `cargo dev` work in the Rust repo, just as it works in the Clippy repo. + E.g. `cargo dev bless` or `cargo dev update_lints`. And even add more things + to it that might be useful for the Rust repo, e.g. `cargo dev deprecate`. +3. Easier sync process. The `subtree` situation is not ideal. + +## Prioritization + +The most pressing issues for users of Clippy are of course the user facing +issues. So there should be a priority on those issues, but without losing track +of the internal issues listed in this document. + +Getting the FP rate of warn/deny-by-default lints under control should have the +highest priority. Other user facing issues should also get a high priority, but +shouldn't be in the way of addressing internal issues. + +To better manage the upcoming projects, the basic internal processes, like +meetings, tracking issues and documentation, should be established as soon as +possible. They might even be necessary to properly manage the projects, +regarding the user facing issues. + +# Prior Art + +## Rust Roadmap + +Rust's roadmap process was established by [RFC 1728] in 2016. Since then every +year a roadmap was published, that defined the bigger plans for the coming +years. This years roadmap can be found [here][Rust Roadmap 2021]. + +[RFC 1728]: https://rust-lang.github.io/rfcs/1728-north-star.html + +# Drawbacks + +## Big Roadmap + +This roadmap is pretty big and not all items listed in this document might be +addressed during 2021. Because this is the first roadmap for Clippy, having open +tasks at the end of 2021 is fine, but they should be revisited in the 2022 +roadmap. diff --git a/book/src/roadmap/README.md b/book/src/roadmap/README.md new file mode 100644 index 0000000000000..e69de29bb2d1d