]> git.lizzy.rs Git - rust.git/commitdiff
redundant_closure_call - extract lint from misc_early.rs, adapt to LatePass
authorTim Nielens <tim.nielens@gmail.com>
Sat, 18 Jul 2020 22:33:54 +0000 (00:33 +0200)
committerTim Nielens <tim.nielens@gmail.com>
Sun, 19 Jul 2020 22:30:43 +0000 (00:30 +0200)
clippy_lints/src/lib.rs
clippy_lints/src/misc_early.rs
clippy_lints/src/redundant_closure_call.rs [new file with mode: 0644]
src/lintlist/mod.rs
tests/ui/redundant_closure_call.rs [deleted file]
tests/ui/redundant_closure_call.stderr [deleted file]
tests/ui/redundant_closure_call_early.rs [new file with mode: 0644]
tests/ui/redundant_closure_call_early.stderr [new file with mode: 0644]
tests/ui/redundant_closure_call_late.rs [new file with mode: 0644]
tests/ui/redundant_closure_call_late.stderr [new file with mode: 0644]

index 823afdfd289bd21205bc27320616bef3f1b3f8a3..f371942dbeec57056ca35176bbda2f005d9d8084 100644 (file)
@@ -276,6 +276,7 @@ macro_rules! declare_clippy_lint {
 mod question_mark;
 mod ranges;
 mod redundant_clone;
+mod redundant_closure_call;
 mod redundant_field_names;
 mod redundant_pub_crate;
 mod redundant_static_lifetimes;
@@ -702,7 +703,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &misc_early::DOUBLE_NEG,
         &misc_early::DUPLICATE_UNDERSCORE_ARGUMENT,
         &misc_early::MIXED_CASE_HEX_LITERALS,
-        &misc_early::REDUNDANT_CLOSURE_CALL,
         &misc_early::REDUNDANT_PATTERN,
         &misc_early::UNNEEDED_FIELD_PATTERN,
         &misc_early::UNNEEDED_WILDCARD_PATTERN,
@@ -759,6 +759,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &ranges::RANGE_ZIP_WITH_LEN,
         &ranges::REVERSED_EMPTY_RANGES,
         &redundant_clone::REDUNDANT_CLONE,
+        &redundant_closure_call::REDUNDANT_CLOSURE_CALL,
         &redundant_field_names::REDUNDANT_FIELD_NAMES,
         &redundant_pub_crate::REDUNDANT_PUB_CRATE,
         &redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
@@ -1018,6 +1019,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| box int_plus_one::IntPlusOne);
     store.register_early_pass(|| box formatting::Formatting);
     store.register_early_pass(|| box misc_early::MiscEarlyLints);
+    store.register_early_pass(|| box redundant_closure_call::RedundantClosureCall);
+    store.register_late_pass(|| box redundant_closure_call::RedundantClosureCall);
     store.register_early_pass(|| box returns::Return);
     store.register_late_pass(|| box let_and_return::LetReturn);
     store.register_early_pass(|| box collapsible_if::CollapsibleIf);
@@ -1359,7 +1362,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&misc_early::DOUBLE_NEG),
         LintId::of(&misc_early::DUPLICATE_UNDERSCORE_ARGUMENT),
         LintId::of(&misc_early::MIXED_CASE_HEX_LITERALS),
-        LintId::of(&misc_early::REDUNDANT_CLOSURE_CALL),
         LintId::of(&misc_early::REDUNDANT_PATTERN),
         LintId::of(&misc_early::UNNEEDED_WILDCARD_PATTERN),
         LintId::of(&misc_early::ZERO_PREFIXED_LITERAL),
@@ -1393,6 +1395,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
         LintId::of(&ranges::REVERSED_EMPTY_RANGES),
         LintId::of(&redundant_clone::REDUNDANT_CLONE),
+        LintId::of(&redundant_closure_call::REDUNDANT_CLOSURE_CALL),
         LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
         LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
         LintId::of(&reference::DEREF_ADDROF),
@@ -1593,7 +1596,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::UNNECESSARY_FILTER_MAP),
         LintId::of(&methods::USELESS_ASREF),
         LintId::of(&misc::SHORT_CIRCUIT_STATEMENT),
-        LintId::of(&misc_early::REDUNDANT_CLOSURE_CALL),
         LintId::of(&misc_early::UNNEEDED_WILDCARD_PATTERN),
         LintId::of(&misc_early::ZERO_PREFIXED_LITERAL),
         LintId::of(&needless_bool::BOOL_COMPARISON),
@@ -1608,6 +1610,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&precedence::PRECEDENCE),
         LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
         LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
+        LintId::of(&redundant_closure_call::REDUNDANT_CLOSURE_CALL),
         LintId::of(&reference::DEREF_ADDROF),
         LintId::of(&reference::REF_IN_DEREF),
         LintId::of(&repeat_once::REPEAT_ONCE),
