mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-12-18 00:53:31 +00:00
Move internal documentation to book
This commit is contained in:
parent
853d7eeed6
commit
af385799e0
12 changed files with 247 additions and 1819 deletions
|
@ -11,21 +11,24 @@ because that's clearly a non-descriptive name.
|
||||||
- [Setup](#setup)
|
- [Setup](#setup)
|
||||||
- [Getting Started](#getting-started)
|
- [Getting Started](#getting-started)
|
||||||
- [Testing](#testing)
|
- [Testing](#testing)
|
||||||
|
- [Cargo lints](#cargo-lints)
|
||||||
- [Rustfix tests](#rustfix-tests)
|
- [Rustfix tests](#rustfix-tests)
|
||||||
- [Edition 2018 tests](#edition-2018-tests)
|
- [Edition 2018 tests](#edition-2018-tests)
|
||||||
- [Testing manually](#testing-manually)
|
- [Testing manually](#testing-manually)
|
||||||
- [Lint declaration](#lint-declaration)
|
- [Lint declaration](#lint-declaration)
|
||||||
|
- [Lint registration](#lint-registration)
|
||||||
- [Lint passes](#lint-passes)
|
- [Lint passes](#lint-passes)
|
||||||
- [Emitting a lint](#emitting-a-lint)
|
- [Emitting a lint](#emitting-a-lint)
|
||||||
- [Adding the lint logic](#adding-the-lint-logic)
|
- [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)
|
- [Specifying the lint's minimum supported Rust version (MSRV)](#specifying-the-lints-minimum-supported-rust-version-msrv)
|
||||||
- [Author lint](#author-lint)
|
- [Author lint](#author-lint)
|
||||||
|
- [Print HIR lint](#print-hir-lint)
|
||||||
- [Documentation](#documentation)
|
- [Documentation](#documentation)
|
||||||
- [Running rustfmt](#running-rustfmt)
|
- [Running rustfmt](#running-rustfmt)
|
||||||
- [Debugging](#debugging)
|
- [Debugging](#debugging)
|
||||||
- [PR Checklist](#pr-checklist)
|
- [PR Checklist](#pr-checklist)
|
||||||
- [Adding configuration to a lint](#adding-configuration-to-a-lint)
|
- [Adding configuration to a lint](#adding-configuration-to-a-lint)
|
||||||
- [Cheatsheet](#cheatsheet)
|
- [Cheat Sheet](#cheat-sheet)
|
||||||
|
|
||||||
## Setup
|
## Setup
|
||||||
|
|
||||||
|
@ -42,9 +45,9 @@ 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
|
lint you can run `cargo dev new_lint --name=foo_functions --pass=early
|
||||||
--category=pedantic` (category will default to nursery if not provided). This
|
--category=pedantic` (category will default to nursery if not provided). This
|
||||||
command will create two files: `tests/ui/foo_functions.rs` and
|
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
|
`clippy_lints/src/foo_functions.rs`, as well as
|
||||||
register the new lint. For cargo lints, two project hierarchies (fail/pass) will
|
[registering the lint](#lint-registration). For cargo lints, two project
|
||||||
be created by default under `tests/ui-cargo`.
|
hierarchies (fail/pass) will be created by default under `tests/ui-cargo`.
|
||||||
|
|
||||||
Next, we'll open up these files and add our lint!
|
Next, we'll open up these files and add our lint!
|
||||||
|
|
||||||
|
@ -155,7 +158,7 @@ Manually testing against an example file can be useful if you have added some
|
||||||
your local modifications, run
|
your local modifications, run
|
||||||
|
|
||||||
```
|
```
|
||||||
env __CLIPPY_INTERNAL_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug input.rs
|
cargo dev lint input.rs
|
||||||
```
|
```
|
||||||
|
|
||||||
from the working copy root. With tests in place, let's have a look at
|
from the working copy root. With tests in place, let's have a look at
|
||||||
|
@ -179,17 +182,15 @@ the auto-generated lint declaration to have a real description, something like t
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// **What it does:**
|
/// ### What it does
|
||||||
///
|
///
|
||||||
/// **Why is this bad?**
|
/// ### Why is this bad?
|
||||||
///
|
|
||||||
/// **Known problems:** None.
|
|
||||||
///
|
|
||||||
/// **Example:**
|
|
||||||
///
|
///
|
||||||
|
/// ### Example
|
||||||
/// ```rust
|
/// ```rust
|
||||||
/// // example code
|
/// // example code
|
||||||
/// ```
|
/// ```
|
||||||
|
#[clippy::version = "1.29.0"]
|
||||||
pub FOO_FUNCTIONS,
|
pub FOO_FUNCTIONS,
|
||||||
pedantic,
|
pedantic,
|
||||||
"function named `foo`, which is not a descriptive name"
|
"function named `foo`, which is not a descriptive name"
|
||||||
|
@ -200,6 +201,10 @@ declare_clippy_lint! {
|
||||||
section. This is the default documentation style and will be displayed
|
section. This is the default documentation style and will be displayed
|
||||||
[like this][example_lint_page]. To render and open this documentation locally
|
[like this][example_lint_page]. To render and open this documentation locally
|
||||||
in a browser, run `cargo dev serve`.
|
in a browser, run `cargo dev serve`.
|
||||||
|
* The `#[clippy::version]` attribute will be rendered as part of the lint documentation.
|
||||||
|
The value should be set to the current Rust version that the lint is developed in,
|
||||||
|
it can be retrieved by running `rustc -vV` in the rust-clippy directory. The version
|
||||||
|
is listed under *release*. (Use the version without the `-nightly`) suffix.
|
||||||
* `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the
|
* `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the
|
||||||
[lint naming guidelines][lint_naming] here when naming your lint.
|
[lint naming guidelines][lint_naming] here when naming your lint.
|
||||||
In short, the name should state the thing that is being checked for and
|
In short, the name should state the thing that is being checked for and
|
||||||
|
@ -222,32 +227,34 @@ declare_lint_pass!(FooFunctions => [FOO_FUNCTIONS]);
|
||||||
impl EarlyLintPass for FooFunctions {}
|
impl EarlyLintPass for FooFunctions {}
|
||||||
```
|
```
|
||||||
|
|
||||||
Normally after declaring the lint, we have to run `cargo dev update_lints`,
|
[declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L60
|
||||||
which updates some files, so Clippy knows about the new lint. Since we used
|
[example_lint_page]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
|
||||||
`cargo dev new_lint ...` to generate the lint declaration, this was done
|
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
|
||||||
automatically. While `update_lints` automates most of the things, it doesn't
|
[category_level_mapping]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L110
|
||||||
automate everything. We will have to register our lint pass manually in the
|
|
||||||
`register_plugins` function in `clippy_lints/src/lib.rs`:
|
## Lint registration
|
||||||
|
|
||||||
|
When using `cargo dev new_lint`, the lint is automatically registered and
|
||||||
|
nothing more has to be done.
|
||||||
|
|
||||||
|
When declaring a new lint by hand and `cargo dev update_lints` is used, the lint
|
||||||
|
pass may have to be registered manually in the `register_plugins` function in
|
||||||
|
`clippy_lints/src/lib.rs`:
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
store.register_early_pass(|| box foo_functions::FooFunctions);
|
store.register_early_pass(|| Box::new(foo_functions::FooFunctions));
|
||||||
```
|
```
|
||||||
|
|
||||||
As one may expect, there is a corresponding `register_late_pass` method
|
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
|
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.
|
`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
|
One reason that `cargo dev update_lints` does not automate this step is that
|
||||||
can use the same lint pass, so registering the lint pass may already be done
|
multiple lints can use the same lint pass, so registering the lint pass may
|
||||||
when adding a new lint. Another reason that this step is not automated is that
|
already be done when adding a new lint. Another reason that this step is not
|
||||||
the order that the passes are registered determines the order the passes
|
automated is that the order that the passes are registered determines the order
|
||||||
actually run, which in turn affects the order that any emitted lints are output
|
the passes actually run, which in turn affects the order that any emitted lints
|
||||||
in.
|
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
|
## Lint passes
|
||||||
|
|
||||||
|
@ -425,7 +432,7 @@ The project's MSRV can then be matched against the feature MSRV in the LintPass
|
||||||
using the `meets_msrv` utility function.
|
using the `meets_msrv` utility function.
|
||||||
|
|
||||||
``` rust
|
``` rust
|
||||||
if !meets_msrv(self.msrv.as_ref(), &msrvs::STR_STRIP_PREFIX) {
|
if !meets_msrv(self.msrv, msrvs::STR_STRIP_PREFIX) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
@ -478,6 +485,19 @@ you are implementing your lint.
|
||||||
|
|
||||||
[author_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=9a12cb60e5c6ad4e3003ac6d5e63cf55
|
[author_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=9a12cb60e5c6ad4e3003ac6d5e63cf55
|
||||||
|
|
||||||
|
## Print HIR lint
|
||||||
|
|
||||||
|
To implement a lint, it's helpful to first understand the internal representation
|
||||||
|
that rustc uses. Clippy has the `#[clippy::dump]` attribute that prints the
|
||||||
|
[_High-Level Intermediate Representation (HIR)_] of the item, statement, or
|
||||||
|
expression that the attribute is attached to. To attach the attribute to expressions
|
||||||
|
you often need to enable `#![feature(stmt_expr_attributes)]`.
|
||||||
|
|
||||||
|
[Here][print_hir_example] you can find an example, just select _Tools_ and run _Clippy_.
|
||||||
|
|
||||||
|
[_High-Level Intermediate Representation (HIR)_]: https://rustc-dev-guide.rust-lang.org/hir.html
|
||||||
|
[print_hir_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=daf14db3a7f39ca467cd1b86c34b9afb
|
||||||
|
|
||||||
## Documentation
|
## Documentation
|
||||||
|
|
||||||
The final thing before submitting our PR is to add some documentation to our
|
The final thing before submitting our PR is to add some documentation to our
|
||||||
|
@ -487,21 +507,23 @@ Please document your lint with a doc comment akin to the following:
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// **What it does:** Checks for ... (describe what the lint matches).
|
/// ### What it does
|
||||||
|
/// Checks for ... (describe what the lint matches).
|
||||||
///
|
///
|
||||||
/// **Why is this bad?** Supply the reason for linting the code.
|
/// ### Why is this bad?
|
||||||
|
/// Supply the reason for linting the code.
|
||||||
///
|
///
|
||||||
/// **Known problems:** None. (Or describe where it could go wrong.)
|
/// ### Example
|
||||||
///
|
|
||||||
/// **Example:**
|
|
||||||
///
|
///
|
||||||
/// ```rust,ignore
|
/// ```rust,ignore
|
||||||
/// // Bad
|
/// // A short example of code that triggers the lint
|
||||||
/// Insert a short example of code that triggers the lint
|
|
||||||
///
|
|
||||||
/// // Good
|
|
||||||
/// Insert a short example of improved code that doesn't trigger the lint
|
|
||||||
/// ```
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```rust,ignore
|
||||||
|
/// // A short example of improved code that doesn't trigger the lint
|
||||||
|
/// ```
|
||||||
|
#[clippy::version = "1.29.0"]
|
||||||
pub FOO_FUNCTIONS,
|
pub FOO_FUNCTIONS,
|
||||||
pedantic,
|
pedantic,
|
||||||
"function named `foo`, which is not a descriptive name"
|
"function named `foo`, which is not a descriptive name"
|
||||||
|
@ -558,14 +580,16 @@ directory. Adding a configuration to a lint can be useful for thresholds or to c
|
||||||
behavior that can be seen as a false positive for some users. Adding a configuration is done
|
behavior that can be seen as a false positive for some users. Adding a configuration is done
|
||||||
in the following steps:
|
in the following steps:
|
||||||
|
|
||||||
1. Adding a new configuration entry to [clippy_utils::conf](/clippy_utils/src/conf.rs)
|
1. Adding a new configuration entry to [clippy_lints::utils::conf](/clippy_lints/src/utils/conf.rs)
|
||||||
like this:
|
like this:
|
||||||
```rust
|
```rust
|
||||||
/// Lint: LINT_NAME. <The configuration field doc comment>
|
/// Lint: LINT_NAME.
|
||||||
|
///
|
||||||
|
/// <The configuration field doc comment>
|
||||||
(configuration_ident: Type = DefaultValue),
|
(configuration_ident: Type = DefaultValue),
|
||||||
```
|
```
|
||||||
The configuration value and identifier should usually be the same. The doc comment will be
|
The doc comment is automatically added to the documentation of the listed lints. The default
|
||||||
automatically added to the lint documentation.
|
value will be formatted using the `Debug` implementation of the type.
|
||||||
2. Adding the configuration value to the lint impl struct:
|
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
|
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
|
generated with the `declare_lint_pass!` macro. This struct needs to be defined manually
|
||||||
|
@ -626,14 +650,14 @@ in the following steps:
|
||||||
with the configuration value and a rust file that should be linted by Clippy. The test can
|
with the configuration value and a rust file that should be linted by Clippy. The test can
|
||||||
otherwise be written as usual.
|
otherwise be written as usual.
|
||||||
|
|
||||||
## Cheatsheet
|
## Cheat Sheet
|
||||||
|
|
||||||
Here are some pointers to things you are likely going to need for every lint:
|
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
|
* [Clippy utils][utils] - Various helper functions. Maybe the function you need
|
||||||
is already in here (`implements_trait`, `match_def_path`, `snippet`, etc)
|
is already in here ([`is_type_diagnostic_item`], [`implements_trait`], [`snippet`], etc)
|
||||||
* [Clippy diagnostics][diagnostics]
|
* [Clippy diagnostics][diagnostics]
|
||||||
* [The `if_chain` macro][if_chain]
|
* [Let chains][let-chains]
|
||||||
* [`from_expansion`][from_expansion] and [`in_external_macro`][in_external_macro]
|
* [`from_expansion`][from_expansion] and [`in_external_macro`][in_external_macro]
|
||||||
* [`Span`][span]
|
* [`Span`][span]
|
||||||
* [`Applicability`][applicability]
|
* [`Applicability`][applicability]
|
||||||
|
@ -657,8 +681,11 @@ 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,
|
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.
|
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
|
[utils]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/index.html
|
||||||
[if_chain]: https://docs.rs/if_chain/*/if_chain/
|
[`is_type_diagnostic_item`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.is_type_diagnostic_item.html
|
||||||
|
[`implements_trait`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.implements_trait.html
|
||||||
|
[`snippet`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/source/fn.snippet.html
|
||||||
|
[let-chains]: https://github.com/rust-lang/rust/pull/94927
|
||||||
[from_expansion]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html#method.from_expansion
|
[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
|
[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
|
[span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html
|
||||||
|
|
|
@ -91,15 +91,18 @@ cargo dev fmt
|
||||||
cargo dev update_lints
|
cargo dev update_lints
|
||||||
# create a new lint and register it
|
# create a new lint and register it
|
||||||
cargo dev new_lint
|
cargo dev new_lint
|
||||||
|
# automatically formatting all code before each commit
|
||||||
|
cargo dev setup git-hook
|
||||||
# (experimental) Setup Clippy to work with IntelliJ-Rust
|
# (experimental) Setup Clippy to work with IntelliJ-Rust
|
||||||
cargo dev ide_setup
|
cargo dev setup intellij
|
||||||
```
|
```
|
||||||
|
More about intellij command usage and reasons [here](../CONTRIBUTING.md#intellij-rust)
|
||||||
|
|
||||||
## lintcheck
|
## lintcheck
|
||||||
`cargo lintcheck` will build and run clippy on a fixed set of crates and generate a log of the results.
|
`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
|
You can `git diff` the updated log against its previous version and
|
||||||
see what impact your lint made on a small set of crates.
|
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
|
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.
|
there are no false positives and that the suggestions are valid.
|
||||||
|
|
||||||
Refer to the tools [README] for more details.
|
Refer to the tools [README] for more details.
|
||||||
|
@ -167,6 +170,5 @@ rustup component add clippy
|
||||||
> [proxies](https://rust-lang.github.io/rustup/concepts/proxies.html). That is, `~/.cargo/bin/cargo-clippy` and
|
> [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
|
> `~/.cargo/bin/clippy-driver` should be hard or soft links to `~/.cargo/bin/rustup`. You can repair these by running
|
||||||
> `rustup update`.
|
> `rustup update`.
|
||||||
|
|
||||||
|
|
||||||
[glossary]: https://rustc-dev-guide.rust-lang.org/appendix/glossary.html
|
[glossary]: https://rustc-dev-guide.rust-lang.org/appendix/glossary.html
|
||||||
|
|
|
@ -4,17 +4,19 @@ You may need following tooltips to catch up with common operations.
|
||||||
|
|
||||||
- [Common tools for writing lints](#common-tools-for-writing-lints)
|
- [Common tools for writing lints](#common-tools-for-writing-lints)
|
||||||
- [Retrieving the type of an expression](#retrieving-the-type-of-an-expression)
|
- [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 an expr is calling a specific method](#checking-if-an-expr-is-calling-a-specific-method)
|
||||||
|
- [Checking for a specific type](#checking-for-a-specific-type)
|
||||||
- [Checking if a type implements a specific trait](#checking-if-a-type-implements-a-specific-trait)
|
- [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)
|
- [Checking if a type defines a specific method](#checking-if-a-type-defines-a-specific-method)
|
||||||
- [Dealing with macros](#dealing-with-macros)
|
- [Dealing with macros](#dealing-with-macros-and-expansions)
|
||||||
|
|
||||||
Useful Rustc dev guide links:
|
Useful Rustc dev guide links:
|
||||||
- [Stages of compilation](https://rustc-dev-guide.rust-lang.org/compiler-src.html#the-main-stages-of-compilation)
|
- [Stages of compilation](https://rustc-dev-guide.rust-lang.org/compiler-src.html#the-main-stages-of-compilation)
|
||||||
|
- [Diagnostic items](https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-items.html)
|
||||||
- [Type checking](https://rustc-dev-guide.rust-lang.org/type-checking.html)
|
- [Type checking](https://rustc-dev-guide.rust-lang.org/type-checking.html)
|
||||||
- [Ty module](https://rustc-dev-guide.rust-lang.org/ty.html)
|
- [Ty module](https://rustc-dev-guide.rust-lang.org/ty.html)
|
||||||
|
|
||||||
# Retrieving the type of an expression
|
## 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:
|
Sometimes you may want to retrieve the type `Ty` of an expression `Expr`, for example to answer following questions:
|
||||||
|
|
||||||
|
@ -24,7 +26,7 @@ Sometimes you may want to retrieve the type `Ty` of an expression `Expr`, for ex
|
||||||
- does it implement a trait?
|
- does it implement a trait?
|
||||||
|
|
||||||
This operation is performed using the [`expr_ty()`][expr_ty] method from the [`TypeckResults`][TypeckResults] struct,
|
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].
|
that gives you access to the underlying structure [`Ty`][Ty].
|
||||||
|
|
||||||
Example of use:
|
Example of use:
|
||||||
```rust
|
```rust
|
||||||
|
@ -53,42 +55,81 @@ Two noticeable items here:
|
||||||
created by type checking step, it includes useful information such as types
|
created by type checking step, it includes useful information such as types
|
||||||
of expressions, ways to resolve methods and so on.
|
of expressions, ways to resolve methods and so on.
|
||||||
|
|
||||||
# Checking if an expr is calling a specific method
|
## 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`:
|
Starting with an `expr`, you can check whether it is calling a specific method `some_method`:
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
impl LateLintPass<'_> for MyStructLint {
|
impl<'tcx> LateLintPass<'tcx> for MyStructLint {
|
||||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
|
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
|
||||||
if_chain! {
|
// Check our expr is calling a method
|
||||||
// Check our expr is calling a method
|
if let hir::ExprKind::MethodCall(path, _, [_self_arg, ..]) = &expr.kind
|
||||||
if let hir::ExprKind::MethodCall(path, _, _args, _) = &expr.kind;
|
|
||||||
// Check the name of this method is `some_method`
|
// Check the name of this method is `some_method`
|
||||||
if path.ident.name == sym!(some_method);
|
&& path.ident.name == sym!(some_method)
|
||||||
then {
|
// Optionally, check the type of the self argument.
|
||||||
|
// - See "Checking for a specific type"
|
||||||
|
{
|
||||||
// ...
|
// ...
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
# Checking if a type implements a specific trait
|
## Checking for a specific type
|
||||||
|
|
||||||
There are two ways to do this, depending if the target trait is part of lang items.
|
There are three ways to check if an expression type is a specific type we want to check for.
|
||||||
|
All of these methods only check for the base type, generic arguments have to be checked separately.
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
use clippy_utils::{implements_trait, match_trait_method, paths};
|
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
|
||||||
|
use clippy_utils::{paths, match_def_path};
|
||||||
|
use rustc_span::symbol::sym;
|
||||||
|
use rustc_hir::LangItem;
|
||||||
|
|
||||||
impl LateLintPass<'_> for MyStructLint {
|
impl LateLintPass<'_> for MyStructLint {
|
||||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
||||||
// 1. Using expression and Clippy's convenient method
|
// Getting the expression type
|
||||||
// we use `match_trait_method` function from Clippy's toolbox
|
let ty = cx.typeck_results().expr_ty(expr);
|
||||||
if match_trait_method(cx, expr, &paths::INTO) {
|
|
||||||
// `expr` implements `Into` trait
|
// 1. Using diagnostic items
|
||||||
|
// The last argument is the diagnostic item to check for
|
||||||
|
if is_type_diagnostic_item(cx, ty, sym::Option) {
|
||||||
|
// The type is an `Option`
|
||||||
}
|
}
|
||||||
|
|
||||||
// 2. Using type context `TyCtxt`
|
// 2. Using lang items
|
||||||
|
if is_type_lang_item(cx, ty, LangItem::RangeFull) {
|
||||||
|
// The type is a full range like `.drain(..)`
|
||||||
|
}
|
||||||
|
|
||||||
|
// 3. Using the type path
|
||||||
|
// This method should be avoided if possible
|
||||||
|
if match_def_path(cx, def_id, &paths::RESULT) {
|
||||||
|
// The type is a `core::result::Result`
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Prefer using diagnostic items and lang items where possible.
|
||||||
|
|
||||||
|
## Checking if a type implements a specific trait
|
||||||
|
|
||||||
|
There are three ways to do this, depending on if the target trait has a diagnostic item, lang item or neither.
|
||||||
|
|
||||||
|
```rust
|
||||||
|
use clippy_utils::{implements_trait, is_trait_method, match_trait_method, paths};
|
||||||
|
use rustc_span::symbol::sym;
|
||||||
|
|
||||||
|
impl LateLintPass<'_> for MyStructLint {
|
||||||
|
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
||||||
|
// 1. Using diagnostic items with the expression
|
||||||
|
// we use `is_trait_method` function from Clippy's utils
|
||||||
|
if is_trait_method(cx, expr, sym::Iterator) {
|
||||||
|
// method call in `expr` belongs to `Iterator` trait
|
||||||
|
}
|
||||||
|
|
||||||
|
// 2. Using lang items with the expression type
|
||||||
let ty = cx.typeck_results().expr_ty(expr);
|
let ty = cx.typeck_results().expr_ty(expr);
|
||||||
if cx.tcx.lang_items()
|
if cx.tcx.lang_items()
|
||||||
// we are looking for the `DefId` of `Drop` trait in lang items
|
// we are looking for the `DefId` of `Drop` trait in lang items
|
||||||
|
@ -97,102 +138,125 @@ impl LateLintPass<'_> for MyStructLint {
|
||||||
.map_or(false, |id| implements_trait(cx, ty, id, &[])) {
|
.map_or(false, |id| implements_trait(cx, ty, id, &[])) {
|
||||||
// `expr` implements `Drop` trait
|
// `expr` implements `Drop` trait
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 3. Using the type path with the expression
|
||||||
|
// we use `match_trait_method` function from Clippy's utils
|
||||||
|
// (This method should be avoided if possible)
|
||||||
|
if match_trait_method(cx, expr, &paths::INTO) {
|
||||||
|
// `expr` implements `Into` trait
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
> Prefer using lang items, if the target trait is available there.
|
> Prefer using diagnostic and lang items, if the target trait has one.
|
||||||
|
|
||||||
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.
|
We access lang items through the type context `tcx`. `tcx` is of type [`TyCtxt`][TyCtxt] and is defined in the `rustc_middle` crate.
|
||||||
|
A list of defined paths for Clippy can be found in [paths.rs][paths]
|
||||||
|
|
||||||
# Checking if a type defines a specific method
|
## Checking if a type defines a specific method
|
||||||
|
|
||||||
To check if our type defines a method called `some_method`:
|
To check if our type defines a method called `some_method`:
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
use clippy_utils::{is_type_diagnostic_item, return_ty};
|
use clippy_utils::ty::is_type_diagnostic_item;
|
||||||
|
use clippy_utils::return_ty;
|
||||||
|
|
||||||
impl<'tcx> LateLintPass<'tcx> for MyTypeImpl {
|
impl<'tcx> LateLintPass<'tcx> for MyTypeImpl {
|
||||||
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
|
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
|
||||||
if_chain! {
|
// Check if item is a method/function
|
||||||
// Check if item is a method/function
|
if let ImplItemKind::Fn(ref signature, _) = impl_item.kind
|
||||||
if let ImplItemKind::Fn(ref signature, _) = impl_item.kind;
|
|
||||||
// Check the method is named `some_method`
|
// Check the method is named `some_method`
|
||||||
if impl_item.ident.name == sym!(some_method);
|
&& impl_item.ident.name == sym!(some_method)
|
||||||
// We can also check it has a parameter `self`
|
// We can also check it has a parameter `self`
|
||||||
if signature.decl.implicit_self.has_implicit_self();
|
&& signature.decl.implicit_self.has_implicit_self()
|
||||||
// We can go further and even check if its return type is `String`
|
// 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));
|
&& is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(string_type))
|
||||||
then {
|
{
|
||||||
// ...
|
// ...
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
# Dealing with macros
|
## Dealing with macros and expansions
|
||||||
|
|
||||||
There are several helpers in [`clippy_utils`][utils] to deal with macros:
|
Keep in mind that macros are already expanded and desugaring is already applied
|
||||||
|
to the code representation that you are working with in Clippy. This unfortunately causes a lot of
|
||||||
|
false positives because macro expansions are "invisible" unless you actively check for them.
|
||||||
|
Generally speaking, code with macro expansions should just be ignored by Clippy because that code can be
|
||||||
|
dynamic in ways that are difficult or impossible to see.
|
||||||
|
Use the following functions to deal with macros:
|
||||||
|
|
||||||
- `in_macro()`: detect if the given span is expanded by a macro
|
- `span.from_expansion()`: detects if a span is from macro expansion or desugaring.
|
||||||
|
Checking this is a common first step in a lint.
|
||||||
|
|
||||||
You may want to use this for example to not start linting in any macro.
|
```rust
|
||||||
|
if expr.span.from_expansion() {
|
||||||
|
// just forget it
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
```rust
|
- `span.ctxt()`: the span's context represents whether it is from expansion, and if so, which macro call expanded it.
|
||||||
macro_rules! foo {
|
It is sometimes useful to check if the context of two spans are equal.
|
||||||
($param:expr) => {
|
|
||||||
match $param {
|
|
||||||
"bar" => println!("whatever"),
|
|
||||||
_ => ()
|
|
||||||
}
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
foo!("bar");
|
```rust
|
||||||
|
// expands to `1 + 0`, but don't lint
|
||||||
|
1 + mac!()
|
||||||
|
```
|
||||||
|
```rust
|
||||||
|
if left.span.ctxt() != right.span.ctxt() {
|
||||||
|
// the coder most likely cannot modify this expression
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Note: Code that is not from expansion is in the "root" context. So any spans where `from_expansion` returns `true` can
|
||||||
|
be assumed to have the same context. And so just using `span.from_expansion()` is often good enough.
|
||||||
|
|
||||||
// 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
|
- `in_external_macro(span)`: detect if the given span is from a macro defined in a foreign crate.
|
||||||
|
If you want the lint to work with macro-generated code, this is the next line of defense to avoid macros
|
||||||
|
not defined in the current crate. It doesn't make sense to lint code that the coder can't change.
|
||||||
|
|
||||||
You may want to use it for example to not start linting in macros from other crates
|
You may want to use it for example to not start linting in macros from other crates
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
#[macro_use]
|
#[macro_use]
|
||||||
extern crate a_crate_with_macros;
|
extern crate a_crate_with_macros;
|
||||||
|
|
||||||
// `foo` is defined in `a_crate_with_macros`
|
// `foo` is defined in `a_crate_with_macros`
|
||||||
foo!("bar");
|
foo!("bar");
|
||||||
|
|
||||||
// if we lint the `match` of `foo` call and test its span
|
// if we lint the `match` of `foo` call and test its span
|
||||||
assert_eq!(in_external_macro(cx.sess(), match_span), true);
|
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
|
- `span.ctxt()`: the span's context represents whether it is from expansion, and if so, what expanded it
|
||||||
|
|
||||||
```rust
|
One thing `SpanContext` is useful for is to check if two spans are in the same context. For example,
|
||||||
macro_rules! m {
|
in `a == b`, `a` and `b` have the same context. In a `macro_rules!` with `a == $b`, `$b` is expanded to some
|
||||||
($a:expr, $b:expr) => {
|
expression with a different context from `a`.
|
||||||
if $a.is_some() {
|
|
||||||
$b;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
let x: Option<u32> = Some(42);
|
```rust
|
||||||
m!(x, x.unwrap());
|
macro_rules! m {
|
||||||
|
($a:expr, $b:expr) => {
|
||||||
|
if $a.is_some() {
|
||||||
|
$b;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// These spans are not from the same context
|
let x: Option<u32> = Some(42);
|
||||||
// x.is_some() is from inside the macro
|
m!(x, x.unwrap());
|
||||||
// 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
|
// 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!(x_is_some_span.ctxt(), x_unwrap_span.ctxt());
|
||||||
|
```
|
||||||
|
|
||||||
|
[Ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html
|
||||||
[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.TyKind.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
|
[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
|
[expr_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.expr_ty
|
||||||
|
@ -200,4 +264,3 @@ assert_eq!(differing_macro_contexts(x_is_some_span, x_unwrap_span), true);
|
||||||
[TyCtxt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.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
|
[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
|
[paths]: ../clippy_utils/src/paths.rs
|
||||||
[utils]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/lib.rs
|
|
||||||
|
|
|
@ -32,7 +32,7 @@ bullet points might be helpful:
|
||||||
need to check out the Rust release tag of the stable release.
|
need to check out the Rust release tag of the stable release.
|
||||||
[Link][rust_stable_tools]
|
[Link][rust_stable_tools]
|
||||||
|
|
||||||
Usually you want to wirte the changelog of the **upcoming stable release**. Make
|
Usually you want to write the changelog of the **upcoming stable release**. Make
|
||||||
sure though, that `beta` was already branched in the Rust repository.
|
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:
|
To find the commit hash, issue the following command when in a `rust-lang/rust` checkout:
|
||||||
|
|
|
@ -121,4 +121,25 @@ happened a stable backport, make sure to re-merge those changes just as with the
|
||||||
|
|
||||||
For this see the document on [how to update the changelog].
|
For this see the document on [how to update the changelog].
|
||||||
|
|
||||||
|
If you don't have time to do a complete changelog update right away, just update
|
||||||
|
the following parts:
|
||||||
|
|
||||||
|
- Remove the `(beta)` from the new stable version:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
## Rust 1.XX (beta) -> ## Rust 1.XX
|
||||||
|
```
|
||||||
|
|
||||||
|
- Update the release date line of the new stable version:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
Current beta, release 20YY-MM-DD -> Current stable, released 20YY-MM-DD
|
||||||
|
```
|
||||||
|
|
||||||
|
- Update the release date line of the previous stable version:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
Current stable, released 20YY-MM-DD -> Released 20YY-MM-DD
|
||||||
|
```
|
||||||
|
|
||||||
[how to update the changelog]: https://github.com/rust-lang/rust-clippy/blob/master/doc/changelog_update.md
|
[how to update the changelog]: https://github.com/rust-lang/rust-clippy/blob/master/doc/changelog_update.md
|
||||||
|
|
|
@ -1,697 +0,0 @@
|
||||||
# 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)
|
|
||||||
- [Cargo lints](#cargo-lints)
|
|
||||||
- [Rustfix tests](#rustfix-tests)
|
|
||||||
- [Edition 2018 tests](#edition-2018-tests)
|
|
||||||
- [Testing manually](#testing-manually)
|
|
||||||
- [Lint declaration](#lint-declaration)
|
|
||||||
- [Lint registration](#lint-registration)
|
|
||||||
- [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)
|
|
||||||
- [Print HIR lint](#print-hir-lint)
|
|
||||||
- [Documentation](#documentation)
|
|
||||||
- [Running rustfmt](#running-rustfmt)
|
|
||||||
- [Debugging](#debugging)
|
|
||||||
- [PR Checklist](#pr-checklist)
|
|
||||||
- [Adding configuration to a lint](#adding-configuration-to-a-lint)
|
|
||||||
- [Cheat Sheet](#cheat-sheet)
|
|
||||||
|
|
||||||
## 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
|
|
||||||
[registering the lint](#lint-registration). 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
|
|
||||||
|
|
||||||
```
|
|
||||||
cargo dev lint 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?
|
|
||||||
///
|
|
||||||
/// ### Example
|
|
||||||
/// ```rust
|
|
||||||
/// // example code
|
|
||||||
/// ```
|
|
||||||
#[clippy::version = "1.29.0"]
|
|
||||||
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`.
|
|
||||||
* The `#[clippy::version]` attribute will be rendered as part of the lint documentation.
|
|
||||||
The value should be set to the current Rust version that the lint is developed in,
|
|
||||||
it can be retrieved by running `rustc -vV` in the rust-clippy directory. The version
|
|
||||||
is listed under *release*. (Use the version without the `-nightly`) suffix.
|
|
||||||
* `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 {}
|
|
||||||
```
|
|
||||||
|
|
||||||
[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 registration
|
|
||||||
|
|
||||||
When using `cargo dev new_lint`, the lint is automatically registered and
|
|
||||||
nothing more has to be done.
|
|
||||||
|
|
||||||
When declaring a new lint by hand and `cargo dev update_lints` is used, the lint
|
|
||||||
pass may have to be registered manually in the `register_plugins` function in
|
|
||||||
`clippy_lints/src/lib.rs`:
|
|
||||||
|
|
||||||
```rust
|
|
||||||
store.register_early_pass(|| Box::new(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 update_lints` 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.
|
|
||||||
|
|
||||||
## 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<RustcVersion>,
|
|
||||||
}
|
|
||||||
|
|
||||||
impl ManualStrip {
|
|
||||||
#[must_use]
|
|
||||||
pub fn new(msrv: Option<RustcVersion>) -> 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, 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_NEWLY_ADDED_LINT>. The minimum rust version that the project supports
|
|
||||||
(msrv: Option<String> = 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
|
|
||||||
|
|
||||||
## Print HIR lint
|
|
||||||
|
|
||||||
To implement a lint, it's helpful to first understand the internal representation
|
|
||||||
that rustc uses. Clippy has the `#[clippy::dump]` attribute that prints the
|
|
||||||
[_High-Level Intermediate Representation (HIR)_] of the item, statement, or
|
|
||||||
expression that the attribute is attached to. To attach the attribute to expressions
|
|
||||||
you often need to enable `#![feature(stmt_expr_attributes)]`.
|
|
||||||
|
|
||||||
[Here][print_hir_example] you can find an example, just select _Tools_ and run _Clippy_.
|
|
||||||
|
|
||||||
[_High-Level Intermediate Representation (HIR)_]: https://rustc-dev-guide.rust-lang.org/hir.html
|
|
||||||
[print_hir_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=daf14db3a7f39ca467cd1b86c34b9afb
|
|
||||||
|
|
||||||
## 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.
|
|
||||||
///
|
|
||||||
/// ### Example
|
|
||||||
///
|
|
||||||
/// ```rust,ignore
|
|
||||||
/// // A short example of code that triggers the lint
|
|
||||||
/// ```
|
|
||||||
///
|
|
||||||
/// Use instead:
|
|
||||||
/// ```rust,ignore
|
|
||||||
/// // A short example of improved code that doesn't trigger the lint
|
|
||||||
/// ```
|
|
||||||
#[clippy::version = "1.29.0"]
|
|
||||||
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:
|
|
||||||
|
|
||||||
<!-- Sync this with `.github/PULL_REQUEST_TEMPLATE` -->
|
|
||||||
|
|
||||||
- \[ ] 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_lints::utils::conf](/clippy_lints/src/utils/conf.rs)
|
|
||||||
like this:
|
|
||||||
```rust
|
|
||||||
/// Lint: LINT_NAME.
|
|
||||||
///
|
|
||||||
/// <The configuration field doc comment>
|
|
||||||
(configuration_ident: Type = DefaultValue),
|
|
||||||
```
|
|
||||||
The doc comment is automatically added to the documentation of the listed lints. The default
|
|
||||||
value will be formatted using the `Debug` implementation of the type.
|
|
||||||
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.
|
|
||||||
|
|
||||||
## Cheat Sheet
|
|
||||||
|
|
||||||
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 ([`is_type_diagnostic_item`], [`implements_trait`], [`snippet`], etc)
|
|
||||||
* [Clippy diagnostics][diagnostics]
|
|
||||||
* [Let chains][let-chains]
|
|
||||||
* [`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://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/index.html
|
|
||||||
[`is_type_diagnostic_item`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.is_type_diagnostic_item.html
|
|
||||||
[`implements_trait`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/ty/fn.implements_trait.html
|
|
||||||
[`snippet`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/source/fn.snippet.html
|
|
||||||
[let-chains]: https://github.com/rust-lang/rust/pull/94927
|
|
||||||
[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
|
|
|
@ -1,71 +0,0 @@
|
||||||
# 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 <SHA> # `<SHA>` 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/<your-github-name>/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.
|
|
174
doc/basics.md
174
doc/basics.md
|
@ -1,174 +0,0 @@
|
||||||
# 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:<your-username>/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
|
|
||||||
# automatically formatting all code before each commit
|
|
||||||
cargo dev setup git-hook
|
|
||||||
# (experimental) Setup Clippy to work with IntelliJ-Rust
|
|
||||||
cargo dev setup intellij
|
|
||||||
```
|
|
||||||
More about intellij command usage and reasons [here](../CONTRIBUTING.md#intellij-rust)
|
|
||||||
|
|
||||||
## 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 <https://rustc-dev-guide.rust-lang.org/contributing.html#opening-a-pr>.
|
|
||||||
|
|
||||||
## 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 <filename>
|
|
||||||
```
|
|
||||||
|
|
||||||
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
|
|
|
@ -1,97 +0,0 @@
|
||||||
# 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 write 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
|
|
|
@ -1,266 +0,0 @@
|
||||||
# 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 expr is calling a specific method](#checking-if-an-expr-is-calling-a-specific-method)
|
|
||||||
- [Checking for a specific type](#checking-for-a-specific-type)
|
|
||||||
- [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-and-expansions)
|
|
||||||
|
|
||||||
Useful Rustc dev guide links:
|
|
||||||
- [Stages of compilation](https://rustc-dev-guide.rust-lang.org/compiler-src.html#the-main-stages-of-compilation)
|
|
||||||
- [Diagnostic items](https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-items.html)
|
|
||||||
- [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 [`Ty`][Ty].
|
|
||||||
|
|
||||||
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<'tcx> LateLintPass<'tcx> for MyStructLint {
|
|
||||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
|
|
||||||
// Check our expr is calling a method
|
|
||||||
if let hir::ExprKind::MethodCall(path, _, [_self_arg, ..]) = &expr.kind
|
|
||||||
// Check the name of this method is `some_method`
|
|
||||||
&& path.ident.name == sym!(some_method)
|
|
||||||
// Optionally, check the type of the self argument.
|
|
||||||
// - See "Checking for a specific type"
|
|
||||||
{
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## Checking for a specific type
|
|
||||||
|
|
||||||
There are three ways to check if an expression type is a specific type we want to check for.
|
|
||||||
All of these methods only check for the base type, generic arguments have to be checked separately.
|
|
||||||
|
|
||||||
```rust
|
|
||||||
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
|
|
||||||
use clippy_utils::{paths, match_def_path};
|
|
||||||
use rustc_span::symbol::sym;
|
|
||||||
use rustc_hir::LangItem;
|
|
||||||
|
|
||||||
impl LateLintPass<'_> for MyStructLint {
|
|
||||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
|
||||||
// Getting the expression type
|
|
||||||
let ty = cx.typeck_results().expr_ty(expr);
|
|
||||||
|
|
||||||
// 1. Using diagnostic items
|
|
||||||
// The last argument is the diagnostic item to check for
|
|
||||||
if is_type_diagnostic_item(cx, ty, sym::Option) {
|
|
||||||
// The type is an `Option`
|
|
||||||
}
|
|
||||||
|
|
||||||
// 2. Using lang items
|
|
||||||
if is_type_lang_item(cx, ty, LangItem::RangeFull) {
|
|
||||||
// The type is a full range like `.drain(..)`
|
|
||||||
}
|
|
||||||
|
|
||||||
// 3. Using the type path
|
|
||||||
// This method should be avoided if possible
|
|
||||||
if match_def_path(cx, def_id, &paths::RESULT) {
|
|
||||||
// The type is a `core::result::Result`
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Prefer using diagnostic items and lang items where possible.
|
|
||||||
|
|
||||||
## Checking if a type implements a specific trait
|
|
||||||
|
|
||||||
There are three ways to do this, depending on if the target trait has a diagnostic item, lang item or neither.
|
|
||||||
|
|
||||||
```rust
|
|
||||||
use clippy_utils::{implements_trait, is_trait_method, match_trait_method, paths};
|
|
||||||
use rustc_span::symbol::sym;
|
|
||||||
|
|
||||||
impl LateLintPass<'_> for MyStructLint {
|
|
||||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
|
||||||
// 1. Using diagnostic items with the expression
|
|
||||||
// we use `is_trait_method` function from Clippy's utils
|
|
||||||
if is_trait_method(cx, expr, sym::Iterator) {
|
|
||||||
// method call in `expr` belongs to `Iterator` trait
|
|
||||||
}
|
|
||||||
|
|
||||||
// 2. Using lang items with the expression type
|
|
||||||
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
|
|
||||||
}
|
|
||||||
|
|
||||||
// 3. Using the type path with the expression
|
|
||||||
// we use `match_trait_method` function from Clippy's utils
|
|
||||||
// (This method should be avoided if possible)
|
|
||||||
if match_trait_method(cx, expr, &paths::INTO) {
|
|
||||||
// `expr` implements `Into` trait
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
> Prefer using diagnostic and lang items, if the target trait has one.
|
|
||||||
|
|
||||||
We access lang items through the type context `tcx`. `tcx` is of type [`TyCtxt`][TyCtxt] and is defined in the `rustc_middle` crate.
|
|
||||||
A list of defined paths for Clippy can be found in [paths.rs][paths]
|
|
||||||
|
|
||||||
## Checking if a type defines a specific method
|
|
||||||
|
|
||||||
To check if our type defines a method called `some_method`:
|
|
||||||
|
|
||||||
```rust
|
|
||||||
use clippy_utils::ty::is_type_diagnostic_item;
|
|
||||||
use clippy_utils::return_ty;
|
|
||||||
|
|
||||||
impl<'tcx> LateLintPass<'tcx> for MyTypeImpl {
|
|
||||||
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
|
|
||||||
// Check if item is a method/function
|
|
||||||
if let ImplItemKind::Fn(ref signature, _) = impl_item.kind
|
|
||||||
// Check the method is named `some_method`
|
|
||||||
&& impl_item.ident.name == sym!(some_method)
|
|
||||||
// We can also check it has a parameter `self`
|
|
||||||
&& signature.decl.implicit_self.has_implicit_self()
|
|
||||||
// We can go further and even check if its return type is `String`
|
|
||||||
&& is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(string_type))
|
|
||||||
{
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## Dealing with macros and expansions
|
|
||||||
|
|
||||||
Keep in mind that macros are already expanded and desugaring is already applied
|
|
||||||
to the code representation that you are working with in Clippy. This unfortunately causes a lot of
|
|
||||||
false positives because macro expansions are "invisible" unless you actively check for them.
|
|
||||||
Generally speaking, code with macro expansions should just be ignored by Clippy because that code can be
|
|
||||||
dynamic in ways that are difficult or impossible to see.
|
|
||||||
Use the following functions to deal with macros:
|
|
||||||
|
|
||||||
- `span.from_expansion()`: detects if a span is from macro expansion or desugaring.
|
|
||||||
Checking this is a common first step in a lint.
|
|
||||||
|
|
||||||
```rust
|
|
||||||
if expr.span.from_expansion() {
|
|
||||||
// just forget it
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
- `span.ctxt()`: the span's context represents whether it is from expansion, and if so, which macro call expanded it.
|
|
||||||
It is sometimes useful to check if the context of two spans are equal.
|
|
||||||
|
|
||||||
```rust
|
|
||||||
// expands to `1 + 0`, but don't lint
|
|
||||||
1 + mac!()
|
|
||||||
```
|
|
||||||
```rust
|
|
||||||
if left.span.ctxt() != right.span.ctxt() {
|
|
||||||
// the coder most likely cannot modify this expression
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
```
|
|
||||||
Note: Code that is not from expansion is in the "root" context. So any spans where `from_expansion` returns `true` can
|
|
||||||
be assumed to have the same context. And so just using `span.from_expansion()` is often good enough.
|
|
||||||
|
|
||||||
|
|
||||||
- `in_external_macro(span)`: detect if the given span is from a macro defined in a foreign crate.
|
|
||||||
If you want the lint to work with macro-generated code, this is the next line of defense to avoid macros
|
|
||||||
not defined in the current crate. It doesn't make sense to lint code that the coder can't change.
|
|
||||||
|
|
||||||
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);
|
|
||||||
```
|
|
||||||
|
|
||||||
- `span.ctxt()`: the span's context represents whether it is from expansion, and if so, what expanded it
|
|
||||||
|
|
||||||
One thing `SpanContext` is useful for is to check if two spans are in the same context. For example,
|
|
||||||
in `a == b`, `a` and `b` have the same context. In a `macro_rules!` with `a == $b`, `$b` is expanded to some
|
|
||||||
expression with a different context from `a`.
|
|
||||||
|
|
||||||
```rust
|
|
||||||
macro_rules! m {
|
|
||||||
($a:expr, $b:expr) => {
|
|
||||||
if $a.is_some() {
|
|
||||||
$b;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
let x: Option<u32> = 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!(x_is_some_span.ctxt(), x_unwrap_span.ctxt());
|
|
||||||
```
|
|
||||||
|
|
||||||
[Ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.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
|
|
145
doc/release.md
145
doc/release.md
|
@ -1,145 +0,0 @@
|
||||||
# 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].
|
|
||||||
|
|
||||||
If you don't have time to do a complete changelog update right away, just update
|
|
||||||
the following parts:
|
|
||||||
|
|
||||||
- Remove the `(beta)` from the new stable version:
|
|
||||||
|
|
||||||
```markdown
|
|
||||||
## Rust 1.XX (beta) -> ## Rust 1.XX
|
|
||||||
```
|
|
||||||
|
|
||||||
- Update the release date line of the new stable version:
|
|
||||||
|
|
||||||
```markdown
|
|
||||||
Current beta, release 20YY-MM-DD -> Current stable, released 20YY-MM-DD
|
|
||||||
```
|
|
||||||
|
|
||||||
- Update the release date line of the previous stable version:
|
|
||||||
|
|
||||||
```markdown
|
|
||||||
Current stable, released 20YY-MM-DD -> Released 20YY-MM-DD
|
|
||||||
```
|
|
||||||
|
|
||||||
[how to update the changelog]: https://github.com/rust-lang/rust-clippy/blob/master/doc/changelog_update.md
|
|
|
@ -1,235 +0,0 @@
|
||||||
# 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.
|
|
Loading…
Reference in a new issue