]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/misc_early.rs
clippy: support `QPath::LangItem`
[rust.git] / clippy_lints / src / misc_early.rs
index acc83d28897affabce176eb00fedf69c10adbe4a..02789735c17a313b1aa152aa01eadc3df3bf1692 100644 (file)
@@ -1,14 +1,15 @@
-use crate::utils::{
-    constants, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then,
+use crate::utils::{constants, snippet_opt, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
+use rustc_ast::ast::{
+    BindingMode, Expr, ExprKind, GenericParamKind, Generics, Lit, LitFloatType, LitIntType, LitKind, Mutability,
+    NodeId, Pat, PatKind, UnOp,
 };
-use if_chain::if_chain;
-use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintContext, LintPass};
-use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_ast::visit::FnKind;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::Applicability;
-use syntax::ast::*;
-use syntax::source_map::Span;
-use syntax::visit::{walk_expr, FnKind, Visitor};
+use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for structure field patterns bound to wildcards.
     /// **Known problems:** None.
     ///
     /// **Example:**
-    /// ```ignore
-    /// let { a: _, b: ref b, c: _ } = ..
+    /// ```rust
+    /// # struct Foo {
+    /// #     a: i32,
+    /// #     b: i32,
+    /// #     c: i32,
+    /// # }
+    /// let f = Foo { a: 0, b: 0, c: 0 };
+    ///
+    /// // Bad
+    /// match f {
+    ///     Foo { a: _, b: 0, .. } => {},
+    ///     Foo { a: _, b: _, c: _ } => {},
+    /// }
+    ///
+    /// // Good
+    /// match f {
+    ///     Foo { b: 0, .. } => {},
+    ///     Foo { .. } => {},
+    /// }
     /// ```
     pub UNNEEDED_FIELD_PATTERN,
-    style,
+    restriction,
     "struct fields bound to a wildcard instead of using `..`"
 }
 
     ///
     /// **Example:**
     /// ```rust
+    /// // Bad
     /// fn foo(a: i32, _a: i32) {}
+    ///
+    /// // Good
+    /// fn bar(a: i32, _b: i32) {}
     /// ```
     pub DUPLICATE_UNDERSCORE_ARGUMENT,
     style,
     "function arguments having names which only differ by an underscore"
 }
 
