Skip to content

Commit

Permalink
Merge block_in_if_condition_expr and block_in_if_condition_stmt l…
Browse files Browse the repository at this point in the history
…ints into `block_in_if_condition` lint
  • Loading branch information
ThibsG committed May 14, 2020
1 parent e1842b0 commit 945c944
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 60 deletions.
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1274,8 +1274,7 @@ Released 2018-09-13
[`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
[`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
[`block_in_if_condition_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_expr
[`block_in_if_condition_stmt`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_stmt
[`block_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
Expand Down
51 changes: 24 additions & 27 deletions clippy_lints/src/block_in_if_condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,43 +8,40 @@ use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:** Checks for `if` conditions that use blocks to contain an
/// expression.
/// **What it does:** Checks for `if` conditions that use blocks containing an
/// expression, statements or conditions that use closures with blocks.
///
/// **Why is this bad?** It isn't really Rust style, same as using parentheses
/// to contain expressions.
/// **Why is this bad?** Style, using blocks in the condition makes it hard to read.
///
/// **Known problems:** None.
///
/// **Example:**
/// **Examples:**
/// ```rust
/// // Bad
/// if { true } { /* ... */ }
///
/// // Good
/// if true { /* ... */ }
/// ```
pub BLOCK_IN_IF_CONDITION_EXPR,
style,
"braces that can be eliminated in conditions, e.g., `if { true } ...`"
}

declare_clippy_lint! {
/// **What it does:** Checks for `if` conditions that use blocks containing
/// statements, or conditions that use closures with blocks.
///
/// **Why is this bad?** Using blocks in the condition makes it hard to read.
/// // or
///
/// **Known problems:** None.
/// ```rust
/// # fn somefunc() -> bool { true };
///
/// **Example:**
/// ```rust,ignore
/// if { let x = somefunc(); x } {}
/// // or
/// if somefunc(|x| { x == 47 }) {}
/// // Bad
/// if { let x = somefunc(); x } { /* ... */ }
///
/// // Good
/// let res = { let x = somefunc(); x };
/// if res { /* ... */ }
/// ```
pub BLOCK_IN_IF_CONDITION_STMT,
pub BLOCK_IN_IF_CONDITION,
style,
"complex blocks in conditions, e.g., `if { let x = true; x } ...`"
"useless or complex blocks that can be eliminated in conditions"
}

declare_lint_pass!(BlockInIfCondition => [BLOCK_IN_IF_CONDITION_EXPR, BLOCK_IN_IF_CONDITION_STMT]);
declare_lint_pass!(BlockInIfCondition => [BLOCK_IN_IF_CONDITION]);

struct ExVisitor<'a, 'tcx> {
found_block: Option<&'tcx Expr<'tcx>>,
Expand Down Expand Up @@ -72,7 +69,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {

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

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
Expand All @@ -92,7 +89,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
BLOCK_IN_IF_CONDITION_EXPR,
BLOCK_IN_IF_CONDITION,
cond.span,
BRACED_EXPR_MESSAGE,
"try",
Expand All @@ -118,7 +115,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
BLOCK_IN_IF_CONDITION_STMT,
BLOCK_IN_IF_CONDITION,
expr.span.with_hi(cond.span.hi()),
COMPLEX_BLOCK_MESSAGE,
"try",
Expand All @@ -140,7 +137,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
let mut visitor = ExVisitor { found_block: None, cx };
walk_expr(&mut visitor, cond);
if let Some(block) = visitor.found_block {
span_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE);
span_lint(cx, BLOCK_IN_IF_CONDITION, block.span, COMPLEX_BLOCK_MESSAGE);
}
}
}
Expand Down
9 changes: 3 additions & 6 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&bit_mask::INEFFECTIVE_BIT_MASK,
&bit_mask::VERBOSE_BIT_MASK,
&blacklisted_name::BLACKLISTED_NAME,
&block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR,
&block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT,
&block_in_if_condition::BLOCK_IN_IF_CONDITION,
&booleans::LOGIC_BUG,
&booleans::NONMINIMAL_BOOL,
&bytecount::NAIVE_BYTECOUNT,
Expand Down Expand Up @@ -1209,8 +1208,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&bit_mask::INEFFECTIVE_BIT_MASK),
LintId::of(&bit_mask::VERBOSE_BIT_MASK),
LintId::of(&blacklisted_name::BLACKLISTED_NAME),
LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR),
LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT),
LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION),
LintId::of(&booleans::LOGIC_BUG),
LintId::of(&booleans::NONMINIMAL_BOOL),
LintId::of(&bytecount::NAIVE_BYTECOUNT),
Expand Down Expand Up @@ -1456,8 +1454,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
LintId::of(&bit_mask::VERBOSE_BIT_MASK),
LintId::of(&blacklisted_name::BLACKLISTED_NAME),
LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR),
LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT),
LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION),
LintId::of(&collapsible_if::COLLAPSIBLE_IF),
LintId::of(&comparison_chain::COMPARISON_CHAIN),
LintId::of(&doc::MISSING_SAFETY_DOC),
Expand Down
11 changes: 2 additions & 9 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,9 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
module: "blacklisted_name",
},
Lint {
name: "block_in_if_condition_expr",
name: "block_in_if_condition",
group: "style",
desc: "braces that can be eliminated in conditions, e.g., `if { true } ...`",
deprecation: None,
module: "block_in_if_condition",
},
Lint {
name: "block_in_if_condition_stmt",
group: "style",
desc: "complex blocks in conditions, e.g., `if { let x = true; x } ...`",
desc: "useless or complex blocks that can be eliminated in conditions",
deprecation: None,
module: "block_in_if_condition",
},
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/block_in_if_condition.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// run-rustfix
#![warn(clippy::block_in_if_condition_expr)]
#![warn(clippy::block_in_if_condition_stmt)]
#![warn(clippy::block_in_if_condition)]
#![allow(unused, clippy::let_and_return)]
#![warn(clippy::nonminimal_bool)]

