Skip to content

Commit 40b558a

Browse files
J-ZhengLiJ-ZhengLi
committed
rename [blocks_in_if_conditions] to [blocks_in_conditions];
add more test cases with `match`; minor fixes in message output regarding review feedback
1 parent fff7aa0 commit 40b558a

22 files changed

+178
-115
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4946,6 +4946,7 @@ Released 2018-09-13
49464946
[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
49474947
[`block_in_if_condition_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_expr
49484948
[`block_in_if_condition_stmt`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_stmt
4949+
[`blocks_in_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions
49494950
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
49504951
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
49514952
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison

‎clippy_lints/src/blocks_in_if_conditions.rs ‎clippy_lints/src/blocks_in_conditions.rs

+21-24
Original file line numberDiff line numberDiff line change
@@ -36,31 +36,36 @@ declare_clippy_lint! {
3636
/// if res { /* ... */ }
3737
/// ```
3838
#[clippy::version = "1.45.0"]
39-
pub BLOCKS_IN_IF_CONDITIONS,
39+
pub BLOCKS_IN_CONDITIONS,
4040
style,
4141
"useless or complex blocks that can be eliminated in conditions"
4242
}
4343

44-
declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]);
44+
declare_lint_pass!(BlocksInConditions => [BLOCKS_IN_CONDITIONS]);
4545

4646
const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition";
47-
const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \
48-
instead, move the block or closure higher and bind it with a `let`";
4947

