From 810de56079b678caf72e9fa712798da61f11fe77 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 27 Feb 2016 17:57:36 +0100 Subject: [PATCH 1/4] Add a lint about suspiciously formatted `=@` ops For `@` in {`*`, `!`, `-`}. --- README.md | 255 ++++++++++++++++--------------- src/formatting.rs | 66 ++++++++ src/lib.rs | 3 + tests/compile-fail/formatting.rs | 28 ++++ 4 files changed, 225 insertions(+), 127 deletions(-) create mode 100644 src/formatting.rs create mode 100755 tests/compile-fail/formatting.rs diff --git a/README.md b/README.md index 62d6695b24d2..9156db1cbdbc 100644 --- a/README.md +++ b/README.md @@ -8,133 +8,134 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 123 lints included in this crate: - -name | default | meaning ----------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ -[absurd_extreme_comparisons](https://github.com/Manishearth/rust-clippy/wiki#absurd_extreme_comparisons) | warn | a comparison involving a maximum or minimum value involves a case that is always true or always false -[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 } ...` -[bool_comparison](https://github.com/Manishearth/rust-clippy/wiki#bool_comparison) | warn | comparing a variable to a boolean, e.g. `if x == true` -[box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec) | warn | usage of `Box>`, vector elements are already on the heap -[boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using Box where unnecessary -[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` -[char_lit_as_u8](https://github.com/Manishearth/rust-clippy/wiki#char_lit_as_u8) | warn | Casting a character literal to u8 -[chars_next_cmp](https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp) | warn | using `.chars().next()` to check if a string starts with a char -[clone_double_ref](https://github.com/Manishearth/rust-clippy/wiki#clone_double_ref) | warn | using `clone` on `&&T` -[clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#clone_on_copy) | warn | using `clone` on a `Copy` type -[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() }` and an `else { if .. } expression can be collapsed to `else if` -[cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions -[deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver -[derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly -[drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value -[duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore -[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected -[enum_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use) | allow | finds use items that import all variants of an enum -[enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names) | warn | finds enums where all variants share a prefix/postfix -[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`) -[expl_impl_clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy) | warn | implementing `Clone` explicitly on `Copy` types -[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 -[extend_from_slice](https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice) | warn | `.extend_from_slice(_)` is a faster way to extend a Vec by a slice -[filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next) | warn | using `filter(p).next()`, which is more succinctly expressed as `.find(p)` -[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) -[for_kv_map](https://github.com/Manishearth/rust-clippy/wiki#for_kv_map) | warn | looping on a map using `iter` when `keys` or `values` would do -[for_loop_over_option](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option) | warn | for-looping over an `Option`, which is more clearly expressed as an `if let` -[for_loop_over_result](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result) | warn | for-looping over a `Result`, which is more clearly expressed as an `if let` -[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1` -[if_same_then_else](https://github.com/Manishearth/rust-clippy/wiki#if_same_then_else) | warn | if with the same *then* and *else* blocks -[ifs_same_cond](https://github.com/Manishearth/rust-clippy/wiki#ifs_same_cond) | warn | consecutive `ifs` with the same condition -[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 -[invalid_regex](https://github.com/Manishearth/rust-clippy/wiki#invalid_regex) | deny | finds invalid regular expressions in `Regex::new(_)` invocations -[items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements) | warn | finds blocks where an item comes after a statement -[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) -[map_entry](https://github.com/Manishearth/rust-clippy/wiki#map_entry) | warn | use of `contains_key` followed by `insert` on a `HashMap` or `BTreeMap` -[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead -[match_overlapping_arm](https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm) | warn | a match has overlapping arms -[match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match or `if let` has all arms prefixed with `&`; the match expression can be dereferenced instead -[match_same_arms](https://github.com/Manishearth/rust-clippy/wiki#match_same_arms) | warn | `match` with identical arm bodies -[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 -[new_ret_no_self](https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self) | warn | not returning `Self` in a `new` method -[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 -[ok_expect](https://github.com/Manishearth/rust-clippy/wiki#ok_expect) | warn | using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result -[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)` -[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)` -[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()` -[or_fun_call](https://github.com/Manishearth/rust-clippy/wiki#or_fun_call) | warn | using any `*or` method when the `*or_else` would do -[out_of_bounds_indexing](https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing) | deny | out of bound constant indexing -[panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params) | warn | missing parameters in `panic!` -[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 -[print_stdout](https://github.com/Manishearth/rust-clippy/wiki#print_stdout) | allow | printing on stdout -[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 -[regex_macro](https://github.com/Manishearth/rust-clippy/wiki#regex_macro) | warn | finds use of `regex!(_)`, suggests `Regex::new(_)` instead -[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` -[search_is_some](https://github.com/Manishearth/rust-clippy/wiki#search_is_some) | warn | using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()` -[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_char_pattern](https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern) | warn | using a single-character str where a char could be used, e.g. `_.split("x")` -[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 -[single_match_else](https://github.com/Manishearth/rust-clippy/wiki#single_match_else) | allow | a match statement with a two arms where the second arm's pattern is a wildcard; 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_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes) | warn | calling `as_bytes` on a string literal; suggests using a byte string literal instead -[string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String::to_string` which is inefficient -[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 `&`. -[trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex) | warn | finds trivial regular expressions in `Regex::new(_)` invocations -[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 -[unneeded_field_pattern](https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern) | warn | Struct fields are bound to a wildcard instead of using `..` -[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 -[unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes) | warn | unused lifetimes in function definitions -[use_debug](https://github.com/Manishearth/rust-clippy/wiki#use_debug) | allow | use `Debug`-based formatting -[used_underscore_binding](https://github.com/Manishearth/rust-clippy/wiki#used_underscore_binding) | warn | using a binding which is prefixed with an underscore -[useless_format](https://github.com/Manishearth/rust-clippy/wiki#useless_format) | warn | useless use of `format!` -[useless_transmute](https://github.com/Manishearth/rust-clippy/wiki#useless_transmute) | warn | transmutes that have the same to and from types -[useless_vec](https://github.com/Manishearth/rust-clippy/wiki#useless_vec) | warn | useless `vec!` -[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 +There are 125 lints included in this crate: + +name | default | meaning +---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ +[absurd_extreme_comparisons](https://github.com/Manishearth/rust-clippy/wiki#absurd_extreme_comparisons) | warn | a comparison involving a maximum or minimum value involves a case that is always true or always false +[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 } ...` +[bool_comparison](https://github.com/Manishearth/rust-clippy/wiki#bool_comparison) | warn | comparing a variable to a boolean, e.g. `if x == true` +[box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec) | warn | usage of `Box>`, vector elements are already on the heap +[boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using Box where unnecessary +[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` +[char_lit_as_u8](https://github.com/Manishearth/rust-clippy/wiki#char_lit_as_u8) | warn | Casting a character literal to u8 +[chars_next_cmp](https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp) | warn | using `.chars().next()` to check if a string starts with a char +[clone_double_ref](https://github.com/Manishearth/rust-clippy/wiki#clone_double_ref) | warn | using `clone` on `&&T` +[clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#clone_on_copy) | warn | using `clone` on a `Copy` type +[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() }` and an `else { if .. } expression can be collapsed to `else if` +[cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions +[deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver +[derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly +[drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value +[duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore +[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected +[enum_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use) | allow | finds use items that import all variants of an enum +[enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names) | warn | finds enums where all variants share a prefix/postfix +[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`) +[expl_impl_clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy) | warn | implementing `Clone` explicitly on `Copy` types +[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 +[extend_from_slice](https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice) | warn | `.extend_from_slice(_)` is a faster way to extend a Vec by a slice +[filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next) | warn | using `filter(p).next()`, which is more succinctly expressed as `.find(p)` +[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) +[for_kv_map](https://github.com/Manishearth/rust-clippy/wiki#for_kv_map) | warn | looping on a map using `iter` when `keys` or `values` would do +[for_loop_over_option](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option) | warn | for-looping over an `Option`, which is more clearly expressed as an `if let` +[for_loop_over_result](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result) | warn | for-looping over a `Result`, which is more clearly expressed as an `if let` +[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1` +[if_same_then_else](https://github.com/Manishearth/rust-clippy/wiki#if_same_then_else) | warn | if with the same *then* and *else* blocks +[ifs_same_cond](https://github.com/Manishearth/rust-clippy/wiki#ifs_same_cond) | warn | consecutive `ifs` with the same condition +[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 +[invalid_regex](https://github.com/Manishearth/rust-clippy/wiki#invalid_regex) | deny | finds invalid regular expressions in `Regex::new(_)` invocations +[items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements) | warn | finds blocks where an item comes after a statement +[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) +[map_entry](https://github.com/Manishearth/rust-clippy/wiki#map_entry) | warn | use of `contains_key` followed by `insert` on a `HashMap` or `BTreeMap` +[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead +[match_overlapping_arm](https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm) | warn | a match has overlapping arms +[match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match or `if let` has all arms prefixed with `&`; the match expression can be dereferenced instead +[match_same_arms](https://github.com/Manishearth/rust-clippy/wiki#match_same_arms) | warn | `match` with identical arm bodies +[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 +[new_ret_no_self](https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self) | warn | not returning `Self` in a `new` method +[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 +[ok_expect](https://github.com/Manishearth/rust-clippy/wiki#ok_expect) | warn | using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result +[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)` +[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)` +[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()` +[or_fun_call](https://github.com/Manishearth/rust-clippy/wiki#or_fun_call) | warn | using any `*or` method when the `*or_else` would do +[out_of_bounds_indexing](https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing) | deny | out of bound constant indexing +[panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params) | warn | missing parameters in `panic!` +[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 +[print_stdout](https://github.com/Manishearth/rust-clippy/wiki#print_stdout) | allow | printing on stdout +[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 +[regex_macro](https://github.com/Manishearth/rust-clippy/wiki#regex_macro) | warn | finds use of `regex!(_)`, suggests `Regex::new(_)` instead +[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` +[search_is_some](https://github.com/Manishearth/rust-clippy/wiki#search_is_some) | warn | using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()` +[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_char_pattern](https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern) | warn | using a single-character str where a char could be used, e.g. `_.split("x")` +[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 +[single_match_else](https://github.com/Manishearth/rust-clippy/wiki#single_match_else) | allow | a match statement with a two arms where the second arm's pattern is a wildcard; 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_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes) | warn | calling `as_bytes` on a string literal; suggests using a byte string literal instead +[string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String::to_string` which is inefficient +[suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn | suspicious formatting of `*=`, `-=` or `!=` +[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 `&`. +[trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex) | warn | finds trivial regular expressions in `Regex::new(_)` invocations +[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 +[unneeded_field_pattern](https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern) | warn | Struct fields are bound to a wildcard instead of using `..` +[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 +[unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes) | warn | unused lifetimes in function definitions +[use_debug](https://github.com/Manishearth/rust-clippy/wiki#use_debug) | allow | use `Debug`-based formatting +[used_underscore_binding](https://github.com/Manishearth/rust-clippy/wiki#used_underscore_binding) | warn | using a binding which is prefixed with an underscore +[useless_format](https://github.com/Manishearth/rust-clippy/wiki#useless_format) | warn | useless use of `format!` +[useless_transmute](https://github.com/Manishearth/rust-clippy/wiki#useless_transmute) | warn | transmutes that have the same to and from types +[useless_vec](https://github.com/Manishearth/rust-clippy/wiki#useless_vec) | warn | useless `vec!` +[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/formatting.rs b/src/formatting.rs new file mode 100644 index 000000000000..ce3c6f45a225 --- /dev/null +++ b/src/formatting.rs @@ -0,0 +1,66 @@ +use rustc::lint::*; +use syntax::codemap::mk_sp; +use syntax::ast; +use utils::{differing_macro_contexts, in_macro, snippet_opt, span_note_and_lint}; +use syntax::ptr::P; + +/// **What it does:** This lint looks for use of the non-existent `=*`, `=!` and `=-` operators. +/// +/// **Why is this bad?** This either a typo of `*=`, `!=` or `-=` or confusing. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust,ignore +/// a =- 42; // confusing, should it be `a -= 42` or `a = -42`? +/// ``` +declare_lint! { + pub SUSPICIOUS_ASSIGNMENT_FORMATTING, + Warn, + "suspicious formatting of `*=`, `-=` or `!=`" +} + +#[derive(Copy,Clone)] +pub struct Formatting; + +impl LintPass for Formatting { + fn get_lints(&self) -> LintArray { + lint_array![SUSPICIOUS_ASSIGNMENT_FORMATTING] + } +} + +impl EarlyLintPass for Formatting { + fn check_expr(&mut self, cx: &EarlyContext, expr: &ast::Expr) { + check_assign(cx, expr); + } +} + +/// Implementation of the SUSPICIOUS_ASSIGNMENT_FORMATTING lint. +fn check_assign(cx: &EarlyContext, expr: &ast::Expr) { + if let ast::ExprKind::Assign(ref lhs, ref rhs) = expr.node { + if !differing_macro_contexts(lhs.span, rhs.span) && !in_macro(cx, lhs.span) { + let eq_span = mk_sp(lhs.span.hi, rhs.span.lo); + + if let Some((sub_rhs, op)) = check_unop(rhs) { + if let Some(eq_snippet) = snippet_opt(cx, eq_span) { + let eqop_span = mk_sp(lhs.span.hi, sub_rhs.span.lo); + if eq_snippet.ends_with('=') { + span_note_and_lint(cx, + SUSPICIOUS_ASSIGNMENT_FORMATTING, + eqop_span, + &format!("this looks like you are trying to use `.. {op}= ..`, but you really are doing `.. = ({op} ..)`", op=op), + eqop_span, + &format!("to remove this lint, use either `{op}=` or `= {op}`", op=op)); + } + } + } + } + } +} + +fn check_unop(expr: &ast::Expr) -> Option<(&P, &'static str)> { + match expr.node { + ast::ExprKind::Unary(op, ref expr) => Some((expr, ast::UnOp::to_string(op))), + _ => None, + } +} diff --git a/src/lib.rs b/src/lib.rs index 8348eb09834b..61029138fc7e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,6 +57,7 @@ pub mod eq_op; pub mod escape; pub mod eta_reduction; pub mod format; +pub mod formatting; pub mod identity_op; pub mod items_after_statements; pub mod len_zero; @@ -165,6 +166,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box regex::RegexPass::default()); reg.register_late_lint_pass(box copies::CopyAndPaste); reg.register_late_lint_pass(box format::FormatMacLint); + reg.register_early_lint_pass(box formatting::Formatting); reg.register_lint_group("clippy_pedantic", vec![ enum_glob_use::ENUM_GLOB_USE, @@ -212,6 +214,7 @@ pub fn plugin_registrar(reg: &mut Registry) { escape::BOXED_LOCAL, eta_reduction::REDUNDANT_CLOSURE, format::USELESS_FORMAT, + formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, identity_op::IDENTITY_OP, items_after_statements::ITEMS_AFTER_STATEMENTS, len_zero::LEN_WITHOUT_IS_EMPTY, diff --git a/tests/compile-fail/formatting.rs b/tests/compile-fail/formatting.rs new file mode 100755 index 000000000000..c2d0f54f906b --- /dev/null +++ b/tests/compile-fail/formatting.rs @@ -0,0 +1,28 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(clippy)] +#![allow(unused_variables)] +#![allow(unused_assignments)] +#![allow(if_same_then_else)] + +fn main() { + // weird op_eq formatting: + let mut a = 42; + a =- 35; + //~^ ERROR this looks like you are trying to use `.. -= ..`, but you really are doing `.. = (- ..)` + //~| NOTE to remove this lint, use either `-=` or `= -` + a =* &191; + //~^ ERROR this looks like you are trying to use `.. *= ..`, but you really are doing `.. = (* ..)` + //~| NOTE to remove this lint, use either `*=` or `= *` + + let mut b = true; + b =! false; + //~^ ERROR this looks like you are trying to use `.. != ..`, but you really are doing `.. = (! ..)` + //~| NOTE to remove this lint, use either `!=` or `= !` + + // those are ok: + a = -35; + a = *&191; + b = !false; +} From 1c3cce8ba586c634d70a304d7e1a0da58b35bbea Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 27 Feb 2016 17:59:04 +0100 Subject: [PATCH 2/4] Add a lint about suspiciously formatted `else if` --- README.md | 1 + src/formatting.rs | 110 ++++++++++++++++++++++++++++++- src/lib.rs | 1 + tests/compile-fail/formatting.rs | 48 ++++++++++++++ 4 files changed, 158 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9156db1cbdbc..c9730c4c58b6 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,7 @@ name [string_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes) | warn | calling `as_bytes` on a string literal; suggests using a byte string literal instead [string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String::to_string` which is inefficient [suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn | suspicious formatting of `*=`, `-=` or `!=` +[suspicious_else_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting) | warn | suspicious formatting of `else if` [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 `&`. [trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex) | warn | finds trivial regular expressions in `Regex::new(_)` invocations diff --git a/src/formatting.rs b/src/formatting.rs index ce3c6f45a225..f82aa7d8188e 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -6,7 +6,7 @@ use syntax::ptr::P; /// **What it does:** This lint looks for use of the non-existent `=*`, `=!` and `=-` operators. /// -/// **Why is this bad?** This either a typo of `*=`, `!=` or `-=` or confusing. +/// **Why is this bad?** This is either a typo of `*=`, `!=` or `-=` or confusing. /// /// **Known problems:** None. /// @@ -20,18 +20,65 @@ declare_lint! { "suspicious formatting of `*=`, `-=` or `!=`" } +/// **What it does:** This lint checks for formatting of `else if`. It lints if the `else` and `if` +/// are not on the same line or the `else` seems to be missing. +/// +/// **Why is this bad?** This is probably some refactoring remnant, even if the code is correct, it +/// might look confusing. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust,ignore +/// if foo { +/// } if bar { // looks like an `else` is missing here +/// } +/// +/// if foo { +/// } else +/// +/// if bar { // this is the `else` block of the previous `if`, but should it be? +/// } +/// ``` +declare_lint! { + pub SUSPICIOUS_ELSE_FORMATTING, + Warn, + "suspicious formatting of `else if`" +} + #[derive(Copy,Clone)] pub struct Formatting; impl LintPass for Formatting { fn get_lints(&self) -> LintArray { - lint_array![SUSPICIOUS_ASSIGNMENT_FORMATTING] + lint_array![SUSPICIOUS_ASSIGNMENT_FORMATTING, SUSPICIOUS_ELSE_FORMATTING] } } impl EarlyLintPass for Formatting { + fn check_block(&mut self, cx: &EarlyContext, block: &ast::Block) { + for w in block.stmts.windows(2) { + match (&w[0].node, &w[1].node) { + (&ast::StmtKind::Expr(ref first, _), &ast::StmtKind::Expr(ref second, _)) | + (&ast::StmtKind::Expr(ref first, _), &ast::StmtKind::Semi(ref second, _)) => { + check_consecutive_ifs(cx, first, second); + } + _ => (), + } + } + + if let Some(ref expr) = block.expr { + if let Some(ref stmt) = block.stmts.iter().last() { + if let ast::StmtKind::Expr(ref first, _) = stmt.node { + check_consecutive_ifs(cx, first, expr); + } + } + } + } + fn check_expr(&mut self, cx: &EarlyContext, expr: &ast::Expr) { check_assign(cx, expr); + check_else_if(cx, expr); } } @@ -64,3 +111,62 @@ fn check_unop(expr: &ast::Expr) -> Option<(&P, &'static str)> { _ => None, } } + +/// Implementation of the SUSPICIOUS_ELSE_FORMATTING lint for weird `else if`. +fn check_else_if(cx: &EarlyContext, expr: &ast::Expr) { + if let Some((then, &Some(ref else_))) = unsugar_if(expr) { + if unsugar_if(else_).is_some() && + !differing_macro_contexts(then.span, else_.span) && + !in_macro(cx, then.span) { + // this will be a span from the closing ‘}’ of the “then” block (excluding) to the + // “if” of the “else if” block (excluding) + let else_span = mk_sp(then.span.hi, else_.span.lo); + + // the snippet should look like " else \n " with maybe comments anywhere + // it’s bad when there is a ‘\n’ after the “else” + if let Some(else_snippet) = snippet_opt(cx, else_span) { + let else_pos = else_snippet.find("else").expect("there must be a `else` here"); + + if else_snippet[else_pos..].contains('\n') { + span_note_and_lint(cx, + SUSPICIOUS_ELSE_FORMATTING, + else_span, + "this is an `else if` but the formatting might hide it", + else_span, + "to remove this lint, remove the `else` or remove the new line between `else` and `if`"); + } + } + } + } +} + +/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for consecutive ifs. +fn check_consecutive_ifs(cx: &EarlyContext, first: &ast::Expr, second: &ast::Expr) { + if !differing_macro_contexts(first.span, second.span) && + !in_macro(cx, first.span) && + unsugar_if(first).is_some() && + unsugar_if(second).is_some() { + // where the else would be + let else_span = mk_sp(first.span.hi, second.span.lo); + + if let Some(else_snippet) = snippet_opt(cx, else_span) { + if !else_snippet.contains('\n') { + span_note_and_lint(cx, + SUSPICIOUS_ELSE_FORMATTING, + else_span, + "this looks like an `else if` but the `else` is missing", + else_span, + "to remove this lint, add the missing `else` or add a new line before the second `if`"); + } + } + } +} + +/// Match `if` or `else if` expressions and return the `then` and `else` block. +fn unsugar_if(expr: &ast::Expr) -> Option<(&P, &Option>)>{ + match expr.node { + ast::ExprKind::If(_, ref then, ref else_) | + ast::ExprKind::IfLet(_, _, ref then, ref else_) => Some((then, else_)), + _ => None, + } +} diff --git a/src/lib.rs b/src/lib.rs index 61029138fc7e..47d9fc6f24c5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -215,6 +215,7 @@ pub fn plugin_registrar(reg: &mut Registry) { eta_reduction::REDUNDANT_CLOSURE, format::USELESS_FORMAT, formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, + formatting::SUSPICIOUS_ELSE_FORMATTING, identity_op::IDENTITY_OP, items_after_statements::ITEMS_AFTER_STATEMENTS, len_zero::LEN_WITHOUT_IS_EMPTY, diff --git a/tests/compile-fail/formatting.rs b/tests/compile-fail/formatting.rs index c2d0f54f906b..14a23111ec7e 100755 --- a/tests/compile-fail/formatting.rs +++ b/tests/compile-fail/formatting.rs @@ -6,7 +6,55 @@ #![allow(unused_assignments)] #![allow(if_same_then_else)] +fn foo() -> bool { true } + fn main() { + // weird `else if` formatting: + if foo() { + } if foo() { //~ERROR this looks like an `else if` but the `else` is missing + } + + let _ = { + if foo() { + } if foo() { //~ERROR this looks like an `else if` but the `else` is missing + } + else { + } + }; + + if foo() { + } else //~ERROR this is an `else if` but the formatting might hide it + if foo() { // the span of the above error should continue here + } + + if foo() { + } //~ERROR this is an `else if` but the formatting might hide it + else + if foo() { // the span of the above error should continue here + } + + // those are ok: + if foo() { + } + if foo() { + } + + if foo() { + } else if foo() { + } + + if foo() { + } + else if foo() { + } + + if foo() { + } + + else if + + foo() {} + // weird op_eq formatting: let mut a = 42; a =- 35; From 3a5b9a707c786bf02ed05d65ca8394d77df601fb Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 27 Feb 2016 18:05:50 +0100 Subject: [PATCH 3/4] Fix (new?) rustc warnings --- src/consts.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index cf32a9a2c825..64f45d72c12e 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -206,7 +206,6 @@ fn lit_to_constant(lit: &LitKind) -> Constant { } fn constant_not(o: Constant) -> Option { - use syntax::ast::LitIntType::*; use self::Constant::*; match o { Bool(b) => Some(Bool(!b)), @@ -232,7 +231,6 @@ fn constant_not(o: Constant) -> Option { } fn constant_negate(o: Constant) -> Option { - use syntax::ast::LitIntType::*; use self::Constant::*; match o { Int(value, LitIntType::Signed(ity), sign) => Some(Int(value, LitIntType::Signed(ity), neg_sign(sign))), From 05178c92b900caeee17bfc4acbaf6baf6600f089 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 27 Feb 2016 18:14:37 +0100 Subject: [PATCH 4/4] Cleanup --- src/formatting.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index f82aa7d8188e..efcb222ebed8 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -88,8 +88,9 @@ fn check_assign(cx: &EarlyContext, expr: &ast::Expr) { if !differing_macro_contexts(lhs.span, rhs.span) && !in_macro(cx, lhs.span) { let eq_span = mk_sp(lhs.span.hi, rhs.span.lo); - if let Some((sub_rhs, op)) = check_unop(rhs) { + if let ast::ExprKind::Unary(op, ref sub_rhs) = rhs.node { if let Some(eq_snippet) = snippet_opt(cx, eq_span) { + let op = ast::UnOp::to_string(op); let eqop_span = mk_sp(lhs.span.hi, sub_rhs.span.lo); if eq_snippet.ends_with('=') { span_note_and_lint(cx, @@ -105,13 +106,6 @@ fn check_assign(cx: &EarlyContext, expr: &ast::Expr) { } } -fn check_unop(expr: &ast::Expr) -> Option<(&P, &'static str)> { - match expr.node { - ast::ExprKind::Unary(op, ref expr) => Some((expr, ast::UnOp::to_string(op))), - _ => None, - } -} - /// Implementation of the SUSPICIOUS_ELSE_FORMATTING lint for weird `else if`. fn check_else_if(cx: &EarlyContext, expr: &ast::Expr) { if let Some((then, &Some(ref else_))) = unsugar_if(expr) {