Expand Down
3 changes: 1 addition & 2 deletions tests/ui/block_in_if_condition.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// run-rustfix
#![warn(clippy::block_in_if_condition_expr)]
#![warn(clippy::block_in_if_condition_stmt)]
#![warn(clippy::block_in_if_condition)]
#![allow(unused, clippy::let_and_return)]
#![warn(clippy::nonminimal_bool)]

Expand Down
10 changes: 4 additions & 6 deletions tests/ui/block_in_if_condition.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
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`
--> $DIR/block_in_if_condition.rs:27:5
--> $DIR/block_in_if_condition.rs:26:5
|
LL | / if {
LL | | let x = 3;
LL | | x == 3
LL | | } {
| |_____^
|
= note: `-D clippy::block-in-if-condition-stmt` implied by `-D warnings`
= note: `-D clippy::block-in-if-condition` implied by `-D warnings`
help: try
|
LL | let res = {
Expand All @@ -17,15 +17,13 @@ LL | }; if res {
|

error: omit braces around single expression condition
--> $DIR/block_in_if_condition.rs:38:8
--> $DIR/block_in_if_condition.rs:37:8
|
LL | if { true } {
| ^^^^^^^^ help: try: `true`
|
= note: `-D clippy::block-in-if-condition-expr` implied by `-D warnings`

error: this boolean expression can be simplified
--> $DIR/block_in_if_condition.rs:47:8
--> $DIR/block_in_if_condition.rs:46:8
|
LL | if true && x == 3 {
| ^^^^^^^^^^^^^^ help: try: `x == 3`
Expand Down
5 changes: 2 additions & 3 deletions tests/ui/block_in_if_condition_closure.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![warn(clippy::block_in_if_condition_expr)]
#![warn(clippy::block_in_if_condition_stmt)]
#![warn(clippy::block_in_if_condition)]
#![allow(unused, clippy::let_and_return)]

fn predicate<F: FnOnce(T) -> bool, T>(pfn: F, val: T) -> bool {
Expand All @@ -10,7 +9,7 @@ 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 nside a closure that the condition is using.
// 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
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/block_in_if_condition_closure.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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`
--> $DIR/block_in_if_condition_closure.rs:19:17
--> $DIR/block_in_if_condition_closure.rs:18:17
|
LL | |x| {
| _________________^
Expand All @@ -8,10 +8,10 @@ LL | | x == target
LL | | },
| |_____________^
|
= note: `-D clippy::block-in-if-condition-stmt` implied by `-D warnings`
= note: `-D clippy::block-in-if-condition` implied by `-D warnings`

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`
--> $DIR/block_in_if_condition_closure.rs:28:13
--> $DIR/block_in_if_condition_closure.rs:27:13
|
LL | |x| {
| _____________^
Expand Down

0 comments on commit 945c944

Please sign in to comment.