From d4cf288b385b9c58cfa0d4c97b3b548bc21b4923 Mon Sep 17 00:00:00 2001 From: John Quigley Date: Fri, 20 Nov 2015 00:22:52 -0500 Subject: [PATCH] Add block_in_if lint, #434 --- README.md | 156 ++++++++++---------- src/block_in_if_condition.rs | 91 ++++++++++++ src/lib.rs | 4 + tests/compile-fail/block_in_if_condition.rs | 64 ++++++++ 4 files changed, 238 insertions(+), 77 deletions(-) create mode 100644 src/block_in_if_condition.rs create mode 100644 tests/compile-fail/block_in_if_condition.rs diff --git a/README.md b/README.md index b893c4e7a..696836f41 100644 --- a/README.md +++ b/README.md @@ -6,84 +6,86 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 74 lints included in this crate: +There are 76 lints included in this crate: -name | default | meaning --------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ -[approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant) | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant -[bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask) | warn | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have) -[box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec) | warn | usage of `Box>`, vector elements are already on the heap -[cast_possible_truncation](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_truncation) | allow | casts that may cause truncation of the value, e.g `x as u8` where `x: u32`, or `x as i32` where `x: f32` -[cast_possible_wrap](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap) | allow | casts that may cause wrapping around the value, e.g `x as i32` where `x: u32` and `x > i32::MAX` -[cast_precision_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss) | allow | casts that cause loss of precision, e.g `x as f32` where `x: u64` -[cast_sign_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss) | allow | casts from signed types to unsigned types, e.g `x as u32` where `x: i32` -[cmp_nan](https://github.com/Manishearth/rust-clippy/wiki#cmp_nan) | deny | comparisons to NAN (which will always return false, which is probably not intended) -[cmp_owned](https://github.com/Manishearth/rust-clippy/wiki#cmp_owned) | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()` -[collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }` -[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected -[eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`) -[explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do -[explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop) | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do -[float_cmp](https://github.com/Manishearth/rust-clippy/wiki#float_cmp) | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds) -[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1` -[ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask) | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2` -[inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always) | warn | `#[inline(always)]` is a bad idea in most cases -[iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop) | warn | for-looping over `_.next()` which is probably not intended -[len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits and impls that have `.len()` but not `.is_empty()` -[len_zero](https://github.com/Manishearth/rust-clippy/wiki#len_zero) | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead -[let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block -[let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value) | warn | creating a let binding to a value of unit type, which usually can't be used afterwards -[linkedlist](https://github.com/Manishearth/rust-clippy/wiki#linkedlist) | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a VecDeque -[map_clone](https://github.com/Manishearth/rust-clippy/wiki#map_clone) | warn | using `.map(|x| x.clone())` to clone an iterator or option's contents (recommends `.cloned()` instead) -[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead -[match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match has all arms prefixed with `&`; the match expression can be dereferenced instead -[min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant -[modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 -[mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references) -[mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic) | warn | using a Mutex where an atomic value could be used instead -[mutex_integer](https://github.com/Manishearth/rust-clippy/wiki#mutex_integer) | allow | using a Mutex for an integer type -[needless_bool](https://github.com/Manishearth/rust-clippy/wiki#needless_bool) | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }` -[needless_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes) | warn | using explicit lifetimes for references in function arguments when elision rules would allow omitting them -[needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop) | warn | for-looping over a range of indices where an iterator over items would do -[needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return) | warn | using a return statement like `return expr;` where an expression would suffice -[needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update) | warn | using `{ ..base }` when there are no missing fields -[no_effect](https://github.com/Manishearth/rust-clippy/wiki#no_effect) | warn | statements with no effect -[non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal) | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead -[nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options) | warn | nonsensical combination of options for opening a file -[option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used) | allow | using `Option.unwrap()`, which should at least get a better message using `expect()` -[precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence) | warn | catches operations where precedence may be unclear. See the wiki for a list of cases caught -[ptr_arg](https://github.com/Manishearth/rust-clippy/wiki#ptr_arg) | warn | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively -[range_step_by_zero](https://github.com/Manishearth/rust-clippy/wiki#range_step_by_zero) | warn | using Range::step_by(0), which produces an infinite iterator -[range_zip_with_len](https://github.com/Manishearth/rust-clippy/wiki#range_zip_with_len) | warn | zipping iterator with a range when enumerate() would do -[redundant_closure](https://github.com/Manishearth/rust-clippy/wiki#redundant_closure) | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`) -[redundant_pattern](https://github.com/Manishearth/rust-clippy/wiki#redundant_pattern) | warn | using `name @ _` in a pattern -[result_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used) | allow | using `Result.unwrap()`, which might be better handled -[reverse_range_loop](https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop) | warn | Iterating over an empty range, such as `10..0` or `5..5` -[shadow_reuse](https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse) | allow | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1` -[shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x` -[shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | allow | The name is re-bound without even using the original value -[should_implement_trait](https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait) | warn | defining a method that should be implementing a std trait -[single_match](https://github.com/Manishearth/rust-clippy/wiki#single_match) | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead -[str_to_string](https://github.com/Manishearth/rust-clippy/wiki#str_to_string) | warn | using `to_string()` on a str, which should be `to_owned()` -[string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead -[string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign) | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead -[string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String.to_string()` which is a no-op -[temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries -[toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`. -[type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions -[unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc) | allow | using a unicode literal not in NFC normal form (see http://www.unicode.org/reports/tr15/ for further information) -[unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp) | warn | comparing unit values (which is always `true` or `false`, respectively) -[unnecessary_mut_passed](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed) | warn | an argument is passed as a mutable reference although the function/method only demands an immutable reference -[unstable_as_mut_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_mut_slice) | warn | as_mut_slice is not stable and can be replaced by &mut v[..]see https://github.com/rust-lang/rust/issues/27729 -[unstable_as_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice) | warn | as_slice is not stable and can be replaced by & v[..]see https://github.com/rust-lang/rust/issues/27729 -[unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop -[useless_transmute](https://github.com/Manishearth/rust-clippy/wiki#useless_transmute) | warn | transmutes that have the same to and from types -[while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop) | warn | `loop { if let { ... } else break }` can be written as a `while let` loop -[while_let_on_iterator](https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator) | warn | using a while-let loop instead of a for loop on an iterator -[wrong_pub_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_pub_self_convention) | allow | defining a public method named with an established prefix (like "into_") that takes `self` with the wrong convention -[wrong_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_self_convention) | warn | defining a method named with an established prefix (like "into_") that takes `self` with the wrong convention -[zero_divided_by_zero](https://github.com/Manishearth/rust-clippy/wiki#zero_divided_by_zero) | warn | usage of `0.0 / 0.0` to obtain NaN instead of std::f32::NaN or std::f64::NaN -[zero_width_space](https://github.com/Manishearth/rust-clippy/wiki#zero_width_space) | deny | using a zero-width space in a string literal, which is confusing +name | default | meaning +---------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ +[approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant) | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant +[bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask) | warn | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have) +[block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces can be eliminated in conditions that are expressions, e.g `if { true } ...` +[block_in_if_condition_stmt](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt) | warn | avoid complex blocks in conditions, instead move the block higher and bind it with 'let'; e.g: `if { let x = true; x } ...` +[box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec) | warn | usage of `Box>`, vector elements are already on the heap +[cast_possible_truncation](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_truncation) | allow | casts that may cause truncation of the value, e.g `x as u8` where `x: u32`, or `x as i32` where `x: f32` +[cast_possible_wrap](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_wrap) | allow | casts that may cause wrapping around the value, e.g `x as i32` where `x: u32` and `x > i32::MAX` +[cast_precision_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_precision_loss) | allow | casts that cause loss of precision, e.g `x as f32` where `x: u64` +[cast_sign_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss) | allow | casts from signed types to unsigned types, e.g `x as u32` where `x: i32` +[cmp_nan](https://github.com/Manishearth/rust-clippy/wiki#cmp_nan) | deny | comparisons to NAN (which will always return false, which is probably not intended) +[cmp_owned](https://github.com/Manishearth/rust-clippy/wiki#cmp_owned) | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()` +[collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }` +[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected +[eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`) +[explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do +[explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop) | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do +[float_cmp](https://github.com/Manishearth/rust-clippy/wiki#float_cmp) | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds) +[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1` +[ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask) | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2` +[inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always) | warn | `#[inline(always)]` is a bad idea in most cases +[iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop) | warn | for-looping over `_.next()` which is probably not intended +[len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits and impls that have `.len()` but not `.is_empty()` +[len_zero](https://github.com/Manishearth/rust-clippy/wiki#len_zero) | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead +[let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block +[let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value) | warn | creating a let binding to a value of unit type, which usually can't be used afterwards +[linkedlist](https://github.com/Manishearth/rust-clippy/wiki#linkedlist) | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a VecDeque +[map_clone](https://github.com/Manishearth/rust-clippy/wiki#map_clone) | warn | using `.map(|x| x.clone())` to clone an iterator or option's contents (recommends `.cloned()` instead) +[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead +[match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match has all arms prefixed with `&`; the match expression can be dereferenced instead +[min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant +[modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 +[mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references) +[mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic) | warn | using a Mutex where an atomic value could be used instead +[mutex_integer](https://github.com/Manishearth/rust-clippy/wiki#mutex_integer) | allow | using a Mutex for an integer type +[needless_bool](https://github.com/Manishearth/rust-clippy/wiki#needless_bool) | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }` +[needless_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes) | warn | using explicit lifetimes for references in function arguments when elision rules would allow omitting them +[needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop) | warn | for-looping over a range of indices where an iterator over items would do +[needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return) | warn | using a return statement like `return expr;` where an expression would suffice +[needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update) | warn | using `{ ..base }` when there are no missing fields +[no_effect](https://github.com/Manishearth/rust-clippy/wiki#no_effect) | warn | statements with no effect +[non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal) | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead +[nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options) | warn | nonsensical combination of options for opening a file +[option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used) | allow | using `Option.unwrap()`, which should at least get a better message using `expect()` +[precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence) | warn | catches operations where precedence may be unclear. See the wiki for a list of cases caught +[ptr_arg](https://github.com/Manishearth/rust-clippy/wiki#ptr_arg) | warn | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively +[range_step_by_zero](https://github.com/Manishearth/rust-clippy/wiki#range_step_by_zero) | warn | using Range::step_by(0), which produces an infinite iterator +[range_zip_with_len](https://github.com/Manishearth/rust-clippy/wiki#range_zip_with_len) | warn | zipping iterator with a range when enumerate() would do +[redundant_closure](https://github.com/Manishearth/rust-clippy/wiki#redundant_closure) | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`) +[redundant_pattern](https://github.com/Manishearth/rust-clippy/wiki#redundant_pattern) | warn | using `name @ _` in a pattern +[result_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used) | allow | using `Result.unwrap()`, which might be better handled +[reverse_range_loop](https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop) | warn | Iterating over an empty range, such as `10..0` or `5..5` +[shadow_reuse](https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse) | allow | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1` +[shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x` +[shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | allow | The name is re-bound without even using the original value +[should_implement_trait](https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait) | warn | defining a method that should be implementing a std trait +[single_match](https://github.com/Manishearth/rust-clippy/wiki#single_match) | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead +[str_to_string](https://github.com/Manishearth/rust-clippy/wiki#str_to_string) | warn | using `to_string()` on a str, which should be `to_owned()` +[string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead +[string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign) | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead +[string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String.to_string()` which is a no-op +[temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries +[toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`. +[type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions +[unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc) | allow | using a unicode literal not in NFC normal form (see http://www.unicode.org/reports/tr15/ for further information) +[unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp) | warn | comparing unit values (which is always `true` or `false`, respectively) +[unnecessary_mut_passed](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed) | warn | an argument is passed as a mutable reference although the function/method only demands an immutable reference +[unstable_as_mut_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_mut_slice) | warn | as_mut_slice is not stable and can be replaced by &mut v[..]see https://github.com/rust-lang/rust/issues/27729 +[unstable_as_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice) | warn | as_slice is not stable and can be replaced by & v[..]see https://github.com/rust-lang/rust/issues/27729 +[unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop +[useless_transmute](https://github.com/Manishearth/rust-clippy/wiki#useless_transmute) | warn | transmutes that have the same to and from types +[while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop) | warn | `loop { if let { ... } else break }` can be written as a `while let` loop +[while_let_on_iterator](https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator) | warn | using a while-let loop instead of a for loop on an iterator +[wrong_pub_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_pub_self_convention) | allow | defining a public method named with an established prefix (like "into_") that takes `self` with the wrong convention +[wrong_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_self_convention) | warn | defining a method named with an established prefix (like "into_") that takes `self` with the wrong convention +[zero_divided_by_zero](https://github.com/Manishearth/rust-clippy/wiki#zero_divided_by_zero) | warn | usage of `0.0 / 0.0` to obtain NaN instead of std::f32::NaN or std::f64::NaN +[zero_width_space](https://github.com/Manishearth/rust-clippy/wiki#zero_width_space) | deny | using a zero-width space in a string literal, which is confusing More to come, please [file an issue](https://github.com/Manishearth/rust-clippy/issues) if you have ideas! diff --git a/src/block_in_if_condition.rs b/src/block_in_if_condition.rs new file mode 100644 index 000000000..39196ed17 --- /dev/null +++ b/src/block_in_if_condition.rs @@ -0,0 +1,91 @@ +use rustc_front::hir::*; +use rustc::lint::{LateLintPass, LateContext, LintArray, LintPass}; +use rustc_front::intravisit::{Visitor, walk_expr}; +use utils::*; + +declare_lint! { + pub BLOCK_IN_IF_CONDITION_EXPR, Warn, + "braces can be eliminated in conditions that are expressions, e.g `if { true } ...`" +} + +declare_lint! { + pub BLOCK_IN_IF_CONDITION_STMT, Warn, + "avoid complex blocks in conditions, instead move the block higher and bind it \ + with 'let'; e.g: `if { let x = true; x } ...`" +} + +#[derive(Copy,Clone)] +pub struct BlockInIfCondition; + +impl LintPass for BlockInIfCondition { + fn get_lints(&self) -> LintArray { + lint_array!(BLOCK_IN_IF_CONDITION_EXPR, BLOCK_IN_IF_CONDITION_STMT) + } +} + +struct ExVisitor<'v> { + found_block: Option<&'v Expr> +} + +impl<'v> Visitor<'v> for ExVisitor<'v> { + fn visit_expr(&mut self, expr: &'v Expr) { + if let ExprClosure(_, _, ref block) = expr.node { + let complex = { + if !block.stmts.is_empty() { + true + } else { + if let Some(ref ex) = block.expr { + match ex.node { + ExprBlock(_) => true, + _ => false + } + } else { + false + } + } + }; + if complex { + self.found_block = Some(& expr); + return; + } + } + walk_expr(self, expr); + } +} + +const BRACED_EXPR_MESSAGE:&'static str = "omit braces around single expression condition"; +const COMPLEX_BLOCK_MESSAGE:&'static 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'"; + +impl LateLintPass for BlockInIfCondition { + fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + if let ExprIf(ref check, ref then, _) = expr.node { + if let ExprBlock(ref block) = check.node { + if block.stmts.is_empty() { + if let Some(ref ex) = block.expr { + // don't dig into the expression here, just suggest that they remove + // the block + + span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_EXPR, check.span, + BRACED_EXPR_MESSAGE, + &format!("try\nif {} {} ... ", snippet_block(cx, ex.span, ".."), + snippet_block(cx, then.span, ".."))); + } + } else { + // move block higher + span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, check.span, + COMPLEX_BLOCK_MESSAGE, + &format!("try\nlet res = {};\nif res {} ... ", + snippet_block(cx, block.span, ".."), + snippet_block(cx, then.span, ".."))); + } + } else { + let mut visitor = ExVisitor { found_block: None }; + walk_expr(&mut visitor, check); + if let Some(ref block) = visitor.found_block { + span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, + COMPLEX_BLOCK_MESSAGE, ""); + } + } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index afbf3d2a9..9507b1d86 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -37,6 +37,7 @@ pub mod mut_reference; pub mod len_zero; pub mod attrs; pub mod collapsible_if; +pub mod block_in_if_condition; pub mod unicode; pub mod shadow; pub mod strings; @@ -81,6 +82,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box misc::CmpOwned); reg.register_late_lint_pass(box attrs::AttrPass); reg.register_late_lint_pass(box collapsible_if::CollapsibleIf); + reg.register_late_lint_pass(box block_in_if_condition::BlockInIfCondition); reg.register_late_lint_pass(box misc::ModuloOne); reg.register_late_lint_pass(box unicode::Unicode); reg.register_late_lint_pass(box strings::StringAdd); @@ -131,6 +133,8 @@ pub fn plugin_registrar(reg: &mut Registry) { attrs::INLINE_ALWAYS, bit_mask::BAD_BIT_MASK, bit_mask::INEFFECTIVE_BIT_MASK, + block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR, + block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT, collapsible_if::COLLAPSIBLE_IF, eq_op::EQ_OP, eta_reduction::REDUNDANT_CLOSURE, diff --git a/tests/compile-fail/block_in_if_condition.rs b/tests/compile-fail/block_in_if_condition.rs new file mode 100644 index 000000000..c075d4829 --- /dev/null +++ b/tests/compile-fail/block_in_if_condition.rs @@ -0,0 +1,64 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(block_in_if_condition_expr)] +#![deny(block_in_if_condition_stmt)] +#![allow(unused)] + +fn condition_has_block() -> i32 { + + if { //~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' + let x = 3; + x == 3 + } { + 6 + } else { + 10 + } +} + +fn condition_has_block_with_single_expression() -> i32 { + if { true } { //~ERROR omit braces around single expression condition + 6 + } else { + 10 + } +} + +fn predicate 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. same principle applies. add some extra + // expressions to make sure linter isn't confused by them. + if v == 3 && sky == "blue" && predicate(|x| { let target = 3; x == target }, v) { //~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' + + } + + if predicate(|x| { let target = 3; x == target }, v) { //~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' + + } + +} + +fn condition_is_normal() -> i32 { + let x = 3; + if true && x == 3 { + 6 + } else { + 10 + } +} + +fn closure_without_block() { + if predicate(|x| x == 3, 6) { + + } +} + +fn main() { +}