index 125df226ceb3793c3ea97d08ec7fc33e8598a64e..29aba7c121873bbd6c57aa3d24d2758942040ba3 100644 (file)
@@ -1,20 +1,15 @@
-use crate::utils::{
-    constants, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_help, span_lint_and_sugg,
-    span_lint_and_then,
-};
-use if_chain::if_chain;
+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, Block, Expr, ExprKind, GenericParamKind, Generics, Lit, LitFloatType, LitIntType, LitKind, Mutability,
-    NodeId, Pat, PatKind, StmtKind, UnOp,
+    BindingMode, Expr, ExprKind, GenericParamKind, Generics, Lit, LitFloatType, LitIntType, LitKind, Mutability,
+    NodeId, Pat, PatKind, UnOp,
 };
-use rustc_ast::visit::{walk_expr, FnKind, Visitor};
+use rustc_ast::visit::FnKind;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::Applicability;
 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;
-use rustc_span::symbol::Ident;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for structure field patterns bound to wildcards.
     "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
-    /// // Bad
-    /// let a = (|| 42)()
-    ///
-    /// // Good
-    /// let a = 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`.
     ///
 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 {
-    #[must_use]
-    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.kind {
-            self.found_return = true;
-        } else if let ExprKind::Try(_) = ex.kind {
-            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 {
@@ -454,30 +402,6 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
             return;
         }
         match expr.kind {
-            ExprKind::Call(ref paren, _) => {
-                if let ExprKind::Paren(ref closure) = paren.kind {
-                    if let ExprKind::Closure(_, _, _, ref decl, ref block, _) = closure.kind {
-                        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.",
-                                |diag| {
-                                    if decl.inputs.is_empty() {
-                                        let mut app = Applicability::MachineApplicable;
-                                        let hint =
-                                            snippet_with_applicability(cx, block.span, "..", &mut app).into_owned();
-                                        diag.span_suggestion(expr.span, "Try doing something like: ", hint, app);
-                                    }
-                                },
-                            );
-                        }
-                    }
-                }
-            },
             ExprKind::Unary(UnOp::Neg, ref inner) => {
                 if let ExprKind::Unary(UnOp::Neg, _) = inner.kind {
                     span_lint(
@@ -492,54 +416,6 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
             _ => (),
         }
     }
-
-    fn check_block(&mut self, cx: &EarlyContext<'_>, block: &Block) {
-        fn count_closure_usage(block: &Block, ident: &Ident) -> usize {
-            struct ClosureUsageCount<'ast> {
-                ident: &'ast Ident,
-                count: usize,
-            };
-            impl<'ast> Visitor<'ast> for ClosureUsageCount<'ast> {
-                fn visit_expr(&mut self, expr: &'ast Expr) {
-                    if_chain! {
-                        if let ExprKind::Call(ref closure, _) = expr.kind;
-                        if let ExprKind::Path(_, ref path) = closure.kind;
-                        if self.ident == &path.segments[0].ident;
-                        then {
-                            self.count += 1;
-                        }
-                    }
-                    walk_expr(self, expr);
-                }
-            }
-            let mut closure_usage_count = ClosureUsageCount { ident, count: 0 };
-            closure_usage_count.visit_block(block);
-            closure_usage_count.count
-        }
-
-        for w in block.stmts.windows(2) {
-            if_chain! {
-                if let StmtKind::Local(ref local) = w[0].kind;
-                if let Option::Some(ref t) = local.init;
-                if let ExprKind::Closure(..) = t.kind;
-                if let PatKind::Ident(_, ident, _) = local.pat.kind;
-                if let StmtKind::Semi(ref second) = w[1].kind;
-                if let ExprKind::Assign(_, ref call, _) = second.kind;
-                if let ExprKind::Call(ref closure, _) = call.kind;
-                if let ExprKind::Path(_, ref path) = closure.kind;
-                if ident == path.segments[0].ident;
-                if  count_closure_usage(block, &ident) == 1;
-                then {
-                    span_lint(
-                        cx,
-                        REDUNDANT_CLOSURE_CALL,
-                        second.span,
-                        "Closure called just once immediately after it was declared",
-                    );
-                }
-            }
-        }
-    }
 }
 
 impl MiscEarlyLints {
diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs
new file mode 100644 (file)
index 0000000..391d70a
--- /dev/null
@@ -0,0 +1,151 @@
+use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_then};
+use if_chain::if_chain;
+use rustc_ast::ast;
+use rustc_ast::visit as ast_visit;
+use rustc_ast::visit::Visitor as AstVisitor;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_hir::intravisit as hir_visit;
+use rustc_hir::intravisit::Visitor as HirVisitor;
+use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
+use rustc_middle::hir::map::Map;
+use rustc_middle::lint::in_external_macro;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::Ident;
+
+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
+    /// // Bad
+    /// let a = (|| 42)()
+    ///
+    /// // Good
+    /// let a = 42
+    /// ```
+    pub REDUNDANT_CLOSURE_CALL,
+    complexity,
+    "throwaway closures called in the expression they are defined"
+}
+
+declare_lint_pass!(RedundantClosureCall => [REDUNDANT_CLOSURE_CALL]);
+
+// Used to find `return` statements or equivalents e.g., `?`
+struct ReturnVisitor {
+    found_return: bool,
+}
+
+impl ReturnVisitor {
+    #[must_use]
+    fn new() -> Self {
+        Self { found_return: false }
+    }
+}
+
+impl<'ast> ast_visit::Visitor<'ast> for ReturnVisitor {
+    fn visit_expr(&mut self, ex: &'ast ast::Expr) {
+        if let ast::ExprKind::Ret(_) = ex.kind {
+            self.found_return = true;
+        } else if let ast::ExprKind::Try(_) = ex.kind {
+            self.found_return = true;
+        }
+
+        ast_visit::walk_expr(self, ex)
+    }
+}
+
+impl EarlyLintPass for RedundantClosureCall {
+    fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
+        if in_external_macro(cx.sess(), expr.span) {
+            return;
+        }
+        if_chain! {
+            if let ast::ExprKind::Call(ref paren, _) = expr.kind;
+            if let ast::ExprKind::Paren(ref closure) = paren.kind;
+            if let ast::ExprKind::Closure(_, _, _, ref decl, ref block, _) = closure.kind;
+            then {
+                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.",
+                        |diag| {
+                            if decl.inputs.is_empty() {
+                                let mut app = Applicability::MachineApplicable;
+                                let hint =
+                                    snippet_with_applicability(cx, block.span, "..", &mut app).into_owned();
+                                diag.span_suggestion(expr.span, "Try doing something like: ", hint, app);
+                            }
+                        },
+                    );
+                }
+            }
+        }
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall {
+    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
+        fn count_closure_usage<'tcx>(block: &'tcx hir::Block<'_>, ident: &'tcx Ident) -> usize {
+            struct ClosureUsageCount<'tcx> {
+                ident: &'tcx Ident,
+                count: usize,
+            };
+            impl<'tcx> hir_visit::Visitor<'tcx> for ClosureUsageCount<'tcx> {
+                type Map = Map<'tcx>;
+
+                fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
+                    if_chain! {
+                        if let hir::ExprKind::Call(ref closure, _) = expr.kind;
+                        if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = closure.kind;
+                        if self.ident == &path.segments[0].ident;
+                        then {
+                            self.count += 1;
+                        }
+                    }
+                    hir_visit::walk_expr(self, expr);
+                }
+
+                fn nested_visit_map(&mut self) -> hir_visit::NestedVisitorMap<Self::Map> {
+                    hir_visit::NestedVisitorMap::None
+                }
+            };
+            let mut closure_usage_count = ClosureUsageCount { ident, count: 0 };
+            closure_usage_count.visit_block(block);
+            closure_usage_count.count
+        }
+
+        for w in block.stmts.windows(2) {
+            if_chain! {
+                if let hir::StmtKind::Local(ref local) = w[0].kind;
+                if let Option::Some(ref t) = local.init;
+                if let hir::ExprKind::Closure(..) = t.kind;
+                if let hir::PatKind::Binding(_, _, ident, _) = local.pat.kind;
+                if let hir::StmtKind::Semi(ref second) = w[1].kind;
+                if let hir::ExprKind::Assign(_, ref call, _) = second.kind;
+                if let hir::ExprKind::Call(ref closure, _) = call.kind;
+                if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = closure.kind;
+                if ident == path.segments[0].ident;
+                if  count_closure_usage(block, &ident) == 1;
+                then {
+                    span_lint(
+                        cx,
+                        REDUNDANT_CLOSURE_CALL,
+                        second.span,
+                        "Closure called just once immediately after it was declared",
+                    );
+                }
+            }
+        }
+    }
+}
index 96b004904aa2dce9032db127e0c20c4e70f2eac6..1879aae77fb650783301cd1bc8500cca8551bdb7 100644 (file)
         group: "complexity",
         desc: "throwaway closures called in the expression they are defined",
         deprecation: None,
