]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/misc_early.rs
Auto merge of #3946 - rchaser53:issue-3920, r=flip1995
[rust.git] / clippy_lints / src / misc_early.rs
index e527a05c85185addd950ab3f23be4cbb5f6656cc..3d41195f34969c624d03f3967964ea9c3293d867 100644 (file)
-use rustc::lint::*;
-use std::collections::HashMap;
+use crate::utils::{constants, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_then};
+use if_chain::if_chain;
+use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintContext, LintPass};
+use rustc::{declare_tool_lint, lint_array};
+use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::Applicability;
 use std::char;
 use syntax::ast::*;
-use syntax::codemap::Span;
-use syntax::visit::FnKind;
-use crate::utils::{constants, in_external_macro, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_then};
+use syntax::source_map::Span;
+use syntax::visit::{walk_expr, FnKind, Visitor};
 
-/// **What it does:** Checks for structure field patterns bound to wildcards.
-///
-/// **Why is this bad?** Using `..` instead is shorter and leaves the focus on
-/// the fields that are actually bound.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-/// ```rust
-/// let { a: _, b: ref b, c: _ } = ..
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Checks for structure field patterns bound to wildcards.
+    ///
+    /// **Why is this bad?** Using `..` instead is shorter and leaves the focus on
+    /// the fields that are actually bound.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```ignore
+    /// let { a: _, b: ref b, c: _ } = ..
+    /// ```
     pub UNNEEDED_FIELD_PATTERN,
     style,
     "struct fields bound to a wildcard instead of using `..`"
 }
 
-/// **What it does:** Checks for function arguments having the similar names
-/// differing by an underscore.
-///
-/// **Why is this bad?** It affects code readability.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-/// ```rust
-/// fn foo(a: i32, _a: i32) {}
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Checks for function arguments having the similar names
+    /// differing by an underscore.
+    ///
+    /// **Why is this bad?** It affects code readability.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// fn foo(a: i32, _a: i32) {}
+    /// ```
     pub DUPLICATE_UNDERSCORE_ARGUMENT,
     style,
     "function arguments having names which only differ by an underscore"
 }
 
-/// **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
-/// (|| 42)()
-/// ```
 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
+    /// (|| 42)()
+    /// ```
     pub REDUNDANT_CLOSURE_CALL,
     complexity,
     "throwaway closures called in the expression they are defined"
 }
 
-/// **What it does:** Detects expressions of the form `--x`.
-///
-/// **Why is this bad?** It can mislead C/C++ programmers to think `x` was
-/// decremented.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-/// ```rust
-/// --x;
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Detects expressions of the form `--x`.
+    ///
+    /// **Why is this bad?** It can mislead C/C++ programmers to think `x` was
+    /// decremented.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// let mut x = 3;
+    /// --x;
+    /// ```
     pub DOUBLE_NEG,
     style,
     "`--x`, which is a double negation of `x` and not a pre-decrement as in C/C++"
 }
 
-/// **What it does:** Warns on hexadecimal literals with mixed-case letter
-/// digits.
-///
-/// **Why is this bad?** It looks confusing.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-/// ```rust
-/// let y = 0x1a9BAcD;
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Warns on hexadecimal literals with mixed-case letter
+    /// digits.
+    ///
+    /// **Why is this bad?** It looks confusing.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// let y = 0x1a9BAcD;
+    /// ```
     pub MIXED_CASE_HEX_LITERALS,
     style,
     "hex literals whose letter digits are not consistently upper- or lowercased"
 }
 
-/// **What it does:** Warns if literal suffixes are not separated by an
-/// underscore.
-///
-/// **Why is this bad?** It is much less readable.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-/// ```rust
-/// let y = 123832i32;
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Warns if literal suffixes are not separated by an
+    /// underscore.
+    ///
+    /// **Why is this bad?** It is much less readable.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// let y = 123832i32;
+    /// ```
     pub UNSEPARATED_LITERAL_SUFFIX,
     pedantic,
     "literals whose suffix is not separated by an underscore"
 }
 
-/// **What it does:** Warns if an integral constant literal starts with `0`.
-///
-/// **Why is this bad?** In some languages (including the infamous C language
-/// and most of its
-/// family), this marks an octal constant. In Rust however, this is a decimal
-/// constant. This could
-/// be confusing for both the writer and a reader of the constant.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// In Rust:
-/// ```rust
-/// fn main() {
-///     let a = 0123;
-///     println!("{}", a);
-/// }
-/// ```
-///
-/// prints `123`, while in C:
-///
-/// ```c
-/// #include <stdio.h>
-///
-/// int main() {
-///     int a = 0123;
-///     printf("%d\n", a);
-/// }
-/// ```
-///
-/// prints `83` (as `83 == 0o123` while `123 == 0o173`).
 declare_clippy_lint! {
+    /// **What it does:** Warns if an integral constant literal starts with `0`.
+    ///
+    /// **Why is this bad?** In some languages (including the infamous C language
+    /// and most of its
+    /// family), this marks an octal constant. In Rust however, this is a decimal
+    /// constant. This could
+    /// be confusing for both the writer and a reader of the constant.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// In Rust:
+    /// ```rust
+    /// fn main() {
+    ///     let a = 0123;
+    ///     println!("{}", a);
+    /// }
+    /// ```
+    ///
+    /// prints `123`, while in C:
+    ///
+    /// ```c
+    /// #include <stdio.h>
+    ///
+    /// int main() {
+    ///     int a = 0123;
+    ///     printf("%d\n", a);
+    /// }
+    /// ```
+    ///
+    /// prints `83` (as `83 == 0o123` while `123 == 0o173`).
     pub ZERO_PREFIXED_LITERAL,
     complexity,
     "integer literals starting with `0`"
 }
 
