]> git.lizzy.rs Git - rust.git/blobdiff - doc/common_tools_writing_lints.md
Merge remote-tracking branch 'upstream/master' into rustup
[rust.git] / doc / common_tools_writing_lints.md
index 412ff99314d9969baa1997008b3d3d3ebbccd916..c7e51d53f511d9e91c044e0c985c9d42ab41b672 100644 (file)
@@ -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)
   - [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 defines a method](#checking-if-a-type-defines-a-method)
-  - [Dealing with macros](#dealing-with-macros)
+  - [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
+## 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:
 
@@ -23,7 +25,7 @@ Sometimes you may want to retrieve the type `Ty` of an expression `Expr`, for ex
 - is it a primitive type?
 - does it implement a trait?
 
-This operation is performed using the [`expr_ty()`][expr_ty] method from the [`TypeckTables`][TypeckTables] 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].
 
 Example of use:
@@ -31,7 +33,7 @@ Example of use:
 impl LateLintPass<'_> for MyStructLint {
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
         // Get type of `expr`
-        let ty = cx.tables().expr_ty(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!"),
@@ -41,17 +43,19 @@ impl LateLintPass<'_> for MyStructLint {
 }
 ```
 
-Similarly in [`TypeckTables`][TypeckTables] methods, you have the [`pat_ty()`][pat_ty] method
+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 `tables`,
-  allowing us to jump to type definitions and other compilation stages such as HIR.
-- `tables` is [`TypeckTables`][TypeckTables] and is created by type checking step,
-  it includes useful information such as types of expressions, ways to resolve methods and so on.
+- `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
+## 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`:
 
@@ -60,9 +64,11 @@ impl LateLintPass<'_> for MyStructLint {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
         if_chain! {
             // Check our expr is calling a method
-            if let hir::ExprKind::MethodCall(path, _, _args) = &expr.kind;
+            if let hir::ExprKind::MethodCall(path, _, [_self_arg, ..], _) = &expr.kind;
             // Check the name of this method is `some_method`
             if path.ident.name == sym!(some_method);
+            // Optionally, check the type of the self argument.
+            // - See "Checking for a specific type"
             then {
                 // ...
             }
@@ -71,23 +77,62 @@ impl LateLintPass<'_> for MyStructLint {
 }
 ```
 
-# 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
-use crate::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 {
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
-        // 1. Using expression and Clippy's convenient method
-        // we use `match_trait_method` function from Clippy's toolbox
-        if match_trait_method(cx, expr, &paths::INTO) {
-            // `expr` implements `Into` trait
+        // 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 type context `TyCtxt`
-        let ty = cx.tables().expr_ty(expr);
+        // 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()
@@ -95,22 +140,28 @@ impl LateLintPass<'_> for MyStructLint {
             .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 lang items, if the target trait is available there.
-
-A list of defined paths for Clippy can be found in [paths.rs][paths]
+> 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
+## Checking if a type defines a specific method
 
 To check if our type defines a method called `some_method`:
 
 ```rust
-use crate::utils::{is_type_diagnostic_item, return_ty};
+use clippy_utils::{is_type_diagnostic_item, return_ty};
 
 impl<'tcx> LateLintPass<'tcx> for MyTypeImpl {
     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
@@ -131,70 +182,84 @@ impl<'tcx> LateLintPass<'tcx> for MyTypeImpl {
 }
 ```
 
-# Dealing with macros
+## Dealing with macros and expansions
 
-There are several helpers in Clippy's 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
-macro_rules! foo {
-    ($param:expr) => {
-        match $param {
-            "bar" => println!("whatever"),
-            _ => ()
-        }
-    };
-}
+- `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.
 
-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
-#[macro_use]
-extern crate a_crate_with_macros;
+   ```rust
+   #[macro_use]
+   extern crate a_crate_with_macros;
 
-// `foo` is defined in `a_crate_with_macros`
-foo!("bar");
+   // `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);
-```
+   // if we lint the `match` of `foo` call and test its span
+   assert_eq!(in_external_macro(cx.sess(), match_span), true);
+   ```
 
 - `differing_macro_contexts()`: returns true if the two given spans are not from the same context
 
-```rust
-macro_rules! m {
-    ($a:expr, $b:expr) => {
-        if $a.is_some() {
-            $b;
-        }
-    }
-}
+   ```rust
+   macro_rules! m {
+       ($a:expr, $b:expr) => {
+           if $a.is_some() {
+               $b;
+           }
+       }
+   }
 
-let x: Option<u32> = Some(42);
-m!(x, x.unwrap());
+   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!(differing_macro_contexts(x_is_some_span, x_unwrap_span), true);
-```
+   // These spans are not from the same context
+   // x.is_some() is from inside the macro
+   // x.unwrap() is from outside the macro
+   assert_eq!(differing_macro_contexts(x_is_some_span, x_unwrap_span), true);
+   ```
 
 [TyS]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TyS.html
 [TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.TyKind.html
-[TypeckTables]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckTables.html
-[expr_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckTables.html#method.expr_ty
+[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.TypeckTables.html#method.pat_ty
-[paths]: ../clippy_lints/src/utils/paths.rs
+[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