diff options
Diffstat (limited to 'src/tools/clippy/book')
| -rw-r--r-- | src/tools/clippy/book/src/SUMMARY.md | 5 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/development/adding_lints.md | 4 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/development/defining_lints.md | 205 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/development/emitting_lints.md | 217 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/development/method_checking.md | 93 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/development/speedtest.md | 8 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/development/trait_checking.md | 105 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/development/writing_tests.md | 218 | ||||
| -rw-r--r-- | src/tools/clippy/book/src/lint_configuration.md | 38 |
9 files changed, 883 insertions, 10 deletions
diff --git a/src/tools/clippy/book/src/SUMMARY.md b/src/tools/clippy/book/src/SUMMARY.md index 22fbdce75..b02457307 100644 --- a/src/tools/clippy/book/src/SUMMARY.md +++ b/src/tools/clippy/book/src/SUMMARY.md @@ -13,8 +13,13 @@ - [Development](development/README.md) - [Basics](development/basics.md) - [Adding Lints](development/adding_lints.md) + - [Defining Lints](development/defining_lints.md) + - [Writing tests](development/writing_tests.md) - [Lint Passes](development/lint_passes.md) + - [Emitting lints](development/emitting_lints.md) - [Type Checking](development/type_checking.md) + - [Trait Checking](development/trait_checking.md) + - [Method Checking](development/method_checking.md) - [Macro Expansions](development/macro_expansions.md) - [Common Tools](development/common_tools_writing_lints.md) - [Infrastructure](development/infrastructure/README.md) diff --git a/src/tools/clippy/book/src/development/adding_lints.md b/src/tools/clippy/book/src/development/adding_lints.md index a0db80892..f6f0c95c7 100644 --- a/src/tools/clippy/book/src/development/adding_lints.md +++ b/src/tools/clippy/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/src/tools/clippy/book/src/development/defining_lints.md b/src/tools/clippy/book/src/development/defining_lints.md new file mode 100644 index 000000000..7c4aa5d45 --- /dev/null +++ b/src/tools/clippy/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/src/tools/clippy/book/src/development/emitting_lints.md b/src/tools/clippy/book/src/development/emitting_lints.md new file mode 100644 index 000000000..a12f6aa91 --- /dev/null +++ b/src/tools/clippy/book/src/development/emitting_lints.md @@ -0,0 +1,217 @@ +# Emitting a lint + +Once we have [defined a lint](defining_lints.md), written [UI +tests](writing_tests.md) and chosen [the lint pass](lint_passes.md) for the lint, +we can begin the implementation of the lint logic so that we can emit it and +gradually work towards a lint that behaves as expected. + +Note that we will not go into concrete implementation of a lint logic in this +chapter. We will go into details in later chapters as well as in two examples of +real Clippy lints. + +To emit a lint, we must implement a pass (see [Lint Passes](lint_passes.md)) for +the lint that we have declared. In this example we'll implement a "late" lint, +so take a look at the [LateLintPass][late_lint_pass] documentation, which +provides an abundance of methods that we can implement for our lint. + +```rust +pub trait LateLintPass<'tcx>: LintPass { + // Trait methods +} +``` + +By far the most common method used for Clippy lints is [`check_expr` +method][late_check_expr], this is because Rust is an expression language and, +more often than not, the lint we want to work on must examine expressions. + +> _Note:_ If you don't fully understand what expressions are in Rust, take a +> look at the official documentation on [expressions][rust_expressions] + +Other common ones include the [`check_fn` method][late_check_fn] and the +[`check_item` method][late_check_item]. + +### Emitting a lint + +Inside the trait method that we implement, we can write down the lint logic and +emit the lint with suggestions. + +Clippy's [diagnostics] provides quite a few diagnostic functions that we can use +to emit lints. Take a look at the documentation to pick one that suits your +lint's needs the best. Some common ones you will encounter in the Clippy +repository includes: + +- [`span_lint`]: Emits a lint without providing any other information +- [`span_lint_and_note`]: Emits a lint and adds a note +- [`span_lint_and_help`]: Emits a lint and provides a helpful message +- [`span_lint_and_sugg`]: Emits a lint and provides a suggestion to fix the code +- [`span_lint_and_then`]: Like `span_lint`, but allows for a lot of output + customization. + +```rust +impl<'tcx> LateLintPass<'tcx> for LintName { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // Imagine that `some_lint_expr_logic` checks for requirements for emitting the lint + if some_lint_expr_logic(expr) { + span_lint_and_help( + cx, // < The context + LINT_NAME, // < The name of the lint in ALL CAPS + expr.span, // < The span to lint + "message on why the lint is emitted", + None, // < An optional help span (to highlight something in the lint) + "message that provides a helpful suggestion", + ); + } + } +} +``` + +> Note: The message should be matter of fact and avoid capitalization and +> punctuation. If multiple sentences are needed, the messages should probably be +> split up into an error + a help / note / suggestion message. + +## Suggestions: Automatic fixes + +Some lints know what to change in order to fix the code. For example, the lint +[`range_plus_one`][range_plus_one] warns for ranges where the user wrote `x..y + +1` instead of using an [inclusive range][inclusive_range] (`x..=y`). The fix to +this code would be changing the `x..y + 1` expression to `x..=y`. **This is +where suggestions come in**. + +A suggestion is a change that the lint provides to fix the issue it is linting. +The output looks something like this (from the example earlier): + +```text +error: an inclusive range would be more readable + --> $DIR/range_plus_minus_one.rs:37:14 + | +LL | for _ in 1..1 + 1 {} + | ^^^^^^^^ help: use: `1..=1` +``` + +**Not all suggestions are always right**, some of them require human +supervision, that's why we have [Applicability][applicability]. + +Applicability indicates confidence in the correctness of the suggestion, some +are always right (`Applicability::MachineApplicable`), but we use +`Applicability::MaybeIncorrect` and others when talking about a suggestion that +may be incorrect. + +### Example + +The same lint `LINT_NAME` but that emits a suggestion would look something like this: + +```rust +impl<'tcx> LateLintPass<'tcx> for LintName { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // Imagine that `some_lint_expr_logic` checks for requirements for emitting the lint + if some_lint_expr_logic(expr) { + span_lint_and_sugg( // < Note this change + cx, + LINT_NAME, + span, + "message on why the lint is emitted", + "use", + format!("foo + {} * bar", snippet(cx, expr.span, "<default>")), // < Suggestion + Applicability::MachineApplicable, + ); + } + } +} +``` + +Suggestions generally use the [`format!`][format_macro] macro to interpolate the +old values with the new ones. To get code snippets, use one of the `snippet*` +functions from `clippy_utils::source`. + +## How to choose between notes, help messages and suggestions + +Notes are presented separately from the main lint message, they provide useful +information that the user needs to understand why the lint was activated. They +are the most helpful when attached to a span. + +Examples: + +### Notes + +```text +error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. + --> $DIR/drop_forget_ref.rs:10:5 + | +10 | forget(&SomeStruct); + | ^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::forget-ref` implied by `-D warnings` +note: argument has type &SomeStruct + --> $DIR/drop_forget_ref.rs:10:12 + | +10 | forget(&SomeStruct); + | ^^^^^^^^^^^ +``` + +### Help Messages + +Help messages are specifically to help the user. These are used in situation +where you can't provide a specific machine applicable suggestion. They can also +be attached to a span. + +Example: + +```text +error: constant division of 0.0 with 0.0 will always result in NaN + --> $DIR/zero_div_zero.rs:6:25 + | +6 | let other_f64_nan = 0.0f64 / 0.0; + | ^^^^^^^^^^^^ + | + = help: consider using `f64::NAN` if you would like a constant representing NaN +``` + +### Suggestions + +Suggestions are the most helpful, they are changes to the source code to fix the +error. The magic in suggestions is that tools like `rustfix` can detect them and +automatically fix your code. + +Example: + +```text +error: This `.fold` can be more succinctly expressed as `.any` +--> $DIR/methods.rs:390:13 + | +390 | let _ = (0..3).fold(false, |acc, x| acc || x > 2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.any(|x| x > 2)` + | +``` + +### Snippets + +Snippets are pieces of the source code (as a string), they are extracted +generally using the [`snippet`][snippet_fn] function. + +For example, if you want to know how an item looks (and you know the item's +span), you could use `snippet(cx, span, "..")`. + +## Final: Run UI Tests to Emit the Lint + +Now, if we run our [UI test](writing_tests.md), we should see that Clippy now +produces output that contains the lint message we designed. + +The next step is to implement the logic properly, which is a detail that we will +cover in the next chapters. + +[diagnostics]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/diagnostics/index.html +[late_check_expr]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html#method.check_expr +[late_check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html#method.check_fn +[late_check_item]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html#method.check_item +[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html +[rust_expressions]: https://doc.rust-lang.org/reference/expressions.html +[`span_lint`]: https://doc.rust-lang.org/beta/nightly-rustc/clippy_utils/diagnostics/fn.span_lint.html +[`span_lint_and_note`]: https://doc.rust-lang.org/beta/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_note.html +[`span_lint_and_help`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_help.html +[`span_lint_and_sugg`]: https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_sugg.html +[`span_lint_and_then`]: https://doc.rust-lang.org/beta/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_then.html +[range_plus_one]: https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one +[inclusive_range]: https://doc.rust-lang.org/std/ops/struct.RangeInclusive.html +[applicability]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_errors/enum.Applicability.html +[snippet_fn]: https://doc.rust-lang.org/beta/nightly-rustc/clippy_utils/source/fn.snippet.html +[format_macro]: https://doc.rust-lang.org/std/macro.format.html diff --git a/src/tools/clippy/book/src/development/method_checking.md b/src/tools/clippy/book/src/development/method_checking.md new file mode 100644 index 000000000..56d1be375 --- /dev/null +++ b/src/tools/clippy/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/src/tools/clippy/book/src/development/speedtest.md b/src/tools/clippy/book/src/development/speedtest.md index 0db718e6a..4ea1c8e5c 100644 --- a/src/tools/clippy/book/src/development/speedtest.md +++ b/src/tools/clippy/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. diff --git a/src/tools/clippy/book/src/development/trait_checking.md b/src/tools/clippy/book/src/development/trait_checking.md new file mode 100644 index 000000000..fb263922c --- /dev/null +++ b/src/tools/clippy/book/src/development/trait_checking.md @@ -0,0 +1,105 @@ +# Trait Checking + +Besides [type checking](type_checking.md), we might want to examine if +a specific type `Ty` implements certain trait when implementing a lint. +There are three approaches to achieve this, depending on if the target trait +that we want to examine has a [diagnostic item][diagnostic_items], +[lang item][lang_items], or neither. + +## Using Diagnostic Items + +As explained in the [Rust Compiler Development Guide][rustc_dev_guide], diagnostic items +are introduced for identifying types via [Symbols][symbol]. + +For instance, if we want to examine whether an expression implements +the `Iterator` trait, we could simply write the following code, +providing the `LateContext` (`cx`), our expression at hand, and +the symbol of the trait in question: + +```rust +use clippy_utils::is_trait_method; +use rustc_hir::Expr; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::symbol::sym; + +impl LateLintPass<'_> for CheckIteratorTraitLint { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + let implements_iterator = cx.tcx.get_diagnostic_item(sym::Iterator).map_or(false, |id| { + implements_trait(cx, cx.typeck_results().expr_ty(arg), id, &[]) + }); + if implements_iterator { + // [...] + } + + } +} +``` + +> **Note**: Refer to [this index][symbol_index] for all the defined `Symbol`s. + +## Using Lang Items + +Besides diagnostic items, we can also use [`lang_items`][lang_items]. +Take a look at the documentation to find that `LanguageItems` contains +all language items defined in the compiler. + +Using one of its `*_trait` method, we could obtain the [DefId] of any +specific item, such as `Clone`, `Copy`, `Drop`, `Eq`, which are familiar +to many Rustaceans. + +For instance, if we want to examine whether an expression `expr` implements +`Drop` trait, we could access `LanguageItems` via our `LateContext`'s +[TyCtxt], which provides a `lang_items` method that will return the id of +`Drop` trait to us. Then, by calling Clippy utils function `implements_trait` +we can check that the `Ty` of the `expr` implements the trait: + +```rust +use clippy_utils::implements_trait; +use rustc_hir::Expr; +use rustc_lint::{LateContext, LateLintPass}; + +impl LateLintPass<'_> for CheckDropTraitLint { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + let ty = cx.typeck_results().expr_ty(expr); + if cx.tcx.lang_items() + .drop_trait() + .map_or(false, |id| implements_trait(cx, ty, id, &[])) { + println!("`expr` implements `Drop` trait!"); + } + } +} +``` + +## Using Type Path + +If neither diagnostic item nor a language item is available, we can use +[`clippy_utils::paths`][paths] with the `match_trait_method` to determine trait +implementation. + +> **Note**: This approach should be avoided if possible, the best thing to do would be to make a PR to [`rust-lang/rust`][rust] adding a diagnostic item. + +Below, we check if the given `expr` implements the `Iterator`'s trait method `cloned` : + +```rust +use clippy_utils::{match_trait_method, paths}; +use rustc_hir::Expr; +use rustc_lint::{LateContext, LateLintPass}; + +impl LateLintPass<'_> for CheckTokioAsyncReadExtTrait { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if match_trait_method(cx, expr, &paths::CORE_ITER_CLONED) { + println!("`expr` implements `CORE_ITER_CLONED` trait!"); + } + } +} +``` + +[DefId]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/def_id/struct.DefId.html +[diagnostic_items]: https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-items.html +[lang_items]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/lang_items/struct.LanguageItems.html +[paths]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/paths.rs +[rustc_dev_guide]: https://rustc-dev-guide.rust-lang.org/ +[symbol]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/struct.Symbol.html +[symbol_index]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_span/symbol/sym/index.html +[TyCtxt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html +[rust]: https://github.com/rust-lang/rust diff --git a/src/tools/clippy/book/src/development/writing_tests.md b/src/tools/clippy/book/src/development/writing_tests.md new file mode 100644 index 000000000..8937e0d8e --- /dev/null +++ b/src/tools/clippy/book/src/development/writing_tests.md @@ -0,0 +1,218 @@ +# Testing + +Developing lints for Clippy is a Test-Driven Development (TDD) process because +our first task before implementing any logic for a new lint is to write some test cases. + +## Develop Lints with Tests + +When we develop Clippy, we enter a complex and chaotic realm full of +programmatic issues, stylistic errors, illogical code and non-adherence to convention. +Tests are the first layer of order we can leverage to define when and where +we want a new lint to trigger or not. + +Moreover, writing tests first help Clippy developers to find a balance for +the first iteration of and further enhancements for a lint. +With test cases on our side, we will not have to worry about over-engineering +a lint on its first version nor missing out some obvious edge cases of the lint. +This approach empowers us to iteratively enhance each lint. + +## Clippy UI Tests + +We use **UI tests** for testing in Clippy. These UI tests check that the output +of Clippy is exactly as we expect it to be. 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 +with the command [`cargo bless`](#cargo-bless) (seen later on). + +### Write Test Cases + +Let us now think about some tests for our imaginary `foo_functions` lint. We +start by opening the test file `tests/ui/foo_functions.rs` that was created by +`cargo dev new_lint`. + +Update the file with some examples to get started: + +```rust +#![warn(clippy::foo_functions)] // < Add this, so the lint is guaranteed to be enabled in this file + +// Impl methods +struct A; +impl A { + pub fn fo(&self) {} + pub fn foo(&self) {} //~ ERROR: function called "foo" + pub fn food(&self) {} +} + +// Default trait methods +trait B { + fn fo(&self) {} + fn foo(&self) {} //~ ERROR: function called "foo" + fn food(&self) {} +} + +// Plain functions +fn fo() {} +fn foo() {} //~ ERROR: function called "foo" +fn food() {} + +fn main() { + // We also don't want to lint method calls + foo(); + let a = A; + a.foo(); +} +``` + +Without actual lint logic to emit the lint when we see a `foo` function name, +this test will just pass, because no lint will be emitted. However, we can now +run the test with the following command: + +```sh +$ TESTNAME=foo_functions cargo uitest +``` + +Clippy will compile and it will conclude with an `ok` for the tests: + +``` +...Clippy warnings and test outputs... +test compile_test ... ok +test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.48s +``` + +This is normal. After all, we wrote a bunch of Rust code but we haven't really +implemented any logic for Clippy to detect `foo` functions and emit a lint. + +As we gradually implement our lint logic, we will keep running this UI test command. +Clippy will begin outputting information that allows us to check if the output is +turning into what we want it to be. + +### Example output + +As our `foo_functions` lint is tested, the output would look something like this: + +``` +failures: +---- compile_test stdout ---- +normalized stderr: +error: function called "foo" + --> $DIR/foo_functions.rs:6:12 + | +LL | pub fn foo(&self) {} + | ^^^ + | + = note: `-D clippy::foo-functions` implied by `-D warnings` +error: function called "foo" + --> $DIR/foo_functions.rs:13:8 + | +LL | fn foo(&self) {} + | ^^^ +error: function called "foo" + --> $DIR/foo_functions.rs:19:4 + | +LL | fn foo() {} + | ^^^ +error: aborting due to 3 previous errors +``` + +Note the *failures* label at the top of the fragment, we'll get rid of it +(saving this output) in the next section. + +> _Note:_ You can run multiple test files by specifying a comma separated list: +> `TESTNAME=foo_functions,bar_methods,baz_structs`. + +### `cargo bless` + +Once we are satisfied with the output, we need to run this command to +generate or update the `.stderr` file for our lint: + +```sh +$ TESTNAME=foo_functions cargo uibless +``` + +This writes the emitted lint suggestions and fixes to the `.stderr` file, with +the reason for the lint, suggested fixes, and line numbers, etc. + +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 bless` for the +specific lint you are creating/editing. + +> _Note:_ If the generated `.stderr`, and `.fixed` files are empty, +> they should be removed. + +## `toml` Tests + +Some lints can be configured through a `clippy.toml` file. Those configuration +values are tested in `tests/ui-toml`. + +To add a new test there, create a new directory and add the files: + +- `clippy.toml`: Put here the configuration value you want to test. +- `lint_name.rs`: A test file where you put the testing code, that should see a + different lint behavior according to the configuration set in the + `clippy.toml` file. + +The potential `.stderr` and `.fixed` files can again be generated with `cargo +bless`. + +## Cargo Lints + +The process of testing is different for Cargo lints in that now we are +interested in the `Cargo.toml` manifest file. In this case, we also need a +minimal crate associated with that manifest. Those tests are generated in +`tests/ui-cargo`. + +Imagine we have a new example lint that is named `foo_categories`, we can run: + +```sh +$ cargo dev new_lint --name=foo_categories --pass=late --category=cargo +``` + +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 as for other lints +and prepending the `TESTNAME` variable to `cargo uitest` works for Cargo lints too. + +## Rustfix Tests + +If the lint you are working on is making use of structured suggestions, +[`rustfix`] will apply the suggestions from the lint to the test file code and +compare that to the contents of a `.fixed` file. + +Structured suggestions tell a user how to fix or re-write certain code that has +been linted with [`span_lint_and_sugg`]. + +Should `span_lint_and_sugg` be used to generate a suggestion, but not all +suggestions lead to valid code, you can use the `//@no-rustfix` comment on top +of the test file, to not run `rustfix` on that file. + +We'll talk about suggestions more in depth in a [later chapter](emitting_lints.md). + +Use `cargo bless` to automatically generate the `.fixed` file after running +the tests. + +[`rustfix`]: https://github.com/rust-lang/rustfix +[`span_lint_and_sugg`]: https://doc.rust-lang.org/beta/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_sugg.html + +## 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 from the working copy root. + +```sh +$ cargo dev lint input.rs +``` diff --git a/src/tools/clippy/book/src/lint_configuration.md b/src/tools/clippy/book/src/lint_configuration.md index caaad6d11..b980083f1 100644 --- a/src/tools/clippy/book/src/lint_configuration.md +++ b/src/tools/clippy/book/src/lint_configuration.md @@ -703,7 +703,7 @@ Minimum chars an ident can have, anything below or equal to this will be linted. ## `accept-comment-above-statement` Whether to accept a safety comment to be placed above the statement containing the `unsafe` block -**Default Value:** `false` (`bool`) +**Default Value:** `true` (`bool`) --- **Affected lints:** @@ -713,7 +713,7 @@ Whether to accept a safety comment to be placed above the statement containing t ## `accept-comment-above-attributes` Whether to accept a safety comment to be placed above the attributes for the `unsafe` block -**Default Value:** `false` (`bool`) +**Default Value:** `true` (`bool`) --- **Affected lints:** @@ -751,3 +751,37 @@ Which crates to allow absolute paths from * [`absolute_paths`](https://rust-lang.github.io/rust-clippy/master/index.html#absolute_paths) +## `allowed-dotfiles` +Additional dotfiles (files or directories starting with a dot) to allow + +**Default Value:** `{}` (`rustc_data_structures::fx::FxHashSet<String>`) + +--- +**Affected lints:** +* [`path_ends_with_ext`](https://rust-lang.github.io/rust-clippy/master/index.html#path_ends_with_ext) + + +## `enforce-iter-loop-reborrow` +#### Example +``` +let mut vec = vec![1, 2, 3]; +let rmvec = &mut vec; +for _ in rmvec.iter() {} +for _ in rmvec.iter_mut() {} +``` + +Use instead: +``` +let mut vec = vec![1, 2, 3]; +let rmvec = &mut vec; +for _ in &*rmvec {} +for _ in &mut *rmvec {} +``` + +**Default Value:** `false` (`bool`) + +--- +**Affected lints:** +* [`explicit_iter_loop`](https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop) + + |