-declare_clippy_lint! {
-    /// **What it does:** Detects closures called in the same expression where they
-    /// are defined.
-    ///
-    /// **Why is this bad?** It is unnecessarily adding to the expression's
-    /// complexity.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    /// ```rust,ignore
-    /// (|| 42)()
-    /// ```
-    pub REDUNDANT_CLOSURE_CALL,
-    complexity,
-    "throwaway closures called in the expression they are defined"
-}
-
 declare_clippy_lint! {
     /// **What it does:** Detects expressions of the form `--x`.
     ///
     ///
     /// **Example:**
     /// ```rust
+    /// // Bad
     /// let y = 0x1a9BAcD;
+    ///
+    /// // Good
+    /// let y = 0x1A9BACD;
     /// ```
     pub MIXED_CASE_HEX_LITERALS,
     style,
     ///
     /// **Example:**
     /// ```rust
+    /// // Bad
     /// let y = 123832i32;
+    ///
+    /// // Good
+    /// let y = 123832_i32;
     /// ```
     pub UNSEPARATED_LITERAL_SUFFIX,
     pedantic,
     /// ```rust
     /// # let v = Some("abc");
     ///
+    /// // Bad
     /// match v {
     ///     Some(x) => (),
-    ///     y @ _ => (), // easier written as `y`,
+    ///     y @ _ => (),
+    /// }
+    ///
+    /// // Good
+    /// match v {
+    ///     Some(x) => (),
+    ///     y => (),
     /// }
     /// ```
     pub REDUNDANT_PATTERN,
     /// # struct TupleStruct(u32, u32, u32);
     /// # let t = TupleStruct(1, 2, 3);
     ///
+    /// // Bad
     /// match t {
     ///     TupleStruct(0, .., _) => (),
     ///     _ => (),
     /// }
-    /// ```
-    /// can be written as
-    /// ```rust
-    /// # struct TupleStruct(u32, u32, u32);
-    /// # let t = TupleStruct(1, 2, 3);
     ///
+    /// // Good
     /// match t {
     ///     TupleStruct(0, ..) => (),
     ///     _ => (),
 declare_lint_pass!(MiscEarlyLints => [
     UNNEEDED_FIELD_PATTERN,
     DUPLICATE_UNDERSCORE_ARGUMENT,
-    REDUNDANT_CLOSURE_CALL,
     DOUBLE_NEG,
     MIXED_CASE_HEX_LITERALS,
     UNSEPARATED_LITERAL_SUFFIX,
     UNNEEDED_WILDCARD_PATTERN,
 ]);
 
-// Used to find `return` statements or equivalents e.g., `?`
-struct ReturnVisitor {
-    found_return: bool,
-}
-
-impl ReturnVisitor {
-    fn new() -> Self {
-        Self { found_return: false }
-    }
-}
-
-impl<'ast> Visitor<'ast> for ReturnVisitor {
-    fn visit_expr(&mut self, ex: &'ast Expr) {
-        if let ExprKind::Ret(_) = ex.node {
-            self.found_return = true;
-        } else if let ExprKind::Try(_) = ex.node {
-            self.found_return = true;
-        }
-
-        walk_expr(self, ex)
-    }
-}
-
 impl EarlyLintPass for MiscEarlyLints {
     fn check_generics(&mut self, cx: &EarlyContext<'_>, gen: &Generics) {
         for param in &gen.params {
@@ -279,7 +271,7 @@ fn check_generics(&mut self, cx: &EarlyContext<'_>, gen: &Generics) {
                         cx,
                         BUILTIN_TYPE_SHADOW,
                         param.ident.span,
-                        &format!("This generic shadows the built-in type `{}`", name),
+                        &format!("this generic shadows the built-in type `{}`", name),
                     );
                 }
             }
@@ -287,7 +279,7 @@ fn check_generics(&mut self, cx: &EarlyContext<'_>, gen: &Generics) {
     }
 
     fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) {
-        if let PatKind::Struct(ref npat, ref pfields, _) = pat.node {
+        if let PatKind::Struct(ref npat, ref pfields, _) = pat.kind {
             let mut wilds = 0;
             let type_name = npat
                 .segments
@@ -297,51 +289,54 @@ fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) {
                 .name;
 
             for field in pfields {
-                if let PatKind::Wild = field.pat.node {
+                if let PatKind::Wild = field.pat.kind {
                     wilds += 1;
                 }
             }
             if !pfields.is_empty() && wilds == pfields.len() {
-                span_help_and_lint(
+                span_lint_and_help(
                     cx,
                     UNNEEDED_FIELD_PATTERN,
                     pat.span,
-                    "All the struct fields are matched to a wildcard pattern, consider using `..`.",
-                    &format!("Try with `{} {{ .. }}` instead", type_name),
+                    "all the struct fields are matched to a wildcard pattern, consider using `..`",
+                    None,
+                    &format!("try with `{} {{ .. }}` instead", type_name),
                 );
                 return;
             }
             if wilds > 0 {
-                let mut normal = vec![];
-
-                for field in pfields {
-                    match field.pat.node {
-                        PatKind::Wild => {},
-                        _ => {
-                            if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) {
-                                normal.push(n);
-                            }
-                        },
-                    }
-                }
                 for field in pfields {
-                    if let PatKind::Wild = field.pat.node {
+                    if let PatKind::Wild = field.pat.kind {
                         wilds -= 1;
                         if wilds > 0 {
                             span_lint(
                                 cx,
                                 UNNEEDED_FIELD_PATTERN,
                                 field.span,
-                                "You matched a field with a wildcard pattern. Consider using `..` instead",
+                                "you matched a field with a wildcard pattern, consider using `..` instead",
                             );
                         } else {
-                            span_help_and_lint(
+                            let mut normal = vec![];
+
+                            for field in pfields {
+                                match field.pat.kind {
+                                    PatKind::Wild => {},
+                                    _ => {
+                                        if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) {
+                                            normal.push(n);
+                                        }
+                                    },
+                                }
+                            }
+
+                            span_lint_and_help(
                                 cx,
                                 UNNEEDED_FIELD_PATTERN,
                                 field.span,
-                                "You matched a field with a wildcard pattern. Consider using `..` \
+                                "you matched a field with a wildcard pattern, consider using `..` \
                                  instead",
-                                &format!("Try with `{} {{ {}, .. }}`", type_name, normal[..].join(", ")),
+                                None,
+                                &format!("try with `{} {{ {}, .. }}`", type_name, normal[..].join(", ")),
                             );
                         }
                     }
@@ -349,8 +344,14 @@ fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) {
             }
         }
 
-        if let PatKind::Ident(_, ident, Some(ref right)) = pat.node {
-            if let PatKind::Wild = right.node {
+        if let PatKind::Ident(left, ident, Some(ref right)) = pat.kind {
+            let left_binding = match left {
+                BindingMode::ByRef(Mutability::Mut) => "ref mut ",
+                BindingMode::ByRef(Mutability::Not) => "ref ",
+                BindingMode::ByValue(..) => "",
+            };
+
+            if let PatKind::Wild = right.kind {
                 span_lint_and_sugg(
                     cx,
                     REDUNDANT_PATTERN,
@@ -360,7 +361,7 @@ fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) {
                         ident.name, ident.name,
                     ),
                     "try",
-                    format!("{}", ident.name),
+                    format!("{}{}", left_binding, ident.name),
                     Applicability::MachineApplicable,
                 );
             }
@@ -369,11 +370,11 @@ fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) {
         check_unneeded_wildcard_pattern(cx, pat);
     }
 
-    fn check_fn(&mut self, cx: &EarlyContext<'_>, _: FnKind<'_>, decl: &FnDecl, _: Span, _: NodeId) {
+    fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: Span, _: NodeId) {
         let mut registered_names: FxHashMap<String, Span> = FxHashMap::default();
 
-        for arg in &decl.inputs {
-            if let PatKind::Ident(_, ident, None) = arg.pat.node {
+        for arg in &fn_kind.decl().inputs {
+            if let PatKind::Ident(_, ident, None) = arg.pat.kind {
                 let arg_name = ident.to_string();
 
                 if arg_name.starts_with('_') {
@@ -400,36 +401,9 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
         if in_external_macro(cx.sess(), expr.span) {
             return;
         }
-        match expr.node {
-            ExprKind::Call(ref paren, _) => {
-                if let ExprKind::Paren(ref closure) = paren.node {
-                    if let ExprKind::Closure(_, _, _, ref decl, ref block, _) = closure.node {
-                        let mut visitor = ReturnVisitor::new();
-                        visitor.visit_expr(block);
-                        if !visitor.found_return {
-                            span_lint_and_then(
-                                cx,
-                                REDUNDANT_CLOSURE_CALL,
-                                expr.span,
-                                "Try not to call a closure in the expression where it is declared.",
-                                |db| {
-                                    if decl.inputs.is_empty() {
-                                        let hint = snippet(cx, block.span, "..").into_owned();
-                                        db.span_suggestion(
-                                            expr.span,
-                                            "Try doing something like: ",
-                                            hint,
-                                            Applicability::MachineApplicable, // snippet
-                                        );
-                                    }
-                                },
-                            );
-                        }
-                    }
-                }
-            },
+        match expr.kind {
             ExprKind::Unary(UnOp::Neg, ref inner) => {
-                if let ExprKind::Unary(UnOp::Neg, _) = inner.node {
+                if let ExprKind::Unary(UnOp::Neg, _) = inner.kind {
                     span_lint(
                         cx,
                         DOUBLE_NEG,
@@ -438,39 +412,14 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
                     );
                 }
             },
-            ExprKind::Lit(ref lit) => self.check_lit(cx, lit),
+            ExprKind::Lit(ref lit) => Self::check_lit(cx, lit),
             _ => (),
         }
     }
-
-    fn check_block(&mut self, cx: &EarlyContext<'_>, block: &Block) {
-        for w in block.stmts.windows(2) {
-            if_chain! {
-                if let StmtKind::Local(ref local) = w[0].node;
-                if let Option::Some(ref t) = local.init;
-                if let ExprKind::Closure(..) = t.node;
-                if let PatKind::Ident(_, ident, _) = local.pat.node;
-                if let StmtKind::Semi(ref second) = w[1].node;
-                if let ExprKind::Assign(_, ref call) = second.node;
-                if let ExprKind::Call(ref closure, _) = call.node;
-                if let ExprKind::Path(_, ref path) = closure.node;
-                then {
-                    if ident == path.segments[0].ident {
-                        span_lint(
-                            cx,
-                            REDUNDANT_CLOSURE_CALL,
-                            second.span,
-                            "Closure called just once immediately after it was declared",
-                        );
-                    }
-                }
-            }
-        }
-    }
 }
 
 impl MiscEarlyLints {
-    fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
+    fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) {
         // We test if first character in snippet is a number, because the snippet could be an expansion
         // from a built-in macro like `line!()` or a proc-macro like `#[wasm_bindgen]`.
         // Note that this check also covers special case that `line!()` is eagerly expanded by compiler.
@@ -481,14 +430,18 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
             _ => return,
         };
 
-        if let LitKind::Int(value, lit_int_type) = lit.node {
+        if let LitKind::Int(value, lit_int_type) = lit.kind {
             let suffix = match lit_int_type {
-                LitIntType::Signed(ty) => ty.ty_to_string(),
-                LitIntType::Unsigned(ty) => ty.ty_to_string(),
+                LitIntType::Signed(ty) => ty.name_str(),
+                LitIntType::Unsigned(ty) => ty.name_str(),
                 LitIntType::Unsuffixed => "",
             };
 
-            let maybe_last_sep_idx = lit_snip.len() - suffix.len() - 1;
+            let maybe_last_sep_idx = if let Some(val) = lit_snip.len().checked_sub(suffix.len() + 1) {
+                val
+            } else {
+                return; // It's useless so shouldn't lint.
+            };
             // Do not lint when literal is unsuffixed.
             if !suffix.is_empty() && lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' {
                 span_lint_and_sugg(
@@ -503,6 +456,10 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
             }
 
             if lit_snip.starts_with("0x") {
+                if maybe_last_sep_idx <= 2 {
+                    // It's meaningless or causes range error.
+                    return;
+                }
                 let mut seen = (false, false);
                 for ch in lit_snip.as_bytes()[2..=maybe_last_sep_idx].iter() {
                     match ch {
@@ -528,14 +485,14 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
                     ZERO_PREFIXED_LITERAL,
                     lit.span,
                     "this is a decimal constant",
-                    |db| {
-                        db.span_suggestion(
+                    |diag| {
+                        diag.span_suggestion(
                             lit.span,
                             "if you mean to use a decimal constant, remove the `0` to avoid confusion",
                             lit_snip.trim_start_matches(|c| c == '_' || c == '0').to_string(),
                             Applicability::MaybeIncorrect,
                         );
-                        db.span_suggestion(
+                        diag.span_suggestion(
                             lit.span,
                             "if you mean to use an octal constant, use `0o`",
                             format!("0o{}", lit_snip.trim_start_matches(|c| c == '_' || c == '0')),
@@ -544,9 +501,13 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
                     },
                 );
             }
-        } else if let LitKind::Float(_, float_ty) = lit.node {
-            let suffix = float_ty.ty_to_string();
-            let maybe_last_sep_idx = lit_snip.len() - suffix.len() - 1;
+        } else if let LitKind::Float(_, LitFloatType::Suffixed(float_ty)) = lit.kind {
+            let suffix = float_ty.name_str();
+            let maybe_last_sep_idx = if let Some(val) = lit_snip.len().checked_sub(suffix.len() + 1) {
+                val
+            } else {
+                return; // It's useless so shouldn't lint.
+            };
             if lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' {
                 span_lint_and_sugg(
                     cx,
@@ -563,7 +524,7 @@ fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
 }
 
 fn check_unneeded_wildcard_pattern(cx: &EarlyContext<'_>, pat: &Pat) {
-    if let PatKind::TupleStruct(_, ref patterns) | PatKind::Tuple(ref patterns) = pat.node {
+    if let PatKind::TupleStruct(_, ref patterns) | PatKind::Tuple(ref patterns) = pat.kind {
         fn span_lint(cx: &EarlyContext<'_>, span: Span, only_one: bool) {
             span_lint_and_sugg(
                 cx,
@@ -580,28 +541,22 @@ fn span_lint(cx: &EarlyContext<'_>, span: Span, only_one: bool) {
             );
         }
 
-        #[allow(clippy::trivially_copy_pass_by_ref)]
-        fn is_wild<P: std::ops::Deref<Target = Pat>>(pat: &&P) -> bool {
-            if let PatKind::Wild = pat.node {
-                true
-            } else {
-                false
-            }
-        }
-
         if let Some(rest_index) = patterns.iter().position(|pat| pat.is_rest()) {
             if let Some((left_index, left_pat)) = patterns[..rest_index]
                 .iter()
                 .rev()
-                .take_while(is_wild)
+                .take_while(|pat| matches!(pat.kind, PatKind::Wild))
                 .enumerate()
                 .last()
             {
                 span_lint(cx, left_pat.span.until(patterns[rest_index].span), left_index == 0);
             }
 
-            if let Some((right_index, right_pat)) =
-                patterns[rest_index + 1..].iter().take_while(is_wild).enumerate().last()
+            if let Some((right_index, right_pat)) = patterns[rest_index + 1..]
+                .iter()
+                .take_while(|pat| matches!(pat.kind, PatKind::Wild))
+                .enumerate()
+                .last()
             {
                 span_lint(
                     cx,