diff options
| author | Philipp Krones <hello@philkrones.com> | 2023-08-24 21:04:57 +0200 |
|---|---|---|
| committer | Philipp Krones <hello@philkrones.com> | 2023-08-24 21:04:57 +0200 |
| commit | ef4d64f1bd3f2a5693f062136f8411ac641bece8 (patch) | |
| tree | 386b8d0068d4b23d23f2a0ba5f8ee052dfc566a0 /book/src/development | |
| parent | 32eecd4b884dd2901aad05d66d78ea643a896e25 (diff) | |
| parent | df68b713270775364dc286eaec1c48fd14ae50e4 (diff) | |
Merge remote-tracking branch 'upstream/master' into rustup
Diffstat (limited to 'book/src/development')
| -rw-r--r-- | book/src/development/adding_lints.md | 4 | ||||
| -rw-r--r-- | book/src/development/defining_lints.md | 205 | ||||
| -rw-r--r-- | book/src/development/method_checking.md | 93 | ||||
| -rw-r--r-- | book/src/development/speedtest.md | 8 |
4 files changed, 302 insertions, 8 deletions
diff --git a/book/src/development/adding_lints.md b/book/src/development/adding_lints.md index a0db8089250..f6f0c95c729 100644 --- a/book/src/development/adding_lints.md +++ b/book/src/development/adding_lints.md @@ -161,8 +161,8 @@ The process of generating the `.stderr` file is the same, and prepending the ## 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 +will create a `.fixed` file by running [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. diff --git a/book/src/development/defining_lints.md b/book/src/development/defining_lints.md new file mode 100644 index 00000000000..7c4aa5d4523 --- /dev/null +++ b/book/src/development/defining_lints.md @@ -0,0 +1,205 @@ +# Define New Lints + +The first step in the journey of a new lint is the definition +and registration of the lint in Clippy's codebase. +We can use the Clippy dev tools to handle this step since setting up the +lint involves some boilerplate code. + +#### Lint types + +A lint type is the category of items and expressions in which your lint focuses on. + +As of the writing of this documentation update, there are 12 _types_ of lints +besides the numerous standalone lints living under `clippy_lints/src/`: + +- `cargo` +- `casts` +- `functions` +- `loops` +- `matches` +- `methods` +- `misc_early` +- `operators` +- `transmute` +- `types` +- `unit_types` +- `utils / internal` (Clippy internal lints) + +These types group together lints that share some common behaviors. For instance, +`functions` groups together lints that deal with some aspects of functions in +Rust, like definitions, signatures and attributes. + +For more information, feel free to compare the lint files under any category +with [All Clippy lints][all_lints] or ask one of the maintainers. + +## Lint name + +A good lint name is important, make sure to check the [lint naming +guidelines][lint_naming]. Don't worry, if the lint name doesn't fit, a Clippy +team member will alert you in the PR process. + +--- + +We'll name our example lint that detects functions named "foo" `foo_functions`. +Check the [lint naming guidelines][lint_naming] to see why this name makes +sense. + +## Add and Register the Lint + +Now that a name is chosen, we shall register `foo_functions` as a lint to the +codebase. There are two ways to register a lint. + +### Standalone + +If you believe that this new lint is a standalone lint (that doesn't belong to +any specific [type](#lint-types) like `functions` or `loops`), you can run the +following command in your Clippy project: + +```sh +$ cargo dev new_lint --name=lint_name --pass=late --category=pedantic +``` + +There are two things to note here: + +1. `--pass`: We set `--pass=late` in this command to do a late lint pass. The + alternative is an `early` lint pass. We will discuss this difference in a + later chapter. + <!-- FIXME: Link that "later chapter" when lint_passes.md is merged --> +2. `--category`: If not provided, the `category` of this new lint will default + to `nursery`. + +The `cargo dev new_lint` command will create a new file: +`clippy_lints/src/foo_functions.rs` as well as [register the +lint](#lint-registration). + +Overall, you should notice that the following files are modified or created: + +```sh +$ git status +On branch foo_functions +Changes not staged for commit: + (use "git add <file>..." to update what will be committed) + (use "git restore <file>..." to discard changes in working directory) + modified: CHANGELOG.md + modified: clippy_lints/src/lib.register_lints.rs + modified: clippy_lints/src/lib.register_pedantic.rs + modified: clippy_lints/src/lib.rs + +Untracked files: + (use "git add <file>..." to include in what will be committed) + clippy_lints/src/foo_functions.rs + tests/ui/foo_functions.rs +``` + + +### Specific Type + +> **Note**: Lint types are listed in the ["Lint types"](#lint-types) section + +If you believe that this new lint belongs to a specific type of lints, +you can run `cargo dev new_lint` with a `--type` option. + +Since our `foo_functions` lint is related to function calls, one could +argue that we should put it into a group of lints that detect some behaviors +of functions, we can put it in the `functions` group. + +Let's run the following command in your Clippy project: + +```sh +$ cargo dev new_lint --name=foo_functions --type=functions --category=pedantic +``` + +This command will create, among other things, a new file: +`clippy_lints/src/{type}/foo_functions.rs`. +In our case, the path will be `clippy_lints/src/functions/foo_functions.rs`. + +Notice how this command has a `--type` flag instead of `--pass`. Unlike a standalone +definition, this lint won't be registered in the traditional sense. Instead, you will +call your lint from within the type's lint pass, found in `clippy_lints/src/{type}/mod.rs`. + +A _type_ is just the name of a directory in `clippy_lints/src`, like `functions` in +the example command. Clippy groups together some lints that share common behaviors, +so if your lint falls into one, it would be best to add it to that type. + +Overall, you should notice that the following files are modified or created: + +```sh +$ git status +On branch foo_functions +Changes not staged for commit: + (use "git add <file>..." to update what will be committed) + (use "git restore <file>..." to discard changes in working directory) + modified: CHANGELOG.md + modified: clippy_lints/src/declared_lints.rs + modified: clippy_lints/src/functions/mod.rs + +Untracked files: + (use "git add <file>..." to include in what will be committed) + clippy_lints/src/functions/foo_functions.rs + tests/ui/foo_functions.rs +``` + + +## The `define_clippy_lints` macro + +After `cargo dev new_lint`, you should see a macro with the name +`define_clippy_lints`. It will be in the same file if you defined a standalone +lint, and it will be in `mod.rs` if you defined a type-specific lint. + +The macro looks something like this: + +```rust +declare_clippy_lint! { + /// ### What it does + /// + /// // Describe here what does the lint do. + /// + /// Triggers when detects... + /// + /// ### Why is this bad? + /// + /// // Describe why this pattern would be bad + /// + /// It can lead to... + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "1.70.0"] // <- In which version was this implemented, keep it up to date! + pub LINT_NAME, // <- The lint name IN_ALL_CAPS + pedantic, // <- The lint group + "default lint description" // <- A lint description, e.g. "A function has an unit return type." +} +``` + +## Lint registration + +If we run the `cargo dev new_lint` command for a new lint, the lint will be +automatically registered and there is nothing more to do. + +However, sometimes we might want to declare a new lint by hand. In this case, +we'd use `cargo dev update_lints` command afterwards. + +When a lint is manually declared, we might need to register the lint pass +manually in the `register_plugins` function in `clippy_lints/src/lib.rs`: + +```rust +store.register_late_pass(|_| Box::new(foo_functions::FooFunctions)); +``` + +As you might have guessed, where there's something late, there is something +early: in Clippy there is a `register_early_pass` method as well. More on early +vs. late passes in a later chapter. +<!-- FIXME: Link that "later chapter" when lint_passes.md is merged --> + +Without a call to one of `register_early_pass` or `register_late_pass`, the lint +pass in question will not be run. + + +[all_lints]: https://rust-lang.github.io/rust-clippy/master/ +[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints diff --git a/book/src/development/method_checking.md b/book/src/development/method_checking.md new file mode 100644 index 00000000000..56d1be37519 --- /dev/null +++ b/book/src/development/method_checking.md @@ -0,0 +1,93 @@ +# Method Checking + +In some scenarios we might want to check for methods when developing +a lint. There are two kinds of questions that we might be curious about: + +- Invocation: Does an expression call a specific method? +- Definition: Does an `impl` define a method? + +## Checking if an `expr` is calling a specific method + +Suppose we have an `expr`, we can check whether it calls a specific +method, e.g. `our_fancy_method`, by performing a pattern match on +the [`ExprKind`] that we can access from `expr.kind`: + +```rust +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::sym; +use clippy_utils::is_trait_method; + +impl<'tcx> LateLintPass<'tcx> for OurFancyMethodLint { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + // Check our expr is calling a method with pattern matching + if let hir::ExprKind::MethodCall(path, _, [self_arg, ..]) = &expr.kind + // Check if the name of this method is `our_fancy_method` + && path.ident.name == sym!(our_fancy_method) + // We can check the type of the self argument whenever necessary. + // (It's necessary if we want to check that method is specifically belonging to a specific trait, + // for example, a `map` method could belong to user-defined trait instead of to `Iterator`) + // See the next section for more information. + && is_trait_method(cx, self_arg, sym::OurFancyTrait) + { + println!("`expr` is a method call for `our_fancy_method`"); + } + } +} +``` + +Take a closer look at the `ExprKind` enum variant [`MethodCall`] for more +information on the pattern matching. As mentioned in [Define +Lints](defining_lints.md#lint-types), the `methods` lint type is full of pattern +matching with `MethodCall` in case the reader wishes to explore more. + +Additionally, we use the [`clippy_utils::sym!`][sym] macro to conveniently +convert an input `our_fancy_method` into a `Symbol` and compare that symbol to +the [`Ident`]'s name in the [`PathSegment`] in the [`MethodCall`]. + +## Checking if a `impl` block implements a method + +While sometimes we want to check whether a method is being called or not, other +times we want to know if our `Ty` defines a method. + +To check if our `impl` block defines a method `our_fancy_method`, we will +utilize the [`check_impl_item`] method that is available in our beloved +[`LateLintPass`] (for more information, refer to the ["Lint +Passes"](lint_passes.md) chapter in the Clippy book). This method provides us +with an [`ImplItem`] struct, which represents anything within an `impl` block. + +Let us take a look at how we might check for the implementation of +`our_fancy_method` on a type: + +```rust +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::return_ty; +use rustc_hir::{ImplItem, ImplItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::symbol::sym; + +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 `our_fancy_method` + && impl_item.ident.name == sym!(our_fancy_method) + // We can also check it has a parameter `self` + && signature.decl.implicit_self.has_implicit_self() + // We can go even further and even check if its return type is `String` + && is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym::String) + { + println!("`our_fancy_method` is implemented!"); + } + } +} +``` + +[`check_impl_item`]: https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/trait.LateLintPass.html#method.check_impl_item +[`ExprKind`]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_hir/hir/enum.ExprKind.html +[`Ident`]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_span/symbol/struct.Ident.html +[`ImplItem`]: https://doc.rust-lang.org/stable/nightly-rustc/rustc_hir/hir/struct.ImplItem.html +[`LateLintPass`]: https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/trait.LateLintPass.html +[`MethodCall`]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_hir/hir/enum.ExprKind.html#variant.MethodCall +[`PathSegment`]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_hir/hir/struct.PathSegment.html +[sym]: https://doc.rust-lang.org/stable/nightly-rustc/clippy_utils/macro.sym.html diff --git a/book/src/development/speedtest.md b/book/src/development/speedtest.md index 0db718e6ad6..4ea1c8e5c9c 100644 --- a/book/src/development/speedtest.md +++ b/book/src/development/speedtest.md @@ -9,16 +9,12 @@ accessed by the `SPEEDTEST` (and `SPEEDTEST_*`) environment variables. To do a simple speed test of a lint (e.g. `allow_attributes`), use this command. ```sh -$ SPEEDTEST=ui TESTNAME="allow_attributes" cargo uitest -- --nocapture +$ SPEEDTEST=ui TESTNAME="allow_attributes" cargo uitest ``` This will test all `ui` tests (`SPEEDTEST=ui`) whose names start with `allow_attributes`. By default, `SPEEDTEST` will iterate your test 1000 times. But you can change this with `SPEEDTEST_ITERATIONS`. ```sh -$ SPEEDTEST=toml SPEEDTEST_ITERATIONS=100 TESTNAME="semicolon_block" cargo uitest -- --nocapture +$ SPEEDTEST=toml SPEEDTEST_ITERATIONS=100 TESTNAME="semicolon_block" cargo uitest ``` - -> **WARNING**: Be sure to use `-- --nocapture` at the end of the command to see the average test time. If you don't -> use `-- --nocapture` (e.g. `SPEEDTEST=ui` `TESTNAME="let_underscore_untyped" cargo uitest -- --nocapture`), this -> will not show up. |
