From 3c50a0a18d322a31ce62a1e7c5b02c8144dff442 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Mon, 10 Apr 2023 18:30:28 +0200 Subject: [PATCH 1/5] Add new chapter: "Trait Checking" --- book/src/SUMMARY.md | 1 + book/src/development/trait_checking.md | 112 +++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 book/src/development/trait_checking.md diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index daaefd06a97..1f93623960e 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -16,6 +16,7 @@ - [Defining Lints](development/defining_lints.md) - [Lint Passes](development/lint_passes.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) diff --git a/book/src/development/trait_checking.md b/book/src/development/trait_checking.md new file mode 100644 index 00000000000..b8c13df5073 --- /dev/null +++ b/book/src/development/trait_checking.md @@ -0,0 +1,112 @@ +# 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]. + +While the Rust Compiler Development Guide has [a section][using_diagnostic_items] on +how to check for a specific trait on a type `Ty`, Clippy provides +a helper function `is_trait_method`, which simplifies the process for us. + +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<'_>) { + if is_trait_method(cx, expr, sym::Iterator) { + println!("This expression implements `Iterator` trait!"); + } + } +} +``` + +> **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 and we find that `LanguageItems` contains +all language items both from the current crate or its +dependencies. + +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 or lang 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. + +Below, we check if the given `expr` implements `tokio`'s +[`AsyncReadExt`][AsyncReadExt] trait: + +```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::TOKIO_IO_ASYNCREADEXT) { + println!("`expr` implements `TOKIO_IO_ASYNCREADEXT` trait!"); + } + } +} +``` + +> **Note**: Even though all the `clippy_utils` methods we have seen in this +> chapter takes `expr` as a parameter, these methods are actually using +> each expression's `HirId` under the hood. + +[AsyncReadExt]: https://docs.rs/tokio/latest/tokio/io/trait.AsyncReadExt.html +[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 +[using_diagnostic_items]: https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-items.html#using-diagnostic-items From 94387efc83f15460fefcbb5c7108f188b2097769 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Tue, 18 Apr 2023 20:50:41 +0200 Subject: [PATCH 2/5] Fixes based on reviews --- book/src/development/trait_checking.md | 32 +++++++++++--------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/book/src/development/trait_checking.md b/book/src/development/trait_checking.md index b8c13df5073..089f20b6ce0 100644 --- a/book/src/development/trait_checking.md +++ b/book/src/development/trait_checking.md @@ -11,10 +11,6 @@ that we want to examine has a [diagnostic item][diagnostic_items], As explained in the [Rust Compiler Development Guide][rustc_dev_guide], diagnostic items are introduced for identifying types via [Symbols][symbol]. -While the Rust Compiler Development Guide has [a section][using_diagnostic_items] on -how to check for a specific trait on a type `Ty`, Clippy provides -a helper function `is_trait_method`, which simplifies the process for us. - 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 @@ -28,9 +24,13 @@ use rustc_span::symbol::sym; impl LateLintPass<'_> for CheckIteratorTraitLint { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if is_trait_method(cx, expr, sym::Iterator) { - println!("This expression implements `Iterator` trait!"); - } + 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 { + // [...] + } + } } ``` @@ -40,9 +40,8 @@ impl LateLintPass<'_> for CheckIteratorTraitLint { ## Using Lang Items Besides diagnostic items, we can also use [`lang_items`][lang_items]. -Take a look at the documentation and we find that `LanguageItems` contains -all language items both from the current crate or its -dependencies. +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 @@ -73,11 +72,11 @@ impl LateLintPass<'_> for CheckDropTraitLint { ## Using Type Path -If neither diagnostic item or lang item is available, we can use +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. +> **Note**: This approach should be avoided if possible, the best thing to do would be to make a PR to [`rust-lang/rust`][rust]. Below, we check if the given `expr` implements `tokio`'s [`AsyncReadExt`][AsyncReadExt] trait: @@ -89,17 +88,13 @@ 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::TOKIO_IO_ASYNCREADEXT) { - println!("`expr` implements `TOKIO_IO_ASYNCREADEXT` trait!"); + if match_trait_method(cx, expr, &paths::CORE_ITER_CLONED) { + println!("`expr` implements `CORE_ITER_CLONED` trait!"); } } } ``` -> **Note**: Even though all the `clippy_utils` methods we have seen in this -> chapter takes `expr` as a parameter, these methods are actually using -> each expression's `HirId` under the hood. - [AsyncReadExt]: https://docs.rs/tokio/latest/tokio/io/trait.AsyncReadExt.html [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 @@ -110,3 +105,4 @@ impl LateLintPass<'_> for CheckTokioAsyncReadExtTrait { [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 [using_diagnostic_items]: https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-items.html#using-diagnostic-items +[rust]: https://github.com/rust-lang/rust \ No newline at end of file From b2e1ede39cbab5b3cf114e41768a27fba729ff69 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Wed, 19 Apr 2023 21:22:13 +0200 Subject: [PATCH 3/5] Change Tokio example description to Core --- book/src/development/trait_checking.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/book/src/development/trait_checking.md b/book/src/development/trait_checking.md index 089f20b6ce0..1c6f70cc059 100644 --- a/book/src/development/trait_checking.md +++ b/book/src/development/trait_checking.md @@ -78,8 +78,7 @@ 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]. -Below, we check if the given `expr` implements `tokio`'s -[`AsyncReadExt`][AsyncReadExt] trait: +Below, we check if the given `expr` implements the `Iterator`'s trait method `cloned` : ```rust use clippy_utils::{match_trait_method, paths}; @@ -95,7 +94,6 @@ impl LateLintPass<'_> for CheckTokioAsyncReadExtTrait { } ``` -[AsyncReadExt]: https://docs.rs/tokio/latest/tokio/io/trait.AsyncReadExt.html [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 @@ -105,4 +103,4 @@ impl LateLintPass<'_> for CheckTokioAsyncReadExtTrait { [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 [using_diagnostic_items]: https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-items.html#using-diagnostic-items -[rust]: https://github.com/rust-lang/rust \ No newline at end of file +[rust]: https://github.com/rust-lang/rust From 2a3f75b0b8405428fa7ac0985a96b2866fc3de17 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Wed, 19 Apr 2023 21:47:35 +0200 Subject: [PATCH 4/5] Fix CI --- book/src/development/trait_checking.md | 1 - 1 file changed, 1 deletion(-) diff --git a/book/src/development/trait_checking.md b/book/src/development/trait_checking.md index 1c6f70cc059..cee4b0df498 100644 --- a/book/src/development/trait_checking.md +++ b/book/src/development/trait_checking.md @@ -102,5 +102,4 @@ impl LateLintPass<'_> for CheckTokioAsyncReadExtTrait { [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 -[using_diagnostic_items]: https://rustc-dev-guide.rust-lang.org/diagnostics/diagnostic-items.html#using-diagnostic-items [rust]: https://github.com/rust-lang/rust From e1a3f635fcdbac410449c869416af9ebd3fee2f5 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Sat, 2 Sep 2023 11:23:27 +0200 Subject: [PATCH 5/5] Apply suggestion --- book/src/development/trait_checking.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/book/src/development/trait_checking.md b/book/src/development/trait_checking.md index cee4b0df498..fb263922cf8 100644 --- a/book/src/development/trait_checking.md +++ b/book/src/development/trait_checking.md @@ -76,7 +76,7 @@ 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]. +> **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` :