From e2e892b59b1f31d75a6d37302ebb9d25d67de4c3 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Wed, 14 Nov 2018 08:01:39 +0200 Subject: [PATCH 1/2] Fix wrong suggestion for `redundant_closure_call` Fixes #1684 --- clippy_lints/src/misc_early.rs | 61 ++++++++++++++++++++++-------- tests/ui/redundant_closure_call.rs | 5 +++ 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index a2fd487078e..550d7f24210 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -15,7 +15,7 @@ use if_chain::if_chain; use std::char; use crate::syntax::ast::*; use crate::syntax::source_map::Span; -use crate::syntax::visit::FnKind; +use crate::syntax::visit::{FnKind, Visitor, walk_expr}; use crate::utils::{constants, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_then}; use crate::rustc_errors::Applicability; @@ -199,6 +199,31 @@ impl LintPass for MiscEarly { } } +// Used to find `return` statements or equivalents e.g. `?` +struct ReturnVisitor { + found_return: bool, +} + +impl ReturnVisitor { + fn new() -> ReturnVisitor { + ReturnVisitor { + 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) { for param in &gen.params { @@ -311,21 +336,25 @@ impl EarlyLintPass for MiscEarly { 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( - 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_with_applicability( - expr.span, - "Try doing something like: ", - hint, - Applicability::MachineApplicable, // snippet - ); - }, - ); + 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_with_applicability( + expr.span, + "Try doing something like: ", + hint, + Applicability::MachineApplicable, // snippet + ); + }, + ); + } } }, ExprKind::Unary(UnOp::Neg, ref inner) => if let ExprKind::Unary(UnOp::Neg, _) = inner.node { diff --git a/tests/ui/redundant_closure_call.rs b/tests/ui/redundant_closure_call.rs index 4912e5fc1b4..e68cdc2c1d1 100644 --- a/tests/ui/redundant_closure_call.rs +++ b/tests/ui/redundant_closure_call.rs @@ -28,4 +28,9 @@ fn main() { i = closure(3); i = closure(4); + + #[allow(clippy::needless_return)] + (|| return 2)(); + (|| -> Option { None? })(); + (|| -> Result { r#try!(Err(2)) })(); } From 3ba4c3a9b179be4ed7e169475914997270f13b77 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Wed, 14 Nov 2018 08:43:35 +0200 Subject: [PATCH 2/2] Fix `use_self` violation --- clippy_lints/src/misc_early.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index 550d7f24210..8f2f36ff560 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -205,8 +205,8 @@ struct ReturnVisitor { } impl ReturnVisitor { - fn new() -> ReturnVisitor { - ReturnVisitor { + fn new() -> Self { + Self { found_return: false, } }