Skip to content

Commit 973e70c

Browse files
bors[bot]Michael Wright
and
Michael Wright
committed
Merge #3427
3427: Fix wrong suggestion for `redundant_closure_call` r=oli-obk a=mikerite Fixes #1684 Co-authored-by: Michael Wright <[email protected]>
2 parents 6cc502d + 3ba4c3a commit 973e70c

File tree

2 files changed

+50
-16
lines changed

2 files changed

+50
-16
lines changed

Diff for: clippy_lints/src/misc_early.rs

+45-16
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use if_chain::if_chain;
1515
use std::char;
1616
use crate::syntax::ast::*;
1717
use crate::syntax::source_map::Span;
18-
use crate::syntax::visit::FnKind;
18+
use crate::syntax::visit::{FnKind, Visitor, walk_expr};
1919
use crate::utils::{constants, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_then};
2020
use crate::rustc_errors::Applicability;
2121

@@ -199,6 +199,31 @@ impl LintPass for MiscEarly {
199199
}
200200
}
201201

202+
// Used to find `return` statements or equivalents e.g. `?`
203+
struct ReturnVisitor {
204+
found_return: bool,
205+
}
206+
207+
impl ReturnVisitor {
208+
fn new() -> Self {
209+
Self {
210+
found_return: false,
211+
}
212+
}
213+
}
214+
215+
impl<'ast> Visitor<'ast> for ReturnVisitor {
216+
fn visit_expr(&mut self, ex: &'ast Expr) {
217+
if let ExprKind::Ret(_) = ex.node {
218+
self.found_return = true;
219+
} else if let ExprKind::Try(_) = ex.node {
220+
self.found_return = true;
221+
}
222+
223+
walk_expr(self, ex)
224+
}
225+
}
226+
202227
impl EarlyLintPass for MiscEarly {
203228
fn check_generics(&mut self, cx: &EarlyContext<'_>, gen: &Generics) {
204229
for param in &gen.params {
@@ -311,21 +336,25 @@ impl EarlyLintPass for MiscEarly {
311336
match expr.node {
312337
ExprKind::Call(ref paren, _) => if let ExprKind::Paren(ref closure) = paren.node {
313338
if let ExprKind::Closure(_, _, _, ref decl, ref block, _) = closure.node {
314-
span_lint_and_then(
315-
cx,
316-
REDUNDANT_CLOSURE_CALL,
317-
expr.span,
318-
"Try not to call a closure in the expression where it is declared.",
319-
|db| if decl.inputs.is_empty() {
320-
let hint = snippet(cx, block.span, "..").into_owned();
321-
db.span_suggestion_with_applicability(
322-
expr.span,
323-
"Try doing something like: ",
324-
hint,
325-
Applicability::MachineApplicable, // snippet
326-
);
327-
},
328-
);
339+
let mut visitor = ReturnVisitor::new();
340+
visitor.visit_expr(block);
341+
if !visitor.found_return {
342+
span_lint_and_then(
343+
cx,
344+
REDUNDANT_CLOSURE_CALL,
345+
expr.span,
346+
"Try not to call a closure in the expression where it is declared.",
347+
|db| if decl.inputs.is_empty() {
348+
let hint = snippet(cx, block.span, "..").into_owned();
349+
db.span_suggestion_with_applicability(
350+
expr.span,
351+
"Try doing something like: ",
352+
hint,
353+
Applicability::MachineApplicable, // snippet
354+
);
355+
},
356+
);
357+
}
329358
}
330359
},
331360
ExprKind::Unary(UnOp::Neg, ref inner) => if let ExprKind::Unary(UnOp::Neg, _) = inner.node {

Diff for: tests/ui/redundant_closure_call.rs

+5
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,9 @@ fn main() {
2828
i = closure(3);
2929

3030
i = closure(4);
31+
32+
#[allow(clippy::needless_return)]
33+
(|| return 2)();
34+
(|| -> Option<i32> { None? })();
35+
(|| -> Result<i32, i32> { r#try!(Err(2)) })();
3136
}

0 commit comments

Comments
 (0)