mirror of
https://github.com/rust-lang/rust-clippy
synced 2025-02-17 06:28:42 +00:00
Rollup merge of #5408 - dtolnay:matchbool, r=flip1995
Downgrade match_bool to pedantic I don't quite buy the justification in https://rust-lang.github.io/rust-clippy/. The justification is: > It makes the code less readable. In the Rust codebases I've worked in, I have found people were comfortable using `match bool` (selectively) to make code more readable. For example, initializing struct fields is a place where the indentation of `match` can work better than the indentation of `if`: ```rust let _ = Struct { v: { ... }, w: match doing_w { true => ..., false => ..., }, x: Nested { c: ..., b: ..., a: ..., }, y: if doing_y { ... } else { // :( ... }, z: ..., }; ``` Or sometimes people prefer something a bit less pithy than `if` when the meaning of the bool doesn't read off clearly from the condition: ```rust if set.insert(...) { ... // ??? } else { ... } match set.insert(...) { // set.insert returns false if already present false => ..., true => ..., } ``` Or `match` can be a better fit when the bool is playing the role more of a value than a branch condition: ```rust impl ErrorCodes { pub fn from(b: bool) -> Self { match b { true => ErrorCodes::Yes, false => ErrorCodes::No, } } } ``` And then there's plain old it's-1-line-shorter, which means we get 25% more content on a screen when stacking a sequence of conditions: ```rust let old_noun = match old_binding.is_import() { true => "import", false => "definition", }; let new_participle = match new_binding.is_import() { true => "imported", false => "defined", }; ``` Bottom line is I think this lint fits the bill better as a pedantic lint; I don't think linting on this by default is justified. changelog: Remove match_bool from default set of enabled lints
This commit is contained in:
commit
e1d13c34b0
10 changed files with 30 additions and 27 deletions
|
@ -1134,6 +1134,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
|
||||
LintId::of(&loops::EXPLICIT_ITER_LOOP),
|
||||
LintId::of(¯o_use::MACRO_USE_IMPORTS),
|
||||
LintId::of(&matches::MATCH_BOOL),
|
||||
LintId::of(&matches::SINGLE_MATCH_ELSE),
|
||||
LintId::of(&methods::FILTER_MAP),
|
||||
LintId::of(&methods::FILTER_MAP_NEXT),
|
||||
|
@ -1279,7 +1280,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
|
||||
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
|
||||
LintId::of(&matches::MATCH_AS_REF),
|
||||
LintId::of(&matches::MATCH_BOOL),
|
||||
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
|
||||
LintId::of(&matches::MATCH_REF_PATS),
|
||||
LintId::of(&matches::MATCH_SINGLE_BINDING),
|
||||
|
@ -1470,7 +1470,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&main_recursion::MAIN_RECURSION),
|
||||
LintId::of(&map_clone::MAP_CLONE),
|
||||
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
|
||||
LintId::of(&matches::MATCH_BOOL),
|
||||
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
|
||||
LintId::of(&matches::MATCH_REF_PATS),
|
||||
LintId::of(&matches::MATCH_WILD_ERR_ARM),
|
||||
|
|
|
@ -138,7 +138,7 @@ declare_clippy_lint! {
|
|||
/// }
|
||||
/// ```
|
||||
pub MATCH_BOOL,
|
||||
style,
|
||||
pedantic,
|
||||
"a `match` on a boolean expression instead of an `if..else` block"
|
||||
}
|
||||
|
||||
|
|
|
@ -1139,7 +1139,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
|
|||
},
|
||||
Lint {
|
||||
name: "match_bool",
|
||||
group: "style",
|
||||
group: "pedantic",
|
||||
desc: "a `match` on a boolean expression instead of an `if..else` block",
|
||||
deprecation: None,
|
||||
module: "matches",
|
||||
|
|
|
@ -21,7 +21,6 @@ fn test_if_block() -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::match_bool)]
|
||||
#[rustfmt::skip]
|
||||
fn test_match(x: bool) -> bool {
|
||||
match x {
|
||||
|
@ -30,7 +29,7 @@ fn test_match(x: bool) -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::match_bool, clippy::needless_return)]
|
||||
#[allow(clippy::needless_return)]
|
||||
fn test_match_with_unreachable(x: bool) -> bool {
|
||||
match x {
|
||||
true => return false,
|
||||
|
|
|
@ -21,7 +21,6 @@ fn test_if_block() -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::match_bool)]
|
||||
#[rustfmt::skip]
|
||||
fn test_match(x: bool) -> bool {
|
||||
match x {
|
||||
|
@ -30,7 +29,7 @@ fn test_match(x: bool) -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::match_bool, clippy::needless_return)]
|
||||
#[allow(clippy::needless_return)]
|
||||
fn test_match_with_unreachable(x: bool) -> bool {
|
||||
match x {
|
||||
true => return false,
|
||||
|
|
|
@ -19,49 +19,49 @@ LL | false
|
|||
| ^^^^^ help: add `return` as shown: `return false`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:28:17
|
||||
--> $DIR/implicit_return.rs:27:17
|
||||
|
|
||||
LL | true => false,
|
||||
| ^^^^^ help: add `return` as shown: `return false`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:29:20
|
||||
--> $DIR/implicit_return.rs:28:20
|
||||
|
|
||||
LL | false => { true },
|
||||
| ^^^^ help: add `return` as shown: `return true`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:44:9
|
||||
--> $DIR/implicit_return.rs:43:9
|
||||
|
|
||||
LL | break true;
|
||||
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:52:13
|
||||
--> $DIR/implicit_return.rs:51:13
|
||||
|
|
||||
LL | break true;
|
||||
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:61:13
|
||||
--> $DIR/implicit_return.rs:60:13
|
||||
|
|
||||
LL | break true;
|
||||
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:79:18
|
||||
--> $DIR/implicit_return.rs:78:18
|
||||
|
|
||||
LL | let _ = || { true };
|
||||
| ^^^^ help: add `return` as shown: `return true`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:80:16
|
||||
--> $DIR/implicit_return.rs:79:16
|
||||
|
|
||||
LL | let _ = || true;
|
||||
| ^^^^ help: add `return` as shown: `return true`
|
||||
|
||||
error: missing `return` statement
|
||||
--> $DIR/implicit_return.rs:88:5
|
||||
--> $DIR/implicit_return.rs:87:5
|
||||
|
|
||||
LL | format!("test {}", "test")
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `return` as shown: `return format!("test {}", "test")`
|
||||
|
|
|
@ -1,3 +1,5 @@
|
|||
#![deny(clippy::match_bool)]
|
||||
|
||||
fn match_bool() {
|
||||
let test: bool = true;
|
||||
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
error: this boolean expression can be simplified
|
||||
--> $DIR/match_bool.rs:29:11
|
||||
--> $DIR/match_bool.rs:31:11
|
||||
|
|
||||
LL | match test && test {
|
||||
| ^^^^^^^^^^^^ help: try: `test`
|
||||
|
@ -7,7 +7,7 @@ LL | match test && test {
|
|||
= note: `-D clippy::nonminimal-bool` implied by `-D warnings`
|
||||
|
||||
error: you seem to be trying to match on a boolean expression
|
||||
--> $DIR/match_bool.rs:4:5
|
||||
--> $DIR/match_bool.rs:6:5
|
||||
|
|
||||
LL | / match test {
|
||||
LL | | true => 0,
|
||||
|
@ -15,10 +15,14 @@ LL | | false => 42,
|
|||
LL | | };
|
||||
| |_____^ help: consider using an `if`/`else` expression: `if test { 0 } else { 42 }`
|
||||
|
|
||||
= note: `-D clippy::match-bool` implied by `-D warnings`
|
||||
note: the lint level is defined here
|
||||
--> $DIR/match_bool.rs:1:9
|
||||
|
|
||||
LL | #![deny(clippy::match_bool)]
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: you seem to be trying to match on a boolean expression
|
||||
--> $DIR/match_bool.rs:10:5
|
||||
--> $DIR/match_bool.rs:12:5
|
||||
|
|
||||
LL | / match option == 1 {
|
||||
LL | | true => 1,
|
||||
|
@ -27,7 +31,7 @@ LL | | };
|
|||
| |_____^ help: consider using an `if`/`else` expression: `if option == 1 { 1 } else { 0 }`
|
||||
|
||||
error: you seem to be trying to match on a boolean expression
|
||||
--> $DIR/match_bool.rs:15:5
|
||||
--> $DIR/match_bool.rs:17:5
|
||||
|
|
||||
LL | / match test {
|
||||
LL | | true => (),
|
||||
|
@ -45,7 +49,7 @@ LL | };
|
|||
|
|
||||
|
||||
error: you seem to be trying to match on a boolean expression
|
||||
--> $DIR/match_bool.rs:22:5
|
||||
--> $DIR/match_bool.rs:24:5
|
||||
|
|
||||
LL | / match test {
|
||||
LL | | false => {
|
||||
|
@ -63,7 +67,7 @@ LL | };
|
|||
|
|
||||
|
||||
error: you seem to be trying to match on a boolean expression
|
||||
--> $DIR/match_bool.rs:29:5
|
||||
--> $DIR/match_bool.rs:31:5
|
||||
|
|
||||
LL | / match test && test {
|
||||
LL | | false => {
|
||||
|
@ -81,7 +85,7 @@ LL | };
|
|||
|
|
||||
|
||||
error: equal expressions as operands to `&&`
|
||||
--> $DIR/match_bool.rs:29:11
|
||||
--> $DIR/match_bool.rs:31:11
|
||||
|
|
||||
LL | match test && test {
|
||||
| ^^^^^^^^^^^^
|
||||
|
@ -89,7 +93,7 @@ LL | match test && test {
|
|||
= note: `#[deny(clippy::eq_op)]` on by default
|
||||
|
||||
error: you seem to be trying to match on a boolean expression
|
||||
--> $DIR/match_bool.rs:36:5
|
||||
--> $DIR/match_bool.rs:38:5
|
||||
|
|
||||
LL | / match test {
|
||||
LL | | false => {
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
// run-rustfix
|
||||
|
||||
#![allow(unused, clippy::needless_bool, clippy::match_bool)]
|
||||
#![allow(unused, clippy::needless_bool)]
|
||||
#![allow(clippy::if_same_then_else, clippy::single_match)]
|
||||
#![warn(clippy::needless_return)]
|
||||
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
// run-rustfix
|
||||
|
||||
#![allow(unused, clippy::needless_bool, clippy::match_bool)]
|
||||
#![allow(unused, clippy::needless_bool)]
|
||||
#![allow(clippy::if_same_then_else, clippy::single_match)]
|
||||
#![warn(clippy::needless_return)]
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue