Auto merge of #12805 - Alexendoo:blocks-in-conditions-closures, r=dswij

Don't lint blocks in closures for blocks_in_conditions

Seemed like an outlier for the lint which generally caught only the syntactically confusing cases, it lints blocks in closures but excludes closures passed to iterator methods, this changes it to ignore closures in general

changelog: none
This commit is contained in:
bors 2024-06-08 09:45:22 +00:00
commit 48686adf48
5 changed files with 28 additions and 158 deletions

View file

@ -1,15 +1,11 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_block_with_applicability;
use clippy_utils::ty::implements_trait;
use clippy_utils::visitors::{for_each_expr_without_closures, Descend};
use clippy_utils::{get_parent_expr, higher, is_from_proc_macro};
use core::ops::ControlFlow;
use clippy_utils::{higher, is_from_proc_macro};
use rustc_errors::Applicability;
use rustc_hir::{BlockCheckMode, Expr, ExprKind, MatchSource};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::declare_lint_pass;
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
@ -124,30 +120,6 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInConditions {
);
}
}
} else {
let _: Option<!> = for_each_expr_without_closures(cond, |e| {
if let ExprKind::Closure(closure) = e.kind {
// do not lint if the closure is called using an iterator (see #1141)
if let Some(parent) = get_parent_expr(cx, e)
&& let ExprKind::MethodCall(_, self_arg, _, _) = &parent.kind
&& let caller = cx.typeck_results().expr_ty(self_arg)
&& let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator)
&& implements_trait(cx, caller, iter_id, &[])
{
return ControlFlow::Continue(Descend::No);
}
let body = cx.tcx.hir().body(closure.body);
let ex = &body.value;
if let ExprKind::Block(block, _) = ex.kind {
if !body.value.span.from_expansion() && !block.stmts.is_empty() {
span_lint(cx, BLOCKS_IN_CONDITIONS, ex.span, complex_block_message.clone());
return ControlFlow::Continue(Descend::No);
}
}
}
ControlFlow::Continue(Descend::Yes)
});
}
}
}

View file

@ -117,4 +117,17 @@ mod issue_12016 {
}
}
fn in_closure() {
let v = vec![1, 2, 3];
if v.into_iter()
.filter(|x| {
let y = x + 1;
y > 3
})
.any(|x| x == 5)
{
println!("contains 4!");
}
}
fn main() {}

View file

@ -117,4 +117,17 @@ mod issue_12016 {
}
}
fn in_closure() {
let v = vec![1, 2, 3];
if v.into_iter()
.filter(|x| {
let y = x + 1;
y > 3
})
.any(|x| x == 5)
{
println!("contains 4!");
}
}
fn main() {}

View file

@ -1,89 +0,0 @@
#![warn(clippy::blocks_in_conditions)]
#![allow(
unused,
clippy::let_and_return,
clippy::needless_if,
clippy::unnecessary_literal_unwrap
)]
fn predicate<F: FnOnce(T) -> bool, T>(pfn: F, val: T) -> bool {
pfn(val)
}
fn pred_test() {
let v = 3;
let sky = "blue";
// This is a sneaky case, where the block isn't directly in the condition,
// but is actually inside a closure that the condition is using.
// The same principle applies -- add some extra expressions to make sure
// linter isn't confused by them.
if v == 3
&& sky == "blue"
&& predicate(
|x| {
//~^ ERROR: in an `if` condition, avoid complex blocks or closures with blocks
//~| NOTE: `-D clippy::blocks-in-conditions` implied by `-D warnings`
let target = 3;
x == target
},
v,
)
{}
if predicate(
|x| {
//~^ ERROR: in an `if` condition, avoid complex blocks or closures with blocks; in
let target = 3;
x == target
},
v,
) {}
}
fn closure_without_block() {
if predicate(|x| x == 3, 6) {}
}
fn macro_in_closure() {
let option = Some(true);
if option.unwrap_or_else(|| unimplemented!()) {
unimplemented!()
}
}
fn closure(_: impl FnMut()) -> bool {
true
}
fn function_with_empty_closure() {
if closure(|| {}) {}
}
// issue #11814
fn match_with_pred() {
let v = 3;
match Some(predicate(
|x| {
//~^ ERROR: in a `match` scrutinee, avoid complex blocks or closures with blocks
let target = 3;
x == target
},
v,
)) {
Some(true) => 1,
Some(false) => 2,
None => 3,
};
}
#[rustfmt::skip]
fn main() {
let mut range = 0..10;
range.all(|i| {i < 10} );
let v = vec![1, 2, 3];
if v.into_iter().any(|x| {x == 4}) {
println!("contains 4!");
}
}

View file

@ -1,39 +0,0 @@
error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
--> tests/ui/blocks_in_conditions_closure.rs:23:17
|
LL | |x| {
| _________________^
LL | |
LL | |
LL | | let target = 3;
LL | | x == target
LL | | },
| |_____________^
|
= note: `-D clippy::blocks-in-conditions` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::blocks_in_conditions)]`
error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
--> tests/ui/blocks_in_conditions_closure.rs:34:13
|
LL | |x| {
| _____________^
LL | |
LL | | let target = 3;
LL | | x == target
LL | | },
| |_________^
error: in a `match` scrutinee, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
--> tests/ui/blocks_in_conditions_closure.rs:67:13
|
LL | |x| {
| _____________^
LL | |
LL | | let target = 3;
LL | | x == target
LL | | },
| |_________^
error: aborting due to 3 previous errors