-/// **What it does:** Warns if a generic shadows a built-in type.
-///
-/// **Why is this bad?** This gives surprising type errors.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// ```rust
-/// impl<u32> Foo<u32> {
-///     fn impl_func(&self) -> u32 {
-///         42
-///     }
-/// }
-/// ```
 declare_clippy_lint! {
+    /// **What it does:** Warns if a generic shadows a built-in type.
+    ///
+    /// **Why is this bad?** This gives surprising type errors.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```ignore
+    /// impl<u32> Foo<u32> {
+    ///     fn impl_func(&self) -> u32 {
+    ///         42
+    ///     }
+    /// }
+    /// ```
     pub BUILTIN_TYPE_SHADOW,
     style,
     "shadowing a builtin type"
@@ -184,10 +188,37 @@ fn get_lints(&self) -> LintArray {
             BUILTIN_TYPE_SHADOW
         )
     }
+
+    fn name(&self) -> &'static str {
+        "MiscEarlyLints"
+    }
+}
+
+// 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 MiscEarly {
-    fn check_generics(&mut self, cx: &EarlyContext, gen: &Generics) {
+    fn check_generics(&mut self, cx: &EarlyContext<'_>, gen: &Generics) {
         for param in &gen.params {
             if let GenericParamKind::Type { .. } = param.kind {
                 let name = param.ident.as_str();
@@ -203,10 +234,11 @@ fn check_generics(&mut self, cx: &EarlyContext, gen: &Generics) {
         }
     }
 
-    fn check_pat(&mut self, cx: &EarlyContext, pat: &Pat) {
+    fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat, _: &mut bool) {
         if let PatKind::Struct(ref npat, ref pfields, _) = pat.node {
             let mut wilds = 0;
-            let type_name = npat.segments
+            let type_name = npat
+                .segments
                 .last()
                 .expect("A path must have at least one segment")
                 .ident
@@ -233,8 +265,10 @@ fn check_pat(&mut self, cx: &EarlyContext, pat: &Pat) {
                 for field in pfields {
                     match field.node.pat.node {
                         PatKind::Wild => {},
-                        _ => if let Ok(n) = cx.sess().codemap().span_to_snippet(field.span) {
-                            normal.push(n);
+                        _ => {
+                            if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) {
+                                normal.push(n);
+                            }
                         },
                     }
                 }
@@ -264,8 +298,8 @@ fn check_pat(&mut self, cx: &EarlyContext, pat: &Pat) {
         }
     }
 
-    fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, decl: &FnDecl, _: Span, _: NodeId) {
-        let mut registered_names: HashMap<String, Span> = HashMap::new();
+    fn check_fn(&mut self, cx: &EarlyContext<'_>, _: FnKind<'_>, decl: &FnDecl, _: 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 {
@@ -291,39 +325,54 @@ fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, decl: &FnDecl, _: Span, _:
         }
     }
 
-    fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
-        if in_external_macro(cx, expr.span) {
+    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 {
-                    span_lint_and_then(
+            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
+                                        );
+                                    }
+                                },
+                            );
+                        }
+                    }
+                }
+            },
+            ExprKind::Unary(UnOp::Neg, ref inner) => {
+                if let ExprKind::Unary(UnOp::Neg, _) = inner.node {
+                    span_lint(
                         cx,
-                        REDUNDANT_CLOSURE_CALL,
+                        DOUBLE_NEG,
                         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);
-                        },
+                        "`--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op",
                     );
                 }
             },
-            ExprKind::Unary(UnOp::Neg, ref inner) => if let ExprKind::Unary(UnOp::Neg, _) = inner.node {
-                span_lint(
-                    cx,
-                    DOUBLE_NEG,
-                    expr.span,
-                    "`--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op",
-                );
-            },
             ExprKind::Lit(ref lit) => self.check_lit(cx, lit),
             _ => (),
         }
     }
 
-    fn check_block(&mut self, cx: &EarlyContext, block: &Block) {
+    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;
@@ -350,7 +399,7 @@ fn check_block(&mut self, cx: &EarlyContext, block: &Block) {
 }
 
 impl MiscEarly {
-    fn check_lit(self, cx: &EarlyContext, lit: &Lit) {
+    fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) {
         if_chain! {
             if let LitKind::Int(value, ..) = lit.node;
             if let Some(src) = snippet_opt(cx, lit.span);
@@ -393,12 +442,14 @@ fn check_lit(self, cx: &EarlyContext, lit: &Lit) {
                         db.span_suggestion(
                             lit.span,
                             "if you mean to use a decimal constant, remove the `0` to remove confusion",
-                            src.trim_left_matches(|c| c == '_' || c == '0').to_string(),
+                            src.trim_start_matches(|c| c == '_' || c == '0').to_string(),
+                            Applicability::MaybeIncorrect,
                         );
                         db.span_suggestion(
                             lit.span,
                             "if you mean to use an octal constant, use `0o`",
-                            format!("0o{}", src.trim_left_matches(|c| c == '_' || c == '0')),
+                            format!("0o{}", src.trim_start_matches(|c| c == '_' || c == '0')),
+                            Applicability::MaybeIncorrect,
                         );
                     });
                 }