50-
impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
48+
impl<'tcx> LateLintPass<'tcx> for BlocksInConditions {
5149
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
5250
if in_external_macro(cx.sess(), expr.span) {
5351
return;
5452
}
55-
let Some((cond, keyword)) = higher::If::hir(expr).map(|hif| (hif.cond, "if")).or(
56-
if let ExprKind::Match(match_ex, _, MatchSource::Normal) = expr.kind {
57-
Some((match_ex, "match"))
53+
54+
let Some((cond, keyword, desc)) = higher::If::hir(expr)
55+
.map(|hif| (hif.cond, "if", "an `if` condition"))
56+
.or(if let ExprKind::Match(match_ex, _, MatchSource::Normal) = expr.kind {
57+
Some((match_ex, "match", "a `match` scrutinee"))
5858
} else {
5959
None
60-
},
61-
) else {
60+
})
61+
else {
6262
return;
6363
};
64+
let complex_block_message = &format!(
65+
"in {desc}, avoid complex blocks or closures with blocks; \
66+
instead, move the block or closure higher and bind it with a `let`",
67+
);
68+
6469
if let ExprKind::Block(block, _) = &cond.kind {
6570
if block.rules == BlockCheckMode::DefaultBlock {
6671
if block.stmts.is_empty() {
@@ -73,20 +78,12 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
7378
let mut applicability = Applicability::MachineApplicable;
7479
span_lint_and_sugg(
7580
cx,
76-
BLOCKS_IN_IF_CONDITIONS,
81+
BLOCKS_IN_CONDITIONS,
7782
cond.span,
7883
BRACED_EXPR_MESSAGE,
7984
"try",
80-
format!(
81-
"{}",
82-
snippet_block_with_applicability(
83-
cx,
84-
ex.span,
85-
"..",
86-
Some(expr.span),
87-
&mut applicability
88-
)
89-
),
85+
snippet_block_with_applicability(cx, ex.span, "..", Some(expr.span), &mut applicability)
86+
.to_string(),
9087
applicability,
9188
);
9289
}
@@ -99,9 +96,9 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
9996
let mut applicability = Applicability::MachineApplicable;
10097
span_lint_and_sugg(
10198
cx,
102-
BLOCKS_IN_IF_CONDITIONS,
99+
BLOCKS_IN_CONDITIONS,
103100
expr.span.with_hi(cond.span.hi()),
104-
COMPLEX_BLOCK_MESSAGE,
101+
complex_block_message,
105102
"try",
106103
format!(
107104
"let res = {}; {keyword} res",
@@ -128,7 +125,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
128125
let ex = &body.value;
129126
if let ExprKind::Block(block, _) = ex.kind {
130127
if !body.value.span.from_expansion() && !block.stmts.is_empty() {
131-
span_lint(cx, BLOCKS_IN_IF_CONDITIONS, ex.span, COMPLEX_BLOCK_MESSAGE);
128+
span_lint(cx, BLOCKS_IN_CONDITIONS, ex.span, complex_block_message);
132129
return ControlFlow::Continue(Descend::No);
133130
}
134131
}

‎clippy_lints/src/declared_lints.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
6363
crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO,
6464
crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO,
6565
crate::await_holding_invalid::AWAIT_HOLDING_REFCELL_REF_INFO,
66-
crate::blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS_INFO,
66+
crate::blocks_in_conditions::BLOCKS_IN_CONDITIONS_INFO,
6767
crate::bool_assert_comparison::BOOL_ASSERT_COMPARISON_INFO,
6868
crate::bool_to_int_with_if::BOOL_TO_INT_WITH_IF_INFO,
6969
crate::booleans::NONMINIMAL_BOOL_INFO,

‎clippy_lints/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ mod assertions_on_result_states;
7474
mod async_yields_async;
7575
mod attrs;
7676
mod await_holding_invalid;
77-
mod blocks_in_if_conditions;
77+
mod blocks_in_conditions;
7878
mod bool_assert_comparison;
7979
mod bool_to_int_with_if;
8080
mod booleans;
@@ -653,7 +653,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
653653
store.register_late_pass(|_| Box::<significant_drop_tightening::SignificantDropTightening<'_>>::default());
654654
store.register_late_pass(|_| Box::new(len_zero::LenZero));
655655
store.register_late_pass(|_| Box::new(attrs::Attributes));
656-
store.register_late_pass(|_| Box::new(blocks_in_if_conditions::BlocksInIfConditions));
656+
store.register_late_pass(|_| Box::new(blocks_in_conditions::BlocksInConditions));
657657
store.register_late_pass(|_| Box::new(unicode::Unicode));
658658
store.register_late_pass(|_| Box::new(uninit_vec::UninitVec));
659659
store.register_late_pass(|_| Box::new(unit_return_expecting_ord::UnitReturnExpectingOrd));

‎clippy_lints/src/renamed_lints.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
pub static RENAMED_LINTS: &[(&str, &str)] = &[
55
("clippy::almost_complete_letter_range", "clippy::almost_complete_range"),
66
("clippy::blacklisted_name", "clippy::disallowed_names"),
7-
("clippy::block_in_if_condition_expr", "clippy::blocks_in_if_conditions"),
8-
("clippy::block_in_if_condition_stmt", "clippy::blocks_in_if_conditions"),
7+
("clippy::block_in_if_condition_expr", "clippy::blocks_in_conditions"),
8+
("clippy::block_in_if_condition_stmt", "clippy::blocks_in_conditions"),
9+
("clippy::blocks_in_if_conditions", "clippy::blocks_in_conditions"),
910
("clippy::box_vec", "clippy::box_collection"),
1011
("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes"),
1112
("clippy::cyclomatic_complexity", "clippy::cognitive_complexity"),

‎tests/ui-toml/excessive_nesting/excessive_nesting.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#![allow(clippy::never_loop)]
1010
#![allow(clippy::needless_if)]
1111
#![warn(clippy::excessive_nesting)]
12-
#![allow(clippy::collapsible_if, clippy::blocks_in_if_conditions)]
12+
#![allow(clippy::collapsible_if, clippy::blocks_in_conditions)]
1313

1414
#[macro_use]
1515
extern crate proc_macros;

‎tests/ui/bind_instead_of_map_multipart.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![deny(clippy::bind_instead_of_map)]
2-
#![allow(clippy::blocks_in_if_conditions)]
2+
#![allow(clippy::blocks_in_conditions)]
33

44
pub fn main() {
55
let _ = Some("42").map(|s| if s.len() < 42 { 0 } else { s.len() });

‎tests/ui/bind_instead_of_map_multipart.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![deny(clippy::bind_instead_of_map)]
2-
#![allow(clippy::blocks_in_if_conditions)]
2+
#![allow(clippy::blocks_in_conditions)]
33

44
pub fn main() {
55
let _ = Some("42").and_then(|s| if s.len() < 42 { Some(0) } else { Some(s.len()) });

‎tests/ui/blocks_in_if_conditions.fixed ‎tests/ui/blocks_in_conditions.fixed

+13-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![warn(clippy::blocks_in_if_conditions)]
1+
#![warn(clippy::blocks_in_conditions)]
22
#![allow(unused, clippy::let_and_return, clippy::needless_if)]
33
#![warn(clippy::nonminimal_bool)]
44

@@ -21,6 +21,7 @@ fn macro_if() {
2121

2222
fn condition_has_block() -> i32 {
2323
let res = {
24+
//~^ 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`
2425
let x = 3;
2526
x == 3
2627
}; if res {
@@ -32,6 +33,7 @@ fn condition_has_block() -> i32 {
3233

3334
fn condition_has_block_with_single_expression() -> i32 {
3435
if true { 6 } else { 10 }
36+
//~^ ERROR: omit braces around single expression condition
3537
}
3638

3739
fn condition_is_normal() -> i32 {
@@ -64,12 +66,22 @@ fn block_in_assert() {
6466
// issue #11814
6567
fn block_in_match_expr(num: i32) -> i32 {
6668
let res = {
69+
//~^ 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`
6770
let opt = Some(2);
6871
opt
6972
}; match res {
7073
Some(0) => 1,
7174
Some(n) => num * 2,
7275
None => 0,
76+
};
77+
78+
match unsafe {
79+
let hearty_hearty_hearty = vec![240, 159, 146, 150];
80+
String::from_utf8_unchecked(hearty_hearty_hearty).as_str()
81+
} {
82+
"💖" => 1,
83+
"what" => 2,
84+
_ => 3,
7385
}
7486
}
7587

‎tests/ui/blocks_in_if_conditions.rs ‎tests/ui/blocks_in_conditions.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![warn(clippy::blocks_in_if_conditions)]
1+
#![warn(clippy::blocks_in_conditions)]
22
#![allow(unused, clippy::let_and_return, clippy::needless_if)]
33
#![warn(clippy::nonminimal_bool)]
44

@@ -21,6 +21,7 @@ fn macro_if() {
2121

2222
fn condition_has_block() -> i32 {
2323
if {
24+
//~^ 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`
2425
let x = 3;
2526
x == 3
2627
} {
@@ -32,6 +33,7 @@ fn condition_has_block() -> i32 {
3233

3334
fn condition_has_block_with_single_expression() -> i32 {
3435
if { true } { 6 } else { 10 }
36+
//~^ ERROR: omit braces around single expression condition
3537
}
3638

3739
fn condition_is_normal() -> i32 {
@@ -64,12 +66,22 @@ fn block_in_assert() {
6466
// issue #11814
6567
fn block_in_match_expr(num: i32) -> i32 {
6668
match {
69+
//~^ 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`
6770
let opt = Some(2);
6871
opt
6972
} {
7073
Some(0) => 1,
7174
Some(n) => num * 2,
7275
None => 0,
76+
};
77+
78+
match unsafe {
79+
let hearty_hearty_hearty = vec![240, 159, 146, 150];
80+
String::from_utf8_unchecked(hearty_hearty_hearty).as_str()
81+
} {
82+
"💖" => 1,
83+
"what" => 2,
84+
_ => 3,
7385
}
7486
}
7587

‎tests/ui/blocks_in_if_conditions.stderr ‎tests/ui/blocks_in_conditions.stderr

+11-7
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,44 @@
11
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`
2-
--> $DIR/blocks_in_if_conditions.rs:23:5
2+
--> $DIR/blocks_in_conditions.rs:23:5
33
|
44
LL | / if {
5+
LL | |
56
LL | | let x = 3;
67
LL | | x == 3
78
LL | | } {
89
| |_____^
910
|
10-
= note: `-D clippy::blocks-in-if-conditions` implied by `-D warnings`
11-
= help: to override `-D warnings` add `#[allow(clippy::blocks_in_if_conditions)]`
11+
= note: `-D clippy::blocks-in-conditions` implied by `-D warnings`
12+
= help: to override `-D warnings` add `#[allow(clippy::blocks_in_conditions)]`
1213
help: try
1314
|
1415
LL ~ let res = {
16+
LL +
1517
LL + let x = 3;
1618
LL + x == 3
1719
LL ~ }; if res {
1820
|
1921

2022
error: omit braces around single expression condition
21-
--> $DIR/blocks_in_if_conditions.rs:34:8
23+
--> $DIR/blocks_in_conditions.rs:35:8
2224
|
2325
LL | if { true } { 6 } else { 10 }
2426
| ^^^^^^^^ help: try: `true`
2527

2628
error: this boolean expression can be simplified
27-
--> $DIR/blocks_in_if_conditions.rs:39:8
29+
--> $DIR/blocks_in_conditions.rs:41:8
2830
|
2931
LL | if true && x == 3 { 6 } else { 10 }
3032
| ^^^^^^^^^^^^^^ help: try: `x == 3`
3133
|
3234
= note: `-D clippy::nonminimal-bool` implied by `-D warnings`
3335
= help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`
3436

35-
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`
36-
--> $DIR/blocks_in_if_conditions.rs:66:5
37+
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`
38+
--> $DIR/blocks_in_conditions.rs:68:5
3739
|
3840
LL | / match {
41+
LL | |
3942
LL | | let opt = Some(2);
4043
LL | | opt
4144
LL | | } {
@@ -44,6 +47,7 @@ LL | | } {
4447
help: try
4548
|
4649
LL ~ let res = {
50+
LL +
4751
LL + let opt = Some(2);
4852
LL + opt
4953
LL ~ }; match res {

‎tests/ui/blocks_in_if_conditions_closure.rs ‎tests/ui/blocks_in_conditions_closure.rs

+19-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![warn(clippy::blocks_in_if_conditions)]
1+
#![warn(clippy::blocks_in_conditions)]
22
#![allow(
33
unused,
44
clippy::let_and_return,
@@ -22,7 +22,7 @@ fn pred_test() {
2222
&& predicate(
2323
|x| {
2424
//~^ ERROR: in an `if` condition, avoid complex blocks or closures with blocks
25-
//~| NOTE: `-D clippy::blocks-in-if-conditions` implied by `-D warnings`
25+
//~| NOTE: `-D clippy::blocks-in-conditions` implied by `-D warnings`
2626
let target = 3;
2727
x == target
2828
},
@@ -60,6 +60,23 @@ fn function_with_empty_closure() {
6060
if closure(|| {}) {}
6161
}
6262

63+
// issue #11814
64+
fn match_with_pred() {
65+
let v = 3;
66+
match Some(predicate(
67+
|x| {
68+
//~^ ERROR: in a `match` scrutinee, avoid complex blocks or closures with blocks
69+
let target = 3;
70+
x == target
71+
},
72+
v,
73+
)) {
74+
Some(true) => 1,
75+
Some(false) => 2,
76+
None => 3,
77+
};
78+
}
79+
6380
#[rustfmt::skip]
6481
fn main() {
6582
let mut range = 0..10;

‎tests/ui/blocks_in_if_conditions_closure.stderr ‎tests/ui/blocks_in_conditions_closure.stderr

+16-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
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`
2-
--> $DIR/blocks_in_if_conditions_closure.rs:23:17
2+
--> $DIR/blocks_in_conditions_closure.rs:23:17
33
|
44
LL | |x| {
55
| _________________^
@@ -10,11 +10,11 @@ LL | | x == target
1010
LL | | },
1111
| |_____________^
1212
|
13-
= note: `-D clippy::blocks-in-if-conditions` implied by `-D warnings`
14-
= help: to override `-D warnings` add `#[allow(clippy::blocks_in_if_conditions)]`
13+
= note: `-D clippy::blocks-in-conditions` implied by `-D warnings`
14+
= help: to override `-D warnings` add `#[allow(clippy::blocks_in_conditions)]`
1515

1616
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`
17-
--> $DIR/blocks_in_if_conditions_closure.rs:34:13
17+
--> $DIR/blocks_in_conditions_closure.rs:34:13
1818
|
1919
LL | |x| {
2020
| _____________^
@@ -24,5 +24,16 @@ LL | | x == target
2424
LL | | },
2525
| |_________^
2626

27-
error: aborting due to 2 previous errors
27+
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`
28+
--> $DIR/blocks_in_conditions_closure.rs:67:13
29+
|
30+
LL | |x| {
31+
| _____________^
32+
LL | |
33+
LL | | let target = 3;
34+
LL | | x == target
35+
LL | | },
36+
| |_________^
37+
38+
error: aborting due to 3 previous errors
2839

‎tests/ui/manual_filter.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ fn main() {
4040
};
4141
}
4242

43-
#[allow(clippy::blocks_in_if_conditions)]
43+
#[allow(clippy::blocks_in_conditions)]
4444
Some(11).filter(|&x| {
4545
println!("foo");
4646
x > 10 && x < 100

0 commit comments

Comments
 (0)