-        module: "misc_early",
+        module: "redundant_closure_call",
     },
     Lint {
         name: "redundant_closure_for_method_calls",
diff --git a/tests/ui/redundant_closure_call.rs b/tests/ui/redundant_closure_call.rs
deleted file mode 100644 (file)
index 0f2ba4a..0000000
+++ /dev/null
@@ -1,30 +0,0 @@
-// non rustfixable, see redundant_closure_call_fixable.rs
-
-#![warn(clippy::redundant_closure_call)]
-
-fn main() {
-    let mut i = 1;
-    let mut k = (|m| m + 1)(i);
-
-    k = (|a, b| a * b)(1, 5);
-
-    // don't lint here, the closure is used more than once
-    let closure = |i| i + 1;
-    i = closure(3);
-    i = closure(4);
-
-    // lint here
-    let redun_closure = || 1;
-    i = redun_closure();
-
-    // the lint is applicable here but the lint doesn't support redefinition
-    let redefined_closure = || 1;
-    i = redefined_closure();
-    let redefined_closure = || 2;
-    i = redefined_closure();
-
-    #[allow(clippy::needless_return)]
-    (|| return 2)();
-    (|| -> Option<i32> { None? })();
-    (|| -> Result<i32, i32> { Err(2)? })();
-}
diff --git a/tests/ui/redundant_closure_call.stderr b/tests/ui/redundant_closure_call.stderr
deleted file mode 100644 (file)
index d5e0664..0000000
+++ /dev/null
@@ -1,22 +0,0 @@
-error: Closure called just once immediately after it was declared
-  --> $DIR/redundant_closure_call.rs:18:5
-   |
-LL |     i = redun_closure();
-   |     ^^^^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::redundant-closure-call` implied by `-D warnings`
-
-error: Try not to call a closure in the expression where it is declared.
-  --> $DIR/redundant_closure_call.rs:7:17
-   |
-LL |     let mut k = (|m| m + 1)(i);
-   |                 ^^^^^^^^^^^^^^
-
-error: Try not to call a closure in the expression where it is declared.
-  --> $DIR/redundant_closure_call.rs:9:9
-   |
-LL |     k = (|a, b| a * b)(1, 5);
-   |         ^^^^^^^^^^^^^^^^^^^^
-
-error: aborting due to 3 previous errors
-
diff --git a/tests/ui/redundant_closure_call_early.rs b/tests/ui/redundant_closure_call_early.rs
new file mode 100644 (file)
index 0000000..3dd3656
--- /dev/null
@@ -0,0 +1,19 @@
+// non rustfixable, see redundant_closure_call_fixable.rs
+
+#![warn(clippy::redundant_closure_call)]
+
+fn main() {
+    let mut i = 1;
+
+    // lint here
+    let mut k = (|m| m + 1)(i);
+
+    // lint here
+    k = (|a, b| a * b)(1, 5);
+
+    // don't lint these
+    #[allow(clippy::needless_return)]
+    (|| return 2)();
+    (|| -> Option<i32> { None? })();
+    (|| -> Result<i32, i32> { Err(2)? })();
+}
diff --git a/tests/ui/redundant_closure_call_early.stderr b/tests/ui/redundant_closure_call_early.stderr
new file mode 100644 (file)
index 0000000..0ac97ae
--- /dev/null
@@ -0,0 +1,16 @@
+error: Try not to call a closure in the expression where it is declared.
+  --> $DIR/redundant_closure_call_early.rs:9:17
+   |
+LL |     let mut k = (|m| m + 1)(i);
+   |                 ^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::redundant-closure-call` implied by `-D warnings`
+
+error: Try not to call a closure in the expression where it is declared.
+  --> $DIR/redundant_closure_call_early.rs:12:9
+   |
+LL |     k = (|a, b| a * b)(1, 5);
+   |         ^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
diff --git a/tests/ui/redundant_closure_call_late.rs b/tests/ui/redundant_closure_call_late.rs
new file mode 100644 (file)
index 0000000..21ee58e
--- /dev/null
@@ -0,0 +1,22 @@
+// non rustfixable, see redundant_closure_call_fixable.rs
+
+#![warn(clippy::redundant_closure_call)]
+
+fn main() {
+    let mut i = 1;
+
+    // don't lint here, the closure is used more than once
+    let closure = |i| i + 1;
+    i = closure(3);
+    i = closure(4);
+
+    // lint here
+    let redun_closure = || 1;
+    i = redun_closure();
+
+    // the lint is applicable here but the lint doesn't support redefinition
+    let redefined_closure = || 1;
+    i = redefined_closure();
+    let redefined_closure = || 2;
+    i = redefined_closure();
+}
diff --git a/tests/ui/redundant_closure_call_late.stderr b/tests/ui/redundant_closure_call_late.stderr
new file mode 100644 (file)
index 0000000..0aebc3c
--- /dev/null
@@ -0,0 +1,10 @@
+error: Closure called just once immediately after it was declared
+  --> $DIR/redundant_closure_call_late.rs:15:5
+   |
+LL |     i = redun_closure();
+   |     ^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::redundant-closure-call` implied by `-D warnings`
+
+error: aborting due to previous error
+