[package]
name = "clippy"
-version = "0.0.33"
+version = "0.0.35"
authors = [
"Manish Goregaokar <manishsmail@gmail.com>",
"Andre Bogus <bogusandre@gmail.com>",
plugin = true
[dependencies]
-unicode-normalization = "*"
+unicode-normalization = "0.1"
+semver = "0.2.1"
[dev-dependencies]
compiletest_rs = "0.0.11"
[features]
-structured_logging = []
debugging = []
[Jump to usage instructions](#usage)
##Lints
-There are 86 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)
-[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<Vec<T>>`, vector elements are already on the heap
-[boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using Box<T> 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`
-[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() }`
-[cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions
-[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
-[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()`
-[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
-[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
-[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
-[used_underscore_binding](https://github.com/Manishearth/rust-clippy/wiki#used_underscore_binding) | warn | using a binding which is prefixed with an underscore
-[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
+There are 92 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)
+[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<Vec<T>>`, vector elements are already on the heap
+[boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using Box<T> 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`
+[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() }`
+[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
+[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
+[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
+[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)
+[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)
+[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
+[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
+[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()`
+[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
+[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`
+[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_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
+[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
+[used_underscore_binding](https://github.com/Manishearth/rust-clippy/wiki#used_underscore_binding) | warn | using a binding which is prefixed with an underscore
+[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!
--- /dev/null
+max_width = 120
+ideal_width = 100
+fn_args_density = "Compressed"
+fn_call_width = 80
+fn_args_paren_newline = false
\ No newline at end of file
}
// Tuples are of the form (constant, name, min_digits)
-const KNOWN_CONSTS : &'static [(f64, &'static str, usize)] = &[
- (f64::E, "E", 4),
- (f64::FRAC_1_PI, "FRAC_1_PI", 4),
- (f64::FRAC_1_SQRT_2, "FRAC_1_SQRT_2", 5),
- (f64::FRAC_2_PI, "FRAC_2_PI", 5),
- (f64::FRAC_2_SQRT_PI, "FRAC_2_SQRT_PI", 5),
- (f64::FRAC_PI_2, "FRAC_PI_2", 5),
- (f64::FRAC_PI_3, "FRAC_PI_3", 5),
- (f64::FRAC_PI_4, "FRAC_PI_4", 5),
- (f64::FRAC_PI_6, "FRAC_PI_6", 5),
- (f64::FRAC_PI_8, "FRAC_PI_8", 5),
- (f64::LN_10, "LN_10", 5),
- (f64::LN_2, "LN_2", 5),
- (f64::LOG10_E, "LOG10_E", 5),
- (f64::LOG2_E, "LOG2_E", 5),
- (f64::PI, "PI", 3),
- (f64::SQRT_2, "SQRT_2", 5),
-];
+const KNOWN_CONSTS: &'static [(f64, &'static str, usize)] = &[(f64::E, "E", 4),
+ (f64::FRAC_1_PI, "FRAC_1_PI", 4),
+ (f64::FRAC_1_SQRT_2, "FRAC_1_SQRT_2", 5),
+ (f64::FRAC_2_PI, "FRAC_2_PI", 5),
+ (f64::FRAC_2_SQRT_PI, "FRAC_2_SQRT_PI", 5),
+ (f64::FRAC_PI_2, "FRAC_PI_2", 5),
+ (f64::FRAC_PI_3, "FRAC_PI_3", 5),
+ (f64::FRAC_PI_4, "FRAC_PI_4", 5),
+ (f64::FRAC_PI_6, "FRAC_PI_6", 5),
+ (f64::FRAC_PI_8, "FRAC_PI_8", 5),
+ (f64::LN_10, "LN_10", 5),
+ (f64::LN_2, "LN_2", 5),
+ (f64::LOG10_E, "LOG10_E", 5),
+ (f64::LOG2_E, "LOG2_E", 5),
+ (f64::PI, "PI", 3),
+ (f64::SQRT_2, "SQRT_2", 5)];
#[derive(Copy,Clone)]
pub struct ApproxConstant;
match lit.node {
LitFloat(ref s, TyF32) => check_known_consts(cx, e, s, "f32"),
LitFloat(ref s, TyF64) => check_known_consts(cx, e, s, "f64"),
- LitFloatUnsuffixed(ref s) =>
- check_known_consts(cx, e, s, "f{32, 64}"),
- _ => ()
+ LitFloatUnsuffixed(ref s) => check_known_consts(cx, e, s, "f{32, 64}"),
+ _ => (),
}
}
if let Ok(_) = s.parse::<f64>() {
for &(constant, name, min_digits) in KNOWN_CONSTS {
if is_approx_const(constant, s, min_digits) {
- span_lint(cx, APPROX_CONSTANT, e.span, &format!(
- "approximate value of `{}::{}` found. \
- Consider using it directly", module, &name));
+ span_lint(cx,
+ APPROX_CONSTANT,
+ e.span,
+ &format!("approximate value of `{}::{}` found. Consider using it directly", module, &name));
return;
}
}
let index = eval_const_expr_partial(cx.tcx, &index, ExprTypeChecked, None);
if let Ok(ConstVal::Uint(index)) = index {
if size as u64 <= index {
- span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span,
- "const index-expr is out of bounds");
+ span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "const index-expr is out of bounds");
}
}
}
use rustc::lint::*;
use rustc_front::hir::*;
use reexport::*;
+use semver::Version;
use syntax::codemap::Span;
use syntax::attr::*;
-use syntax::ast::{Attribute, MetaList, MetaWord};
+use syntax::ast::{Attribute, Lit, Lit_, MetaList, MetaWord, MetaNameValue};
use utils::{in_macro, match_path, span_lint, BEGIN_UNWIND};
-/// **What it does:** This lint warns on items annotated with `#[inline(always)]`, unless the annotated function is empty or simply panics.
+/// **What it does:** This lint `Warn`s on items annotated with `#[inline(always)]`, unless the annotated function is empty or simply panics.
///
/// **Why is this bad?** While there are valid uses of this annotation (and once you know when to use it, by all means `allow` this lint), it's a common newbie-mistake to pepper one's code with it.
///
declare_lint! { pub INLINE_ALWAYS, Warn,
"`#[inline(always)]` is a bad idea in most cases" }
+/// **What it does:** This lint `Warn`s on `#[deprecated]` annotations with a `since` field that is not a valid semantic version..
+///
+/// **Why is this bad?** For checking the version of the deprecation, it must be valid semver. Failing that, the contained information is useless.
+///
+/// **Known problems:** None
+///
+/// **Example:**
+/// ```
+/// #[deprecated(since = "forever")]
+/// fn something_else(..) { ... }
+/// ```
+declare_lint! { pub DEPRECATED_SEMVER, Warn,
+ "`Warn` on `#[deprecated(since = \"x\")]` where x is not semver" }
#[derive(Copy,Clone)]
pub struct AttrPass;
impl LintPass for AttrPass {
fn get_lints(&self) -> LintArray {
- lint_array!(INLINE_ALWAYS)
+ lint_array!(INLINE_ALWAYS, DEPRECATED_SEMVER)
}
}
impl LateLintPass for AttrPass {
+ fn check_attribute(&mut self, cx: &LateContext, attr: &Attribute) {
+ if let MetaList(ref name, ref items) = attr.node.value.node {
+ if items.is_empty() || name != &"deprecated" {
+ return;
+ }
+ for ref item in items {
+ if let MetaNameValue(ref name, ref lit) = item.node {
+ if name == &"since" {
+ check_semver(cx, item.span, lit);
+ }
+ }
+ }
+ }
+ }
+
fn check_item(&mut self, cx: &LateContext, item: &Item) {
if is_relevant_item(item) {
check_attrs(cx, item.span, &item.name, &item.attrs)
fn is_relevant_item(item: &Item) -> bool {
if let ItemFn(_, _, _, _, _, ref block) = item.node {
is_relevant_block(block)
- } else { false }
+ } else {
+ false
+ }
}
fn is_relevant_impl(item: &ImplItem) -> bool {
match item.node {
ImplItemKind::Method(_, ref block) => is_relevant_block(block),
- _ => false
+ _ => false,
}
}
match item.node {
MethodTraitItem(_, None) => true,
MethodTraitItem(_, Some(ref block)) => is_relevant_block(block),
- _ => false
+ _ => false,
}
}
ExprCall(ref path_expr, _) => {
if let ExprPath(_, ref path) = path_expr.node {
!match_path(path, &BEGIN_UNWIND)
- } else { true }
+ } else {
+ true
+ }
}
- _ => true
+ _ => true,
}
}
-fn check_attrs(cx: &LateContext, span: Span, name: &Name,
- attrs: &[Attribute]) {
- if in_macro(cx, span) { return; }
+fn check_attrs(cx: &LateContext, span: Span, name: &Name, attrs: &[Attribute]) {
+ if in_macro(cx, span) {
+ return;
+ }
for attr in attrs {
if let MetaList(ref inline, ref values) = attr.node.value.node {
- if values.len() != 1 || inline != &"inline" { continue; }
+ if values.len() != 1 || inline != &"inline" {
+ continue;
+ }
if let MetaWord(ref always) = values[0].node {
- if always != &"always" { continue; }
- span_lint(cx, INLINE_ALWAYS, attr.span, &format!(
- "you have declared `#[inline(always)]` on `{}`. This \
- is usually a bad idea",
- name));
+ if always != &"always" {
+ continue;
+ }
+ span_lint(cx,
+ INLINE_ALWAYS,
+ attr.span,
+ &format!("you have declared `#[inline(always)]` on `{}`. This is usually a bad idea",
+ name));
}
}
}
}
+
+fn check_semver(cx: &LateContext, span: Span, lit: &Lit) {
+ if let Lit_::LitStr(ref is, _) = lit.node {
+ if Version::parse(&*is).is_ok() {
+ return;
+ }
+ }
+ span_lint(cx,
+ DEPRECATED_SEMVER,
+ span,
+ "the since field must contain a semver-compliant version");
+}
fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
if let ExprBinary(ref cmp, ref left, ref right) = e.node {
if is_comparison_binop(cmp.node) {
- fetch_int_literal(cx, right).map_or_else(||
- fetch_int_literal(cx, left).map_or((), |cmp_val|
- check_compare(cx, right, invert_cmp(cmp.node),
- cmp_val, &e.span)),
- |cmp_opt| check_compare(cx, left, cmp.node, cmp_opt,
- &e.span))
+ fetch_int_literal(cx, right).map_or_else(|| {
+ fetch_int_literal(cx, left).map_or((), |cmp_val| {
+ check_compare(cx,
+ right,
+ invert_cmp(cmp.node),
+ cmp_val,
+ &e.span)
+ })
+ },
+ |cmp_opt| check_compare(cx, left, cmp.node, cmp_opt, &e.span))
}
}
}
}
-fn invert_cmp(cmp : BinOp_) -> BinOp_ {
+fn invert_cmp(cmp: BinOp_) -> BinOp_ {
match cmp {
BiEq => BiEq,
BiNe => BiNe,
BiGt => BiLt,
BiLe => BiGe,
BiGe => BiLe,
- _ => BiOr // Dummy
+ _ => BiOr, // Dummy
}
}
if op.node != BiBitAnd && op.node != BiBitOr {
return;
}
- fetch_int_literal(cx, right).or_else(|| fetch_int_literal(
- cx, left)).map_or((), |mask| check_bit_mask(cx, op.node,
- cmp_op, mask, cmp_value, span))
+ fetch_int_literal(cx, right)
+ .or_else(|| fetch_int_literal(cx, left))
+ .map_or((), |mask| check_bit_mask(cx, op.node, cmp_op, mask, cmp_value, span))
}
}
-fn check_bit_mask(cx: &LateContext, bit_op: BinOp_, cmp_op: BinOp_,
- mask_value: u64, cmp_value: u64, span: &Span) {
+fn check_bit_mask(cx: &LateContext, bit_op: BinOp_, cmp_op: BinOp_, mask_value: u64, cmp_value: u64, span: &Span) {
match cmp_op {
- BiEq | BiNe => match bit_op {
- BiBitAnd => if mask_value & cmp_value != cmp_value {
- if cmp_value != 0 {
- span_lint(cx, BAD_BIT_MASK, *span, &format!(
- "incompatible bit mask: `_ & {}` can never be equal to `{}`",
- mask_value, cmp_value));
+ BiEq | BiNe => {
+ match bit_op {
+ BiBitAnd => {
+ if mask_value & cmp_value != cmp_value {
+ if cmp_value != 0 {
+ span_lint(cx,
+ BAD_BIT_MASK,
+ *span,
+ &format!("incompatible bit mask: `_ & {}` can never be equal to `{}`",
+ mask_value,
+ cmp_value));
+ }
+ } else {
+ if mask_value == 0 {
+ span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
+ }
+ }
}
- } else {
- if mask_value == 0 {
- span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
+ BiBitOr => {
+ if mask_value | cmp_value != cmp_value {
+ span_lint(cx,
+ BAD_BIT_MASK,
+ *span,
+ &format!("incompatible bit mask: `_ | {}` can never be equal to `{}`",
+ mask_value,
+ cmp_value));
+ }
}
- },
- BiBitOr => if mask_value | cmp_value != cmp_value {
- span_lint(cx, BAD_BIT_MASK, *span, &format!(
- "incompatible bit mask: `_ | {}` can never be equal to `{}`",
- mask_value, cmp_value));
- },
- _ => ()
- },
- BiLt | BiGe => match bit_op {
- BiBitAnd => if mask_value < cmp_value {
- span_lint(cx, BAD_BIT_MASK, *span, &format!(
- "incompatible bit mask: `_ & {}` will always be lower than `{}`",
- mask_value, cmp_value));
- } else {
- if mask_value == 0 {
- span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
+ _ => (),
+ }
+ }
+ BiLt | BiGe => {
+ match bit_op {
+ BiBitAnd => {
+ if mask_value < cmp_value {
+ span_lint(cx,
+ BAD_BIT_MASK,
+ *span,
+ &format!("incompatible bit mask: `_ & {}` will always be lower than `{}`",
+ mask_value,
+ cmp_value));
+ } else {
+ if mask_value == 0 {
+ span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
+ }
+ }
}
- },
- BiBitOr => if mask_value >= cmp_value {
- span_lint(cx, BAD_BIT_MASK, *span, &format!(
- "incompatible bit mask: `_ | {}` will never be lower than `{}`",
- mask_value, cmp_value));
- } else {
- check_ineffective_lt(cx, *span, mask_value, cmp_value, "|");
- },
- BiBitXor =>
- check_ineffective_lt(cx, *span, mask_value, cmp_value, "^"),
- _ => ()
- },
- BiLe | BiGt => match bit_op {
- BiBitAnd => if mask_value <= cmp_value {
- span_lint(cx, BAD_BIT_MASK, *span, &format!(
- "incompatible bit mask: `_ & {}` will never be higher than `{}`",
- mask_value, cmp_value));
- } else {
- if mask_value == 0 {
- span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
+ BiBitOr => {
+ if mask_value >= cmp_value {
+ span_lint(cx,
+ BAD_BIT_MASK,
+ *span,
+ &format!("incompatible bit mask: `_ | {}` will never be lower than `{}`",
+ mask_value,
+ cmp_value));
+ } else {
+ check_ineffective_lt(cx, *span, mask_value, cmp_value, "|");
+ }
}
- },
- BiBitOr => if mask_value > cmp_value {
- span_lint(cx, BAD_BIT_MASK, *span, &format!(
- "incompatible bit mask: `_ | {}` will always be higher than `{}`",
- mask_value, cmp_value));
- } else {
- check_ineffective_gt(cx, *span, mask_value, cmp_value, "|");
- },
- BiBitXor =>
- check_ineffective_gt(cx, *span, mask_value, cmp_value, "^"),
- _ => ()
- },
- _ => ()
+ BiBitXor => check_ineffective_lt(cx, *span, mask_value, cmp_value, "^"),
+ _ => (),
+ }
+ }
+ BiLe | BiGt => {
+ match bit_op {
+ BiBitAnd => {
+ if mask_value <= cmp_value {
+ span_lint(cx,
+ BAD_BIT_MASK,
+ *span,
+ &format!("incompatible bit mask: `_ & {}` will never be higher than `{}`",
+ mask_value,
+ cmp_value));
+ } else {
+ if mask_value == 0 {
+ span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero");
+ }
+ }
+ }
+ BiBitOr => {
+ if mask_value > cmp_value {
+ span_lint(cx,
+ BAD_BIT_MASK,
+ *span,
+ &format!("incompatible bit mask: `_ | {}` will always be higher than `{}`",
+ mask_value,
+ cmp_value));
+ } else {
+ check_ineffective_gt(cx, *span, mask_value, cmp_value, "|");
+ }
+ }
+ BiBitXor => check_ineffective_gt(cx, *span, mask_value, cmp_value, "^"),
+ _ => (),
+ }
+ }
+ _ => (),
}
}
fn check_ineffective_lt(cx: &LateContext, span: Span, m: u64, c: u64, op: &str) {
if c.is_power_of_two() && m < c {
- span_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!(
- "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly",
- op, m, c));
+ span_lint(cx,
+ INEFFECTIVE_BIT_MASK,
+ span,
+ &format!("ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly",
+ op,
+ m,
+ c));
}
}
fn check_ineffective_gt(cx: &LateContext, span: Span, m: u64, c: u64, op: &str) {
if (c + 1).is_power_of_two() && m <= c {
- span_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!(
- "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly",
- op, m, c));
+ span_lint(cx,
+ INEFFECTIVE_BIT_MASK,
+ span,
+ &format!("ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly",
+ op,
+ m,
+ c));
}
}
-fn fetch_int_literal(cx: &LateContext, lit : &Expr) -> Option<u64> {
+fn fetch_int_literal(cx: &LateContext, lit: &Expr) -> Option<u64> {
match lit.node {
ExprLit(ref lit_ptr) => {
if let LitInt(value, _) = lit_ptr.node {
Some(value) //TODO: Handle sign
- } else { None }
+ } else {
+ None
+ }
}
ExprPath(_, _) => {
- // Important to let the borrow expire before the const lookup to avoid double
- // borrowing.
- let def_map = cx.tcx.def_map.borrow();
- match def_map.get(&lit.id) {
- Some(&PathResolution { base_def: DefConst(def_id), ..}) => Some(def_id),
- _ => None
+ {
+ // Important to let the borrow expire before the const lookup to avoid double
+ // borrowing.
+ let def_map = cx.tcx.def_map.borrow();
+ match def_map.get(&lit.id) {
+ Some(&PathResolution { base_def: DefConst(def_id), ..}) => Some(def_id),
+ _ => None,
+ }
}
+ .and_then(|def_id| lookup_const_by_id(cx.tcx, def_id, None))
+ .and_then(|l| fetch_int_literal(cx, l))
}
- .and_then(|def_id| lookup_const_by_id(cx.tcx, def_id, None))
- .and_then(|l| fetch_int_literal(cx, l)),
- _ => None
+ _ => None,
}
}
use rustc_front::intravisit::{Visitor, walk_expr};
use utils::*;
-/// **What it does:** This lint checks for `if` conditions that use blocks to contain an expression.
+/// **What it does:** This lint checks for `if` conditions that use blocks to contain an expression. It is `Warn` by default.
///
/// **Why is this bad?** It isn't really rust style, same as using parentheses to contain expressions.
///
"braces can be eliminated in conditions that are expressions, e.g `if { true } ...`"
}
-/// **What it does:** This lint checks for `if` conditions that use blocks containing statements, or conditions that use closures with blocks.
+/// **What it does:** This lint checks for `if` conditions that use blocks containing statements, or conditions that use closures with blocks. It is `Warn` by default.
///
/// **Why is this bad?** Using blocks in the condition makes it hard to read.
///
}
struct ExVisitor<'v> {
- found_block: Option<&'v Expr>
+ found_block: Option<&'v Expr>,
}
impl<'v> Visitor<'v> for ExVisitor<'v> {
if let Some(ref ex) = block.expr {
match ex.node {
ExprBlock(_) => true,
- _ => false
+ _ => false,
}
} else {
false
}
};
if complex {
- self.found_block = Some(& expr);
+ self.found_block = Some(&expr);
return;
}
}
}
}
-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'";
+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 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, "..")));
+ if in_macro(cx, expr.span) || differing_macro_contexts(expr.span, ex.span) {
+ return;
+ }
+ 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 {
+ if in_macro(cx, expr.span) || differing_macro_contexts(expr.span, block.stmts[0].span) {
+ return;
+ }
// 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, "..")));
+ 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, "");
+ span_help_and_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE, "");
}
}
}
fn check_if(cx: &LateContext, e: &Expr) {
if let ExprIf(ref check, ref then, None) = e.node {
if let Some(&Expr{ node: ExprIf(ref check_inner, ref content, None), span: sp, ..}) =
- single_stmt_of_block(then) {
- if e.span.expn_id != sp.expn_id {
- return;
- }
- span_help_and_lint(cx, COLLAPSIBLE_IF, e.span,
- "this if statement can be collapsed",
- &format!("try\nif {} && {} {}",
- check_to_string(cx, check), check_to_string(cx, check_inner),
- snippet_block(cx, content.span, "..")));
+ single_stmt_of_block(then) {
+ if e.span.expn_id != sp.expn_id {
+ return;
}
+ span_help_and_lint(cx,
+ COLLAPSIBLE_IF,
+ e.span,
+ "this if statement can be collapsed",
+ &format!("try\nif {} && {} {}",
+ check_to_string(cx, check),
+ check_to_string(cx, check_inner),
+ snippet_block(cx, content.span, "..")));
+ }
}
}
fn requires_brackets(e: &Expr) -> bool {
match e.node {
ExprBinary(Spanned {node: n, ..}, _, _) if n == BiEq => false,
- _ => true
+ _ => true,
}
}
if block.stmts.len() == 1 && block.expr.is_none() {
if let StmtExpr(ref expr, _) = block.stmts[0].node {
single_stmt_of_expr(expr)
- } else { None }
+ } else {
+ None
+ }
} else {
if block.stmts.is_empty() {
- if let Some(ref p) = block.expr { Some(p) } else { None }
- } else { None }
+ if let Some(ref p) = block.expr {
+ Some(p)
+ } else {
+ None
+ }
+ } else {
+ None
+ }
}
}
fn single_stmt_of_expr(expr: &Expr) -> Option<&Expr> {
if let ExprBlock(ref block) = expr.node {
single_stmt_of_block(block)
- } else { Some(expr) }
+ } else {
+ Some(expr)
+ }
}
pub enum FloatWidth {
Fw32,
Fw64,
- FwAny
+ FwAny,
}
impl From<FloatTy> for FloatWidth {
match *self {
ConstantByte(b) => Some(b as f64),
ConstantFloat(ref s, _) => s.parse().ok(),
- ConstantInt(i, ty) => Some(if is_negative(ty) {
- -(i as f64) } else { i as f64 }),
- _ => None
+ ConstantInt(i, ty) => {
+ Some(if is_negative(ty) {
+ -(i as f64)
+ } else {
+ i as f64
+ })
+ }
+ _ => None,
}
}
}
impl PartialEq for Constant {
fn eq(&self, other: &Constant) -> bool {
match (self, other) {
- (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) =>
- ls == rs && lsty == rsty,
+ (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) => ls == rs && lsty == rsty,
(&ConstantBinary(ref l), &ConstantBinary(ref r)) => l == r,
(&ConstantByte(l), &ConstantByte(r)) => l == r,
(&ConstantChar(l), &ConstantChar(r)) => l == r,
- (&ConstantInt(lv, lty), &ConstantInt(rv, rty)) => lv == rv &&
- (is_negative(lty) & (lv != 0)) == (is_negative(rty) & (rv != 0)),
- (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) =>
+ (&ConstantInt(lv, lty), &ConstantInt(rv, rty)) => {
+ lv == rv && (is_negative(lty) & (lv != 0)) == (is_negative(rty) & (rv != 0))
+ }
+ (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => {
if match (lw, rw) {
(FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true,
_ => false,
(Ok(l), Ok(r)) => l.eq(&r),
_ => false,
}
- } else { false },
+ } else {
+ false
+ }
+ }
(&ConstantBool(l), &ConstantBool(r)) => l == r,
(&ConstantVec(ref l), &ConstantVec(ref r)) => l == r,
- (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) =>
- ls == rs && lv == rv,
+ (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) => ls == rs && lv == rv,
(&ConstantTuple(ref l), &ConstantTuple(ref r)) => l == r,
_ => false, //TODO: Are there inter-type equalities?
}
impl PartialOrd for Constant {
fn partial_cmp(&self, other: &Constant) -> Option<Ordering> {
match (self, other) {
- (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) =>
- if lsty != rsty { None } else { Some(ls.cmp(rs)) },
+ (&ConstantStr(ref ls, ref lsty), &ConstantStr(ref rs, ref rsty)) => {
+ if lsty != rsty {
+ None
+ } else {
+ Some(ls.cmp(rs))
+ }
+ }
(&ConstantByte(ref l), &ConstantByte(ref r)) => Some(l.cmp(r)),
(&ConstantChar(ref l), &ConstantChar(ref r)) => Some(l.cmp(r)),
- (&ConstantInt(ref lv, lty), &ConstantInt(ref rv, rty)) =>
- Some(match (is_negative(lty) && *lv != 0,
- is_negative(rty) && *rv != 0) {
+ (&ConstantInt(ref lv, lty), &ConstantInt(ref rv, rty)) => {
+ Some(match (is_negative(lty) && *lv != 0, is_negative(rty) && *rv != 0) {
(true, true) => rv.cmp(lv),
(false, false) => lv.cmp(rv),
(true, false) => Less,
(false, true) => Greater,
- }),
- (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) =>
+ })
+ }
+ (&ConstantFloat(ref ls, lw), &ConstantFloat(ref rs, rw)) => {
if match (lw, rw) {
(FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true,
_ => false,
(Ok(ref l), Ok(ref r)) => l.partial_cmp(r),
_ => None,
}
- } else { None },
+ } else {
+ None
+ }
+ }
(&ConstantBool(ref l), &ConstantBool(ref r)) => Some(l.cmp(r)),
(&ConstantVec(ref l), &ConstantVec(ref r)) => l.partial_cmp(&r),
- (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) =>
+ (&ConstantRepeat(ref lv, ref ls), &ConstantRepeat(ref rv, ref rs)) => {
match lv.partial_cmp(rv) {
Some(Equal) => Some(ls.cmp(rs)),
x => x,
- },
+ }
+ }
(&ConstantTuple(ref l), &ConstantTuple(ref r)) => l.partial_cmp(r),
- _ => None, //TODO: Are there any useful inter-type orderings?
- }
+ _ => None, //TODO: Are there any useful inter-type orderings?
+ }
}
}
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match *self {
ConstantStr(ref s, _) => write!(fmt, "{:?}", s),
- ConstantByte(ref b) =>
- write!(fmt, "b'").and_then(|_| format_byte(fmt, *b))
- .and_then(|_| write!(fmt, "'")),
+ ConstantByte(ref b) => {
+ write!(fmt, "b'")
+ .and_then(|_| format_byte(fmt, *b))
+ .and_then(|_| write!(fmt, "'"))
+ }
ConstantBinary(ref bs) => {
try!(write!(fmt, "b\""));
for b in bs.iter() {
ConstantChar(ref c) => write!(fmt, "'{}'", c),
ConstantInt(ref i, ref ity) => {
let (sign, suffix) = match *ity {
- LitIntType::SignedIntLit(ref sity, ref sign) =>
- (if let Sign::Minus = *sign { "-" } else { "" },
- sity.ty_to_string()),
- LitIntType::UnsignedIntLit(ref uity) =>
- ("", uity.ty_to_string()),
- LitIntType::UnsuffixedIntLit(ref sign) =>
- (if let Sign::Minus = *sign { "-" } else { "" },
- "".into()),
+ LitIntType::SignedIntLit(ref sity, ref sign) => {
+ (if let Sign::Minus = *sign {
+ "-"
+ } else {
+ ""
+ },
+ sity.ty_to_string())
+ }
+ LitIntType::UnsignedIntLit(ref uity) => ("", uity.ty_to_string()),
+ LitIntType::UnsuffixedIntLit(ref sign) => {
+ (if let Sign::Minus = *sign {
+ "-"
+ } else {
+ ""
+ },
+ "".into())
+ }
};
write!(fmt, "{}{}{}", sign, i, suffix)
}
}
ConstantBool(ref b) => write!(fmt, "{}", b),
ConstantRepeat(ref c, ref n) => write!(fmt, "[{}; {}]", c, n),
- ConstantVec(ref v) => write!(fmt, "[{}]",
- v.iter().map(|i| format!("{}", i))
- .collect::<Vec<_>>().join(", ")),
- ConstantTuple(ref t) => write!(fmt, "({})",
- t.iter().map(|i| format!("{}", i))
- .collect::<Vec<_>>().join(", ")),
+ ConstantVec(ref v) => {
+ write!(fmt,
+ "[{}]",
+ v.iter()
+ .map(|i| format!("{}", i))
+ .collect::<Vec<_>>()
+ .join(", "))
+ }
+ ConstantTuple(ref t) => {
+ write!(fmt,
+ "({})",
+ t.iter()
+ .map(|i| format!("{}", i))
+ .collect::<Vec<_>>()
+ .join(", "))
+ }
}
}
}
ConstantInt(value, ty) => {
let (nvalue, nty) = match ty {
SignedIntLit(ity, Plus) => {
- if value == ::std::u64::MAX { return None; }
+ if value == ::std::u64::MAX {
+ return None;
+ }
(value + 1, SignedIntLit(ity, Minus))
}
SignedIntLit(ity, Minus) => {
UintTy::TyU16 => ::std::u16::MAX as u64,
UintTy::TyU32 => ::std::u32::MAX as u64,
UintTy::TyU64 => ::std::u64::MAX,
- UintTy::TyUs => { return None; } // refuse to guess
+ UintTy::TyUs => {
+ return None;
+ } // refuse to guess
};
(!value & mask, UnsignedIntLit(ity))
}
- UnsuffixedIntLit(_) => { return None; } // refuse to guess
+ UnsuffixedIntLit(_) => {
+ return None;
+ } // refuse to guess
};
ConstantInt(nvalue, nty)
}
- _ => { return None; }
+ _ => {
+ return None;
+ }
})
}
fn constant_negate(o: Constant) -> Option<Constant> {
Some(match o {
- ConstantInt(value, ty) =>
- ConstantInt(value, match ty {
- SignedIntLit(ity, sign) =>
- SignedIntLit(ity, neg_sign(sign)),
- UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)),
- _ => { return None; }
- }),
- ConstantFloat(is, ty) =>
- ConstantFloat(neg_float_str(is), ty),
- _ => { return None; }
+ ConstantInt(value, ty) => {
+ ConstantInt(value,
+ match ty {
+ SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)),
+ UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)),
+ _ => {
+ return None;
+ }
+ })
+ }
+ ConstantFloat(is, ty) => ConstantFloat(neg_float_str(is), ty),
+ _ => {
+ return None;
+ }
})
}
fn unify_int_type(l: LitIntType, r: LitIntType, s: Sign) -> Option<LitIntType> {
match (l, r) {
- (SignedIntLit(lty, _), SignedIntLit(rty, _)) => if lty == rty {
- Some(SignedIntLit(lty, s)) } else { None },
- (UnsignedIntLit(lty), UnsignedIntLit(rty)) =>
+ (SignedIntLit(lty, _), SignedIntLit(rty, _)) => {
+ if lty == rty {
+ Some(SignedIntLit(lty, s))
+ } else {
+ None
+ }
+ }
+ (UnsignedIntLit(lty), UnsignedIntLit(rty)) => {
if s == Plus && lty == rty {
Some(UnsignedIntLit(lty))
- } else { None },
+ } else {
+ None
+ }
+ }
(UnsuffixedIntLit(_), UnsuffixedIntLit(_)) => Some(UnsuffixedIntLit(s)),
(SignedIntLit(lty, _), UnsuffixedIntLit(_)) => Some(SignedIntLit(lty, s)),
- (UnsignedIntLit(lty), UnsuffixedIntLit(rs)) => if rs == Plus {
- Some(UnsignedIntLit(lty)) } else { None },
+ (UnsignedIntLit(lty), UnsuffixedIntLit(rs)) => {
+ if rs == Plus {
+ Some(UnsignedIntLit(lty))
+ } else {
+ None
+ }
+ }
(UnsuffixedIntLit(_), SignedIntLit(rty, _)) => Some(SignedIntLit(rty, s)),
- (UnsuffixedIntLit(ls), UnsignedIntLit(rty)) => if ls == Plus {
- Some(UnsignedIntLit(rty)) } else { None },
+ (UnsuffixedIntLit(ls), UnsignedIntLit(rty)) => {
+ if ls == Plus {
+ Some(UnsignedIntLit(rty))
+ } else {
+ None
+ }
+ }
_ => None,
}
}
-fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) ->
- Option<Constant> {
+fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> Option<Constant> {
if neg > pos {
unify_int_type(nty, pty, Minus).map(|ty| ConstantInt(neg - pos, ty))
} else {
}
}
-fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: bool) ->
- Option<Constant> {
- unify_int_type(lty, rty, if neg { Minus } else { Plus }).and_then(
- |ty| l.checked_sub(r).map(|v| ConstantInt(v, ty)))
+fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: bool) -> Option<Constant> {
+ unify_int_type(lty,
+ rty,
+ if neg {
+ Minus
+ } else {
+ Plus
+ })
+ .and_then(|ty| l.checked_sub(r).map(|v| ConstantInt(v, ty)))
}
pub fn constant(lcx: &LateContext, e: &Expr) -> Option<(Constant, bool)> {
- let mut cx = ConstEvalLateContext { lcx: Some(lcx), needed_resolution: false };
+ let mut cx = ConstEvalLateContext {
+ lcx: Some(lcx),
+ needed_resolution: false,
+ };
cx.expr(e).map(|cst| (cst, cx.needed_resolution))
}
pub fn constant_simple(e: &Expr) -> Option<Constant> {
- let mut cx = ConstEvalLateContext { lcx: None, needed_resolution: false };
+ let mut cx = ConstEvalLateContext {
+ lcx: None,
+ needed_resolution: false,
+ };
cx.expr(e)
}
struct ConstEvalLateContext<'c, 'cc: 'c> {
lcx: Option<&'c LateContext<'c, 'cc>>,
- needed_resolution: bool
+ needed_resolution: bool,
}
impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
-
/// simple constant folding: Insert an expression, get a constant or none.
fn expr(&mut self, e: &Expr) -> Option<Constant> {
match e.node {
ExprPath(_, _) => self.fetch_path(e),
ExprBlock(ref block) => self.block(block),
- ExprIf(ref cond, ref then, ref otherwise) =>
- self.ifthenelse(cond, then, otherwise),
+ ExprIf(ref cond, ref then, ref otherwise) => self.ifthenelse(cond, then, otherwise),
ExprLit(ref lit) => Some(lit_to_constant(&lit.node)),
ExprVec(ref vec) => self.multi(vec).map(ConstantVec),
ExprTup(ref tup) => self.multi(tup).map(ConstantTuple),
- ExprRepeat(ref value, ref number) =>
- self.binop_apply(value, number, |v, n|
- Some(ConstantRepeat(Box::new(v), n.as_u64() as usize))),
- ExprUnary(op, ref operand) => self.expr(operand).and_then(
- |o| match op {
- UnNot => constant_not(o),
- UnNeg => constant_negate(o),
- UnDeref => Some(o),
- }),
- ExprBinary(op, ref left, ref right) =>
- self.binop(op, left, right),
- //TODO: add other expressions
+ ExprRepeat(ref value, ref number) => {
+ self.binop_apply(value, number, |v, n| Some(ConstantRepeat(Box::new(v), n.as_u64() as usize)))
+ }
+ ExprUnary(op, ref operand) => {
+ self.expr(operand).and_then(|o| {
+ match op {
+ UnNot => constant_not(o),
+ UnNeg => constant_negate(o),
+ UnDeref => Some(o),
+ }
+ })
+ }
+ ExprBinary(op, ref left, ref right) => self.binop(op, left, right),
+ // TODO: add other expressions
_ => None,
}
}
/// create `Some(Vec![..])` of all constants, unless there is any
/// non-constant part
- fn multi<E: Deref<Target=Expr> + Sized>(&mut self, vec: &[E]) ->
- Option<Vec<Constant>> {
- vec.iter().map(|elem| self.expr(elem))
- .collect::<Option<_>>()
+ fn multi<E: Deref<Target = Expr> + Sized>(&mut self, vec: &[E]) -> Option<Vec<Constant>> {
+ vec.iter()
+ .map(|elem| self.expr(elem))
+ .collect::<Option<_>>()
}
/// lookup a possibly constant expression from a ExprPath
fn fetch_path(&mut self, e: &Expr) -> Option<Constant> {
if let Some(lcx) = self.lcx {
let mut maybe_id = None;
- if let Some(&PathResolution { base_def: DefConst(id), ..}) =
- lcx.tcx.def_map.borrow().get(&e.id) {
+ if let Some(&PathResolution { base_def: DefConst(id), ..}) = lcx.tcx.def_map.borrow().get(&e.id) {
maybe_id = Some(id);
}
// separate if lets to avoid doubleborrowing the defmap
fn block(&mut self, block: &Block) -> Option<Constant> {
if block.stmts.is_empty() {
block.expr.as_ref().and_then(|ref b| self.expr(b))
- } else { None }
+ } else {
+ None
+ }
}
- fn ifthenelse(&mut self, cond: &Expr, then: &Block, otherwise: &Option<P<Expr>>)
- -> Option<Constant> {
+ fn ifthenelse(&mut self, cond: &Expr, then: &Block, otherwise: &Option<P<Expr>>) -> Option<Constant> {
if let Some(ConstantBool(b)) = self.expr(cond) {
if b {
self.block(then)
} else {
otherwise.as_ref().and_then(|expr| self.expr(expr))
}
- } else { None }
+ } else {
+ None
+ }
}
fn binop(&mut self, op: BinOp, left: &Expr, right: &Expr) -> Option<Constant> {
match op.node {
- BiAdd => self.binop_apply(left, right, |l, r|
- match (l, r) {
- (ConstantByte(l8), ConstantByte(r8)) =>
- l8.checked_add(r8).map(ConstantByte),
- (ConstantInt(l64, lty), ConstantInt(r64, rty)) => {
- let (ln, rn) = (is_negative(lty), is_negative(rty));
- if ln == rn {
- unify_int_type(lty, rty, if ln { Minus } else { Plus })
- .and_then(|ty| l64.checked_add(r64).map(
- |v| ConstantInt(v, ty)))
- } else {
- if ln {
- add_neg_int(r64, rty, l64, lty)
+ BiAdd => {
+ self.binop_apply(left, right, |l, r| {
+ match (l, r) {
+ (ConstantByte(l8), ConstantByte(r8)) => l8.checked_add(r8).map(ConstantByte),
+ (ConstantInt(l64, lty), ConstantInt(r64, rty)) => {
+ let (ln, rn) = (is_negative(lty), is_negative(rty));
+ if ln == rn {
+ unify_int_type(lty,
+ rty,
+ if ln {
+ Minus
+ } else {
+ Plus
+ })
+ .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty)))
} else {
- add_neg_int(l64, lty, r64, rty)
+ if ln {
+ add_neg_int(r64, rty, l64, lty)
+ } else {
+ add_neg_int(l64, lty, r64, rty)
+ }
}
}
+ // TODO: float (would need bignum library?)
+ _ => None,
}
- // TODO: float (would need bignum library?)
- _ => None
- }),
- BiSub => self.binop_apply(left, right, |l, r|
- match (l, r) {
- (ConstantByte(l8), ConstantByte(r8)) => if r8 > l8 {
- None } else { Some(ConstantByte(l8 - r8)) },
- (ConstantInt(l64, lty), ConstantInt(r64, rty)) =>
- match (is_negative(lty), is_negative(rty)) {
- (false, false) => sub_int(l64, lty, r64, rty, r64 > l64),
- (true, true) => sub_int(l64, lty, r64, rty, l64 > r64),
- (true, false) => unify_int_type(lty, rty, Minus)
- .and_then(|ty| l64.checked_add(r64).map(
- |v| ConstantInt(v, ty))),
- (false, true) => unify_int_type(lty, rty, Plus)
- .and_then(|ty| l64.checked_add(r64).map(
- |v| ConstantInt(v, ty))),
- },
- _ => None,
- }),
+ })
+ }
+ BiSub => {
+ self.binop_apply(left, right, |l, r| {
+ match (l, r) {
+ (ConstantByte(l8), ConstantByte(r8)) => {
+ if r8 > l8 {
+ None
+ } else {
+ Some(ConstantByte(l8 - r8))
+ }
+ }
+ (ConstantInt(l64, lty), ConstantInt(r64, rty)) => {
+ match (is_negative(lty), is_negative(rty)) {
+ (false, false) => sub_int(l64, lty, r64, rty, r64 > l64),
+ (true, true) => sub_int(l64, lty, r64, rty, l64 > r64),
+ (true, false) => {
+ unify_int_type(lty, rty, Minus)
+ .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty)))
+ }
+ (false, true) => {
+ unify_int_type(lty, rty, Plus)
+ .and_then(|ty| l64.checked_add(r64).map(|v| ConstantInt(v, ty)))
+ }
+ }
+ }
+ _ => None,
+ }
+ })
+ }
BiMul => self.divmul(left, right, u64::checked_mul),
BiDiv => self.divmul(left, right, u64::checked_div),
- //BiRem,
+ // BiRem,
BiAnd => self.short_circuit(left, right, false),
BiOr => self.short_circuit(left, right, true),
BiBitXor => self.bitop(left, right, |x, y| x ^ y),
BiBitOr => self.bitop(left, right, |x, y| (x | y)),
BiShl => self.bitop(left, right, |x, y| x << y),
BiShr => self.bitop(left, right, |x, y| x >> y),
- BiEq => self.binop_apply(left, right,
- |l, r| Some(ConstantBool(l == r))),
- BiNe => self.binop_apply(left, right,
- |l, r| Some(ConstantBool(l != r))),
+ BiEq => self.binop_apply(left, right, |l, r| Some(ConstantBool(l == r))),
+ BiNe => self.binop_apply(left, right, |l, r| Some(ConstantBool(l != r))),
BiLt => self.cmp(left, right, Less, true),
BiLe => self.cmp(left, right, Greater, false),
BiGe => self.cmp(left, right, Less, false),
BiGt => self.cmp(left, right, Greater, true),
- _ => None
+ _ => None,
}
}
- fn divmul<F>(&mut self, left: &Expr, right: &Expr, f: F)
- -> Option<Constant> where F: Fn(u64, u64) -> Option<u64> {
- self.binop_apply(left, right, |l, r|
+ fn divmul<F>(&mut self, left: &Expr, right: &Expr, f: F) -> Option<Constant>
+ where F: Fn(u64, u64) -> Option<u64>
+ {
+ self.binop_apply(left, right, |l, r| {
match (l, r) {
(ConstantInt(l64, lty), ConstantInt(r64, rty)) => {
- f(l64, r64).and_then(|value|
- unify_int_type(lty, rty, if is_negative(lty) ==
- is_negative(rty) { Plus } else { Minus })
- .map(|ty| ConstantInt(value, ty)))
+ f(l64, r64).and_then(|value| {
+ unify_int_type(lty,
+ rty,
+ if is_negative(lty) == is_negative(rty) {
+ Plus
+ } else {
+ Minus
+ })
+ .map(|ty| ConstantInt(value, ty))
+ })
}
_ => None,
- })
+ }
+ })
}
- fn bitop<F>(&mut self, left: &Expr, right: &Expr, f: F)
- -> Option<Constant> where F: Fn(u64, u64) -> u64 {
- self.binop_apply(left, right, |l, r| match (l, r) {
- (ConstantBool(l), ConstantBool(r)) =>
- Some(ConstantBool(f(l as u64, r as u64) != 0)),
- (ConstantByte(l8), ConstantByte(r8)) =>
- Some(ConstantByte(f(l8 as u64, r8 as u64) as u8)),
- (ConstantInt(l, lty), ConstantInt(r, rty)) =>
- unify_int_type(lty, rty, Plus).map(|ty| ConstantInt(f(l, r), ty)),
- _ => None
+ fn bitop<F>(&mut self, left: &Expr, right: &Expr, f: F) -> Option<Constant>
+ where F: Fn(u64, u64) -> u64
+ {
+ self.binop_apply(left, right, |l, r| {
+ match (l, r) {
+ (ConstantBool(l), ConstantBool(r)) => Some(ConstantBool(f(l as u64, r as u64) != 0)),
+ (ConstantByte(l8), ConstantByte(r8)) => Some(ConstantByte(f(l8 as u64, r8 as u64) as u8)),
+ (ConstantInt(l, lty), ConstantInt(r, rty)) => {
+ unify_int_type(lty, rty, Plus).map(|ty| ConstantInt(f(l, r), ty))
+ }
+ _ => None,
+ }
})
}
fn cmp(&mut self, left: &Expr, right: &Expr, ordering: Ordering, b: bool) -> Option<Constant> {
- self.binop_apply(left, right, |l, r| l.partial_cmp(&r).map(|o|
- ConstantBool(b == (o == ordering))))
+ self.binop_apply(left,
+ right,
+ |l, r| l.partial_cmp(&r).map(|o| ConstantBool(b == (o == ordering))))
}
fn binop_apply<F>(&mut self, left: &Expr, right: &Expr, op: F) -> Option<Constant>
- where F: Fn(Constant, Constant) -> Option<Constant> {
+ where F: Fn(Constant, Constant) -> Option<Constant>
+ {
if let (Some(lc), Some(rc)) = (self.expr(left), self.expr(right)) {
op(lc, rc)
- } else { None }
+ } else {
+ None
+ }
}
fn short_circuit(&mut self, left: &Expr, right: &Expr, b: bool) -> Option<Constant> {
- self.expr(left).and_then(|left|
+ self.expr(left).and_then(|left| {
if let ConstantBool(lbool) = left {
if lbool == b {
Some(left)
} else {
- self.expr(right).and_then(|right|
+ self.expr(right).and_then(|right| {
if let ConstantBool(_) = right {
Some(right)
- } else { None }
- )
+ } else {
+ None
+ }
+ })
}
- } else { None }
- )
+ } else {
+ None
+ }
+ })
}
}
use syntax::ast::Attribute;
use rustc_front::intravisit::{Visitor, walk_expr};
-use utils::{in_macro, LimitStack};
+use utils::{in_macro, LimitStack, span_help_and_lint};
/// **What it does:** It `Warn`s on methods with high cyclomatic complexity
///
impl CyclomaticComplexity {
pub fn new(limit: u64) -> Self {
- CyclomaticComplexity {
- limit: LimitStack::new(limit),
- }
+ CyclomaticComplexity { limit: LimitStack::new(limit) }
}
}
impl CyclomaticComplexity {
fn check<'a, 'tcx>(&mut self, cx: &'a LateContext<'a, 'tcx>, block: &Block, span: Span) {
- if in_macro(cx, span) { return; }
+ if in_macro(cx, span) {
+ return;
+ }
let cfg = CFG::new(cx.tcx, block);
let n = cfg.graph.len_nodes() as u64;
let e = cfg.graph.len_edges() as u64;
} else {
let rust_cc = cc + divergence - narms;
if rust_cc > self.limit.limit() {
- cx.span_lint_help(CYCLOMATIC_COMPLEXITY, span,
- &format!("The function has a cyclomatic complexity of {}.", rust_cc),
- "You could split it up into multiple smaller functions");
+ span_help_and_lint(cx,
+ CYCLOMATIC_COMPLEXITY,
+ span,
+ &format!("The function has a cyclomatic complexity of {}", rust_cc),
+ "You could split it up into multiple smaller functions");
}
}
}
if arms_n > 1 {
self.0 += arms_n - 2;
}
- },
- ExprClosure(..) => {},
+ }
+ ExprClosure(..) => {}
_ => walk_expr(self, e),
}
}
self.0 += 1;
}
}
- },
- ExprClosure(..) => {},
+ }
+ ExprClosure(..) => {}
_ => walk_expr(self, e),
}
}
#[cfg(feature="debugging")]
fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) {
- cx.sess().span_bug(span, &format!("Clippy encountered a bug calculating cyclomatic complexity: \
- cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div));;
+ cx.sess().span_bug(span,
+ &format!("Clippy encountered a bug calculating cyclomatic complexity: cc = {}, arms = {}, \
+ div = {}. Please file a bug report.",
+ cc,
+ narms,
+ div));;
}
#[cfg(not(feature="debugging"))]
fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) {
if cx.current_level(CYCLOMATIC_COMPLEXITY) != Level::Allow {
- cx.sess().span_note(span, &format!("Clippy encountered a bug calculating cyclomatic complexity \
- (hide this message with `#[allow(cyclomatic_complexity)]`): \
- cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div));
+ cx.sess().span_note_without_error(span,
+ &format!("Clippy encountered a bug calculating cyclomatic complexity \
+ (hide this message with `#[allow(cyclomatic_complexity)]`): cc \
+ = {}, arms = {}, div = {}. Please file a bug report.",
+ cc,
+ narms,
+ div));
}
}
--- /dev/null
+use rustc::lint::*;
+use rustc_front::hir::*;
+use syntax::codemap::Span;
+use utils::{get_item_name, is_exp_equal, match_type, snippet, span_lint_and_then, walk_ptrs_ty};
+use utils::{BTREEMAP_PATH, HASHMAP_PATH};
+
+/// **What it does:** This lint checks for uses of `contains_key` + `insert` on `HashMap` or
+/// `BTreeMap`.
+///
+/// **Why is this bad?** Using `entry` is more efficient.
+///
+/// **Known problems:** Some false negatives, eg.:
+/// ```
+/// let k = &key;
+/// if !m.contains_key(k) { m.insert(k.clone(), v); }
+/// ```
+///
+/// **Example:**
+/// ```rust
+/// if !m.contains_key(&k) { m.insert(k, v) }
+/// ```
+/// can be rewritten as:
+/// ```rust
+/// m.entry(k).or_insert(v);
+/// ```
+declare_lint! {
+ pub MAP_ENTRY,
+ Warn,
+ "use of `contains_key` followed by `insert` on a `HashMap` or `BTreeMap`"
+}
+
+#[derive(Copy,Clone)]
+pub struct HashMapLint;
+
+impl LintPass for HashMapLint {
+ fn get_lints(&self) -> LintArray {
+ lint_array!(MAP_ENTRY)
+ }
+}
+
+impl LateLintPass for HashMapLint {
+ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
+ if_let_chain! {
+ [
+ let ExprIf(ref check, ref then, _) = expr.node,
+ let ExprUnary(UnOp::UnNot, ref check) = check.node,
+ let ExprMethodCall(ref name, _, ref params) = check.node,
+ params.len() >= 2,
+ name.node.as_str() == "contains_key"
+ ], {
+ let key = match params[1].node {
+ ExprAddrOf(_, ref key) => key,
+ _ => return
+ };
+
+ let map = ¶ms[0];
+ let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(map));
+
+ let kind = if match_type(cx, obj_ty, &BTREEMAP_PATH) {
+ "BTreeMap"
+ }
+ else if match_type(cx, obj_ty, &HASHMAP_PATH) {
+ "HashMap"
+ }
+ else {
+ return
+ };
+
+ let sole_expr = if then.expr.is_some() { 1 } else { 0 } + then.stmts.len() == 1;
+
+ if let Some(ref then) = then.expr {
+ check_for_insert(cx, expr.span, map, key, then, sole_expr, kind);
+ }
+
+ for stmt in &then.stmts {
+ if let StmtSemi(ref stmt, _) = stmt.node {
+ check_for_insert(cx, expr.span, map, key, stmt, sole_expr, kind);
+ }
+ }
+ }
+ }
+ }
+}
+
+fn check_for_insert(cx: &LateContext, span: Span, map: &Expr, key: &Expr, expr: &Expr, sole_expr: bool, kind: &str) {
+ if_let_chain! {
+ [
+ let ExprMethodCall(ref name, _, ref params) = expr.node,
+ params.len() == 3,
+ name.node.as_str() == "insert",
+ get_item_name(cx, map) == get_item_name(cx, &*params[0]),
+ is_exp_equal(cx, key, ¶ms[1])
+ ], {
+ let help = if sole_expr {
+ format!("{}.entry({}).or_insert({})",
+ snippet(cx, map.span, ".."),
+ snippet(cx, params[1].span, ".."),
+ snippet(cx, params[2].span, ".."))
+ }
+ else {
+ format!("{}.entry({})",
+ snippet(cx, map.span, ".."),
+ snippet(cx, params[1].span, ".."))
+ };
+
+ span_lint_and_then(cx, MAP_ENTRY, span,
+ &format!("usage of `contains_key` followed by `insert` on `{}`", kind), |db| {
+ db.span_suggestion(span, "Consider using", help.clone());
+ });
+ }
+ }
+}
use rustc::lint::*;
use rustc_front::hir::*;
use rustc_front::util as ast_util;
-use syntax::ptr::P;
-use consts::constant;
-use utils::span_lint;
+use utils::{is_exp_equal, span_lint};
/// **What it does:** This lint checks for equal operands to comparisons and bitwise binary operators (`&`, `|` and `^`). It is `Warn` by default.
///
fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
if let ExprBinary(ref op, ref left, ref right) = e.node {
if is_cmp_or_bit(op) && is_exp_equal(cx, left, right) {
- span_lint(cx, EQ_OP, e.span, &format!(
- "equal expressions as operands to {}",
- ast_util::binop_to_string(op.node)));
+ span_lint(cx,
+ EQ_OP,
+ e.span,
+ &format!("equal expressions as operands to {}", ast_util::binop_to_string(op.node)));
}
}
}
}
-pub fn is_exp_equal(cx: &LateContext, left : &Expr, right : &Expr) -> bool {
- if let (Some(l), Some(r)) = (constant(cx, left), constant(cx, right)) {
- if l == r {
- return true;
- }
- }
- match (&left.node, &right.node) {
- (&ExprField(ref lfexp, ref lfident),
- &ExprField(ref rfexp, ref rfident)) =>
- lfident.node == rfident.node && is_exp_equal(cx, lfexp, rfexp),
- (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node,
- (&ExprPath(ref lqself, ref lsubpath),
- &ExprPath(ref rqself, ref rsubpath)) =>
- both(lqself, rqself, is_qself_equal) &&
- is_path_equal(lsubpath, rsubpath),
- (&ExprTup(ref ltup), &ExprTup(ref rtup)) =>
- is_exps_equal(cx, ltup, rtup),
- (&ExprVec(ref l), &ExprVec(ref r)) => is_exps_equal(cx, l, r),
- (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) =>
- is_exp_equal(cx, lx, rx) && is_cast_ty_equal(lt, rt),
- _ => false
- }
-}
-fn is_exps_equal(cx: &LateContext, left : &[P<Expr>], right : &[P<Expr>]) -> bool {
- over(left, right, |l, r| is_exp_equal(cx, l, r))
-}
-
-fn is_path_equal(left : &Path, right : &Path) -> bool {
- // The == of idents doesn't work with different contexts,
- // we have to be explicit about hygiene
- left.global == right.global && over(&left.segments, &right.segments,
- |l, r| l.identifier.name == r.identifier.name
- && l.parameters == r.parameters)
-}
-
-fn is_qself_equal(left : &QSelf, right : &QSelf) -> bool {
- left.ty.node == right.ty.node && left.position == right.position
-}
-
-fn over<X, F>(left: &[X], right: &[X], mut eq_fn: F) -> bool
- where F: FnMut(&X, &X) -> bool {
- left.len() == right.len() && left.iter().zip(right).all(|(x, y)|
- eq_fn(x, y))
-}
-
-fn both<X, F>(l: &Option<X>, r: &Option<X>, mut eq_fn : F) -> bool
- where F: FnMut(&X, &X) -> bool {
- l.as_ref().map_or_else(|| r.is_none(), |x| r.as_ref().map_or(false,
- |y| eq_fn(x, y)))
-}
-
-fn is_cmp_or_bit(op : &BinOp) -> bool {
+fn is_cmp_or_bit(op: &BinOp) -> bool {
match op.node {
- BiEq | BiLt | BiLe | BiGt | BiGe | BiNe | BiAnd | BiOr |
- BiBitXor | BiBitAnd | BiBitOr => true,
- _ => false
- }
-}
-
-fn is_cast_ty_equal(left: &Ty, right: &Ty) -> bool {
- match (&left.node, &right.node) {
- (&TyVec(ref lvec), &TyVec(ref rvec)) => is_cast_ty_equal(lvec, rvec),
- (&TyPtr(ref lmut), &TyPtr(ref rmut)) =>
- lmut.mutbl == rmut.mutbl &&
- is_cast_ty_equal(&*lmut.ty, &*rmut.ty),
- (&TyRptr(_, ref lrmut), &TyRptr(_, ref rrmut)) =>
- lrmut.mutbl == rrmut.mutbl &&
- is_cast_ty_equal(&*lrmut.ty, &*rrmut.ty),
- (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) =>
- both(lq, rq, is_qself_equal) && is_path_equal(lpath, rpath),
- (&TyInfer, &TyInfer) => true,
- _ => false
+ BiEq |
+ BiLt |
+ BiLe |
+ BiGt |
+ BiGe |
+ BiNe |
+ BiAnd |
+ BiOr |
+ BiBitXor |
+ BiBitAnd |
+ BiBitOr => true,
+ _ => false,
}
}
pub struct EscapePass;
-/// **What it does:** This lint checks for usage of `Box<T>` where an unboxed `T` would work fine
+/// **What it does:** This lint checks for usage of `Box<T>` where an unboxed `T` would work fine. It is `Warn` by default.
///
-/// **Why is this bad?** This is an unnecessary allocation, and bad for performance
+/// **Why is this bad?** This is an unnecessary allocation, and bad for performance. It is only necessary to allocate if you wish to move the box into something.
///
-/// It is only necessary to allocate if you wish to move the box into something.
+/// **Known problems:** None
///
/// **Example:**
///
}
impl LateLintPass for EscapePass {
- fn check_fn(&mut self,
- cx: &LateContext,
- _: visit::FnKind,
- decl: &FnDecl,
- body: &Block,
- _: Span,
- id: NodeId) {
+ fn check_fn(&mut self, cx: &LateContext, _: visit::FnKind, decl: &FnDecl, body: &Block, _: Span, id: NodeId) {
let param_env = ty::ParameterEnvironment::for_item(cx.tcx, id);
let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, Some(param_env), false);
let mut v = EscapeDelegate {
}
impl<'a, 'tcx: 'a> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
- fn consume(&mut self,
- _: NodeId,
- _: Span,
- cmt: cmt<'tcx>,
- mode: ConsumeMode) {
+ fn consume(&mut self, _: NodeId, _: Span, cmt: cmt<'tcx>, mode: ConsumeMode) {
if let Categorization::Local(lid) = cmt.cat {
if self.set.contains(&lid) {
}
}
- fn borrow(&mut self,
- borrow_id: NodeId,
- _: Span,
- cmt: cmt<'tcx>,
- _: ty::Region,
- _: ty::BorrowKind,
+ fn borrow(&mut self, borrow_id: NodeId, _: Span, cmt: cmt<'tcx>, _: ty::Region, _: ty::BorrowKind,
loan_cause: LoanCause) {
if let Categorization::Local(lid) = cmt.cat {
}
} else if LoanCause::AddrOf == loan_cause {
// &x
- if let Some(&AutoAdjustment::AdjustDerefRef(adj)) =
- self.cx.tcx.tables.borrow().adjustments
- .get(&self.cx.tcx.map.get_parent_node(borrow_id)) {
+ if let Some(&AutoAdjustment::AdjustDerefRef(adj)) = self.cx
+ .tcx
+ .tables
+ .borrow()
+ .adjustments
+ .get(&self.cx
+ .tcx
+ .map
+ .get_parent_node(borrow_id)) {
if adj.autoderefs <= 1 {
// foo(&x) where no extra autoreffing is happening
self.set.remove(&lid);
}
}
fn decl_without_init(&mut self, _: NodeId, _: Span) {}
- fn mutate(&mut self,
- _: NodeId,
- _: Span,
- _: cmt<'tcx>,
- _: MutateMode) {
- }
+ fn mutate(&mut self, _: NodeId, _: Span, _: cmt<'tcx>, _: MutateMode) {}
}
}
if p.segments[0].identifier != ident.node {
// The two idents should be the same
- return
+ return;
}
} else {
- return
+ return;
}
} else {
- return
+ return;
}
}
- span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span,
- "redundant closure found",
- || {
+ span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |db| {
if let Some(snippet) = snippet_opt(cx, caller.span) {
- cx.sess().span_suggestion(expr.span,
- "remove closure as shown:",
- snippet);
+ db.span_suggestion(expr.span, "remove closure as shown:", snippet);
}
});
}
impl LateLintPass for IdentityOp {
fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
- if in_macro(cx, e.span) { return; }
+ if in_macro(cx, e.span) {
+ return;
+ }
if let ExprBinary(ref cmp, ref left, ref right) = e.node {
match cmp.node {
BiAdd | BiBitOr | BiBitXor => {
check(cx, left, 0, e.span, right.span);
check(cx, right, 0, e.span, left.span);
}
- BiShl | BiShr | BiSub =>
- check(cx, right, 0, e.span, left.span),
+ BiShl | BiShr | BiSub => check(cx, right, 0, e.span, left.span),
BiMul => {
check(cx, left, 1, e.span, right.span);
check(cx, right, 1, e.span, left.span);
}
- BiDiv =>
- check(cx, right, 1, e.span, left.span),
+ BiDiv => check(cx, right, 1, e.span, left.span),
BiBitAnd => {
check(cx, left, -1, e.span, right.span);
check(cx, right, -1, e.span, left.span);
}
- _ => ()
+ _ => (),
}
}
}
1 => !is_negative(ty) && v == 1,
_ => unreachable!(),
} {
- span_lint(cx, IDENTITY_OP, span, &format!(
- "the operation is ineffective. Consider reducing it to `{}`",
- snippet(cx, arg, "..")));
+ span_lint(cx,
+ IDENTITY_OP,
+ span,
+ &format!("the operation is ineffective. Consider reducing it to `{}`",
+ snippet(cx, arg, "..")));
}
}
}
impl LateLintPass for LenZero {
fn check_item(&mut self, cx: &LateContext, item: &Item) {
match item.node {
- ItemTrait(_, _, _, ref trait_items) =>
- check_trait_items(cx, item, trait_items),
- ItemImpl(_, _, _, None, _, ref impl_items) => // only non-trait
- check_impl_items(cx, item, impl_items),
- _ => ()
+ ItemTrait(_, _, _, ref trait_items) => check_trait_items(cx, item, trait_items),
+ ItemImpl(_, _, _, None, _, ref impl_items) => check_impl_items(cx, item, impl_items),
+ _ => (),
}
}
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
- if let ExprBinary(Spanned{node: cmp, ..}, ref left, ref right) =
- expr.node {
+ if let ExprBinary(Spanned{node: cmp, ..}, ref left, ref right) = expr.node {
match cmp {
BiEq => check_cmp(cx, expr.span, left, right, ""),
BiGt | BiNe => check_cmp(cx, expr.span, left, right, "!"),
- _ => ()
+ _ => (),
}
}
}
fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItem]) {
fn is_named_self(item: &TraitItem, name: &str) -> bool {
- item.name.as_str() == name && if let MethodTraitItem(ref sig, _) =
- item.node { is_self_sig(sig) } else { false }
+ item.name.as_str() == name &&
+ if let MethodTraitItem(ref sig, _) = item.node {
+ is_self_sig(sig)
+ } else {
+ false
+ }
}
if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) {
- //span_lint(cx, LEN_WITHOUT_IS_EMPTY, item.span, &format!("trait {}", item.ident));
+ // span_lint(cx, LEN_WITHOUT_IS_EMPTY, item.span, &format!("trait {}", item.ident));
for i in trait_items {
if is_named_self(i, "len") {
- span_lint(cx, LEN_WITHOUT_IS_EMPTY, i.span,
- &format!("trait `{}` has a `.len(_: &Self)` method, but no \
- `.is_empty(_: &Self)` method. Consider adding one",
+ span_lint(cx,
+ LEN_WITHOUT_IS_EMPTY,
+ i.span,
+ &format!("trait `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \
+ Consider adding one",
item.name));
}
- };
+ }
}
}
fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItem]) {
fn is_named_self(item: &ImplItem, name: &str) -> bool {
- item.name.as_str() == name && if let ImplItemKind::Method(ref sig, _) =
- item.node { is_self_sig(sig) } else { false }
+ item.name.as_str() == name &&
+ if let ImplItemKind::Method(ref sig, _) = item.node {
+ is_self_sig(sig)
+ } else {
+ false
+ }
}
if !impl_items.iter().any(|i| is_named_self(i, "is_empty")) {
for i in impl_items {
if is_named_self(i, "len") {
let s = i.span;
- span_lint(cx, LEN_WITHOUT_IS_EMPTY,
- Span{ lo: s.lo, hi: s.lo, expn_id: s.expn_id },
- &format!("item `{}` has a `.len(_: &Self)` method, but no \
- `.is_empty(_: &Self)` method. Consider adding one",
+ span_lint(cx,
+ LEN_WITHOUT_IS_EMPTY,
+ Span {
+ lo: s.lo,
+ hi: s.lo,
+ expn_id: s.expn_id,
+ },
+ &format!("item `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \
+ Consider adding one",
item.name));
return;
}
fn is_self_sig(sig: &MethodSig) -> bool {
if let SelfStatic = sig.explicit_self.node {
- false } else { sig.decl.inputs.len() == 1 }
+ false
+ } else {
+ sig.decl.inputs.len() == 1
+ }
}
fn check_cmp(cx: &LateContext, span: Span, left: &Expr, right: &Expr, op: &str) {
// check if we are in an is_empty() method
if let Some(name) = get_item_name(cx, left) {
- if name.as_str() == "is_empty" { return; }
+ if name.as_str() == "is_empty" {
+ return;
+ }
}
match (&left.node, &right.node) {
- (&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) =>
- check_len_zero(cx, span, &method.node, args, lit, op),
- (&ExprMethodCall(ref method, _, ref args), &ExprLit(ref lit)) =>
- check_len_zero(cx, span, &method.node, args, lit, op),
- _ => ()
+ (&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) => {
+ check_len_zero(cx, span, &method.node, args, lit, op)
+ }
+ (&ExprMethodCall(ref method, _, ref args), &ExprLit(ref lit)) => {
+ check_len_zero(cx, span, &method.node, args, lit, op)
+ }
+ _ => (),
}
}
-fn check_len_zero(cx: &LateContext, span: Span, name: &Name,
- args: &[P<Expr>], lit: &Lit, op: &str) {
+fn check_len_zero(cx: &LateContext, span: Span, name: &Name, args: &[P<Expr>], lit: &Lit, op: &str) {
if let Spanned{node: LitInt(0, _), ..} = *lit {
- if name.as_str() == "len" && args.len() == 1 &&
- has_is_empty(cx, &args[0]) {
- span_lint(cx, LEN_ZERO, span, &format!(
- "consider replacing the len comparison with `{}{}.is_empty()`",
- op, snippet(cx, args[0].span, "_")))
- }
+ if name.as_str() == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) {
+ span_lint(cx,
+ LEN_ZERO,
+ span,
+ &format!("consider replacing the len comparison with `{}{}.is_empty()`",
+ op,
+ snippet(cx, args[0].span, "_")));
+ }
}
}
/// get a ImplOrTraitItem and return true if it matches is_empty(self)
fn is_is_empty(cx: &LateContext, id: &ImplOrTraitItemId) -> bool {
if let MethodTraitItemId(def_id) = *id {
- if let ty::MethodTraitItem(ref method) =
- cx.tcx.impl_or_trait_item(def_id) {
- method.name.as_str() == "is_empty"
- && method.fty.sig.skip_binder().inputs.len() == 1
- } else { false }
- } else { false }
+ if let ty::MethodTraitItem(ref method) = cx.tcx.impl_or_trait_item(def_id) {
+ method.name.as_str() == "is_empty" && method.fty.sig.skip_binder().inputs.len() == 1
+ } else {
+ false
+ }
+ } else {
+ false
+ }
}
/// check the inherent impl's items for an is_empty(self) method
fn has_is_empty_impl(cx: &LateContext, id: &DefId) -> bool {
let impl_items = cx.tcx.impl_items.borrow();
- cx.tcx.inherent_impls.borrow().get(id).map_or(false,
- |ids| ids.iter().any(|iid| impl_items.get(iid).map_or(false,
- |iids| iids.iter().any(|i| is_is_empty(cx, i)))))
+ cx.tcx.inherent_impls.borrow().get(id).map_or(false, |ids| {
+ ids.iter().any(|iid| impl_items.get(iid).map_or(false, |iids| iids.iter().any(|i| is_is_empty(cx, i))))
+ })
}
let ty = &walk_ptrs_ty(&cx.tcx.expr_ty(expr));
match ty.sty {
- ty::TyTrait(_) => cx.tcx.trait_item_def_ids.borrow().get(
- &ty.ty_to_def_id().expect("trait impl not found")).map_or(false,
- |ids| ids.iter().any(|i| is_is_empty(cx, i))),
- ty::TyProjection(_) => ty.ty_to_def_id().map_or(false,
- |id| has_is_empty_impl(cx, &id)),
- ty::TyEnum(ref id, _) | ty::TyStruct(ref id, _) =>
- has_is_empty_impl(cx, &id.did),
+ ty::TyTrait(_) => {
+ cx.tcx
+ .trait_item_def_ids
+ .borrow()
+ .get(&ty.ty_to_def_id().expect("trait impl not found"))
+ .map_or(false, |ids| ids.iter().any(|i| is_is_empty(cx, i)))
+ }
+ ty::TyProjection(_) => ty.ty_to_def_id().map_or(false, |id| has_is_empty_impl(cx, &id)),
+ ty::TyEnum(ref id, _) | ty::TyStruct(ref id, _) => has_is_empty_impl(cx, &id.did),
ty::TyArray(..) => true,
_ => false,
}
#![feature(plugin_registrar, box_syntax)]
#![feature(rustc_private, collections)]
#![feature(num_bits_bytes, iter_arith)]
+#![feature(custom_attribute)]
#![allow(unknown_lints)]
// this only exists to allow the "dogfood" integration test to work
#[allow(dead_code)]
-fn main() { println!("What are you doing? Don't run clippy as an executable"); }
+fn main() {
+ println!("What are you doing? Don't run clippy as an executable");
+}
#[macro_use]
extern crate syntax;
// for unicode nfc normalization
extern crate unicode_normalization;
+// for semver check in attrs.rs
+extern crate semver;
+
extern crate rustc_plugin;
use rustc_plugin::Registry;
pub mod transmute;
pub mod cyclomatic_complexity;
pub mod escape;
+pub mod entry;
pub mod misc_early;
pub mod array_indexing;
pub mod panic;
pub use syntax::ast::{Name, NodeId};
}
+#[allow(unused_attributes)]
#[plugin_registrar]
+#[rustfmt_skip]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box types::TypePass);
reg.register_late_lint_pass(box misc::TopLevelRefPass);
reg.register_late_lint_pass(box types::UnitCmp);
reg.register_late_lint_pass(box loops::LoopsPass);
reg.register_late_lint_pass(box lifetimes::LifetimePass);
+ reg.register_late_lint_pass(box entry::HashMapLint);
reg.register_late_lint_pass(box ranges::StepByZero);
reg.register_late_lint_pass(box types::CastPass);
reg.register_late_lint_pass(box types::TypeComplexityPass);
reg.register_late_lint_pass(box array_indexing::ArrayIndexing);
reg.register_late_lint_pass(box panic::PanicPass);
+
reg.register_lint_group("clippy_pedantic", vec![
methods::OPTION_UNWRAP_USED,
methods::RESULT_UNWRAP_USED,
reg.register_lint_group("clippy", vec![
approx_const::APPROX_CONSTANT,
array_indexing::OUT_OF_BOUNDS_INDEXING,
+ attrs::DEPRECATED_SEMVER,
attrs::INLINE_ALWAYS,
bit_mask::BAD_BIT_MASK,
bit_mask::INEFFECTIVE_BIT_MASK,
block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT,
collapsible_if::COLLAPSIBLE_IF,
cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
+ entry::MAP_ENTRY,
eq_op::EQ_OP,
escape::BOXED_LOCAL,
eta_reduction::REDUNDANT_CLOSURE,
loops::WHILE_LET_ON_ITERATOR,
map_clone::MAP_CLONE,
matches::MATCH_BOOL,
+ matches::MATCH_OVERLAPPING_ARM,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,
+ methods::FILTER_NEXT,
methods::OK_EXPECT,
methods::OPTION_MAP_UNWRAP_OR,
methods::OPTION_MAP_UNWRAP_OR_ELSE,
+ methods::SEARCH_IS_SOME,
methods::SHOULD_IMPLEMENT_TRAIT,
methods::STR_TO_STRING,
methods::STRING_TO_STRING,
misc::REDUNDANT_PATTERN,
misc::TOPLEVEL_REF_ARG,
misc::USED_UNDERSCORE_BINDING,
+ misc_early::DUPLICATE_UNDERSCORE_ARGUMENT,
misc_early::UNNEEDED_FIELD_PATTERN,
mut_reference::UNNECESSARY_MUT_PASSED,
mutex_atomic::MUTEX_ATOMIC,
fn check_impl_item(&mut self, cx: &LateContext, item: &ImplItem) {
if let ImplItemKind::Method(ref sig, _) = item.node {
- check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self),
- &sig.generics, item.span);
+ check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self), &sig.generics, item.span);
}
}
fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) {
if let MethodTraitItem(ref sig, _) = item.node {
- check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self),
- &sig.generics, item.span);
+ check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self), &sig.generics, item.span);
}
}
}
}
use self::RefLt::*;
-fn check_fn_inner(cx: &LateContext, decl: &FnDecl, slf: Option<&ExplicitSelf>,
- generics: &Generics, span: Span) {
+fn check_fn_inner(cx: &LateContext, decl: &FnDecl, slf: Option<&ExplicitSelf>, generics: &Generics, span: Span) {
if in_external_macro(cx, span) || has_where_lifetimes(cx, &generics.where_clause) {
return;
}
if could_use_elision(cx, decl, slf, &generics.lifetimes) {
- span_lint(cx, NEEDLESS_LIFETIMES, span,
+ span_lint(cx,
+ NEEDLESS_LIFETIMES,
+ span,
"explicit lifetimes given in parameter types where they could be elided");
}
- report_extra_lifetimes(cx, decl, &generics);
+ report_extra_lifetimes(cx, decl, &generics, slf);
}
-fn could_use_elision(cx: &LateContext, func: &FnDecl, slf: Option<&ExplicitSelf>,
- named_lts: &[LifetimeDef]) -> bool {
+fn could_use_elision(cx: &LateContext, func: &FnDecl, slf: Option<&ExplicitSelf>, named_lts: &[LifetimeDef]) -> bool {
// There are two scenarios where elision works:
// * no output references, all input references have different LT
// * output references, exactly one input reference with same LT
match slf.node {
SelfRegion(ref opt_lt, _, _) => input_visitor.record(opt_lt),
SelfExplicit(ref ty, _) => walk_ty(&mut input_visitor, ty),
- _ => { }
+ _ => {}
}
}
// extract lifetimes in input argument types
(&Named(n1), &Named(n2)) if n1 == n2 => true,
(&Named(_), &Unnamed) => true,
(&Unnamed, &Named(_)) => true,
- _ => false // already elided, different named lifetimes
- // or something static going on
+ _ => false, // already elided, different named lifetimes
+ // or something static going on
}
} else {
false
/// A visitor usable for rustc_front::visit::walk_ty().
struct RefVisitor<'v, 't: 'v> {
cx: &'v LateContext<'v, 't>, // context reference
- lts: Vec<RefLt>
+ lts: Vec<RefLt>,
}
-impl <'v, 't> RefVisitor<'v, 't> {
+impl<'v, 't> RefVisitor<'v, 't> {
fn new(cx: &'v LateContext<'v, 't>) -> RefVisitor<'v, 't> {
- RefVisitor { cx: cx, lts: Vec::new() }
+ RefVisitor {
+ cx: cx,
+ lts: Vec::new(),
+ }
}
fn record(&mut self, lifetime: &Option<Lifetime>) {
for _ in type_scheme.generics.regions.as_slice() {
self.record(&None);
}
- },
+ }
DefTrait(def_id) => {
let trait_def = self.cx.tcx.trait_defs.borrow()[&def_id];
for _ in &trait_def.generics.regions {
self.record(&None);
}
- },
+ }
_ => {}
}
}
}
impl<'v, 't> Visitor<'v> for RefVisitor<'v, 't> {
-
// for lifetimes as parameters of generics
fn visit_lifetime(&mut self, lifetime: &'v Lifetime) {
self.record(&Some(*lifetime));
let mut visitor = RefVisitor::new(cx);
// walk the type F, it may not contain LT refs
walk_ty(&mut visitor, &pred.bounded_ty);
- if !visitor.lts.is_empty() { return true; }
+ if !visitor.lts.is_empty() {
+ return true;
+ }
// if the bounds define new lifetimes, they are fine to occur
let allowed_lts = allowed_lts_from(&pred.bound_lifetimes);
// now walk the bounds
WherePredicate::EqPredicate(ref pred) => {
let mut visitor = RefVisitor::new(cx);
walk_ty(&mut visitor, &pred.ty);
- if !visitor.lts.is_empty() { return true; }
+ if !visitor.lts.is_empty() {
+ return true;
+ }
}
}
}
struct LifetimeChecker(HashMap<Name, Span>);
impl<'v> Visitor<'v> for LifetimeChecker {
-
// for lifetimes as parameters of generics
fn visit_lifetime(&mut self, lifetime: &'v Lifetime) {
self.0.remove(&lifetime.name);
}
fn report_extra_lifetimes(cx: &LateContext, func: &FnDecl,
- generics: &Generics) {
- let hs = generics.lifetimes.iter()
+ generics: &Generics, slf: Option<&ExplicitSelf>) {
+ let hs = generics.lifetimes
+ .iter()
.map(|lt| (lt.lifetime.name, lt.lifetime.span))
.collect();
let mut checker = LifetimeChecker(hs);
+
walk_generics(&mut checker, generics);
walk_fn_decl(&mut checker, func);
+
+ if let Some(slf) = slf {
+ match slf.node {
+ SelfRegion(Some(ref lt), _, _) => checker.visit_lifetime(lt),
+ SelfExplicit(ref t, _) => walk_ty(&mut checker, t),
+ _ => {}
+ }
+ }
+
for (_, v) in checker.0 {
- span_lint(cx, UNUSED_LIFETIMES, v,
- "this lifetime isn't used in the function definition");
+ span_lint(cx, UNUSED_LIFETIMES, v, "this lifetime isn't used in the function definition");
}
}
use rustc::middle::ty;
use rustc::middle::def::DefLocal;
use consts::{constant_simple, Constant};
-use rustc::front::map::Node::{NodeBlock};
+use rustc::front::map::Node::NodeBlock;
use std::borrow::Cow;
-use std::collections::{HashSet,HashMap};
-use syntax::ast::Lit_::*;
+use std::collections::{HashSet, HashMap};
-use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type,
- in_external_macro, expr_block, span_help_and_lint, is_integer_literal,
- get_enclosing_block};
-use utils::{VEC_PATH, LL_PATH};
+use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro, expr_block,
+ span_help_and_lint, is_integer_literal, get_enclosing_block};
+use utils::{HASHMAP_PATH, VEC_PATH, LL_PATH};
/// **What it does:** This lint checks for looping over the range of `0..len` of some collection just to get the values by index. It is `Warn` by default.
///
declare_lint!{ pub ITER_NEXT_LOOP, Warn,
"for-looping over `_.next()` which is probably not intended" }
-/// **What it does:** This lint detects `loop + match` combinations that are easier written as a `while let` loop.
+/// **What it does:** This lint detects `loop + match` combinations that are easier written as a `while let` loop. It is `Warn` by default.
///
/// **Why is this bad?** The `while let` loop is usually shorter and more readable
///
"`collect()`ing an iterator without using the result; this is usually better \
written as a for loop" }
-/// **What it does:** This lint checks for loops over ranges `x..y` where both `x` and `y` are constant and `x` is greater or equal to `y`, unless the range is reversed or has a negative `.step_by(_)`.
+/// **What it does:** This lint checks for loops over ranges `x..y` where both `x` and `y` are constant and `x` is greater or equal to `y`, unless the range is reversed or has a negative `.step_by(_)`. It is `Warn` by default.
///
/// **Why is it bad?** Such loops will either be skipped or loop until wrap-around (in debug code, this may `panic!()`). Both options are probably not intended.
///
impl LintPass for LoopsPass {
fn get_lints(&self) -> LintArray {
- lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP,
- WHILE_LET_LOOP, UNUSED_COLLECT, REVERSE_RANGE_LOOP,
- EXPLICIT_COUNTER_LOOP, EMPTY_LOOP,
+ lint_array!(NEEDLESS_RANGE_LOOP,
+ EXPLICIT_ITER_LOOP,
+ ITER_NEXT_LOOP,
+ WHILE_LET_LOOP,
+ UNUSED_COLLECT,
+ REVERSE_RANGE_LOOP,
+ EXPLICIT_COUNTER_LOOP,
+ EMPTY_LOOP,
WHILE_LET_ON_ITERATOR)
}
}
if let ExprLoop(ref block, _) = expr.node {
// also check for empty `loop {}` statements
if block.stmts.is_empty() && block.expr.is_none() {
- span_lint(cx, EMPTY_LOOP, expr.span,
- "empty `loop {}` detected. You may want to either \
- use `panic!()` or add `std::thread::sleep(..);` to \
- the loop body.");
+ span_lint(cx,
+ EMPTY_LOOP,
+ expr.span,
+ "empty `loop {}` detected. You may want to either use `panic!()` or add \
+ `std::thread::sleep(..);` to the loop body.");
}
// extract the expression from the first statement (if any) in a block
if let ExprMatch(ref matchexpr, ref arms, ref source) = inner.node {
// collect the remaining statements below the match
let mut other_stuff = block.stmts
- .iter()
- .skip(1)
- .map(|stmt| {
- format!("{}", snippet(cx, stmt.span, ".."))
- }).collect::<Vec<String>>();
+ .iter()
+ .skip(1)
+ .map(|stmt| format!("{}", snippet(cx, stmt.span, "..")))
+ .collect::<Vec<String>>();
if inner_stmt_expr.is_some() {
// if we have a statement which has a match,
if let Some(ref expr) = block.expr {
// ensure "if let" compatible match structure
match *source {
- MatchSource::Normal | MatchSource::IfLetDesugar{..} => if
- arms.len() == 2 &&
- arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
- arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
- // finally, check for "break" in the second clause
- is_break_expr(&arms[1].body)
- {
- if in_external_macro(cx, expr.span) { return; }
- let loop_body = if inner_stmt_expr.is_some() {
- // FIXME: should probably be an ellipsis
- // tabbing and newline is probably a bad idea, especially for large blocks
- Cow::Owned(format!("{{\n {}\n}}", other_stuff.join("\n ")))
- } else {
- expr_block(cx, &arms[0].body, Some(other_stuff.join("\n ")), "..")
- };
- span_help_and_lint(cx, WHILE_LET_LOOP, expr.span,
- "this loop could be written as a `while let` loop",
- &format!("try\nwhile let {} = {} {}",
- snippet(cx, arms[0].pats[0].span, ".."),
- snippet(cx, matchexpr.span, ".."),
- loop_body));
- },
- _ => ()
+ MatchSource::Normal | MatchSource::IfLetDesugar{..} => {
+ if arms.len() == 2 && arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
+ arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
+ is_break_expr(&arms[1].body) {
+ if in_external_macro(cx, expr.span) {
+ return;
+ }
+ let loop_body = if inner_stmt_expr.is_some() {
+ // FIXME: should probably be an ellipsis
+ // tabbing and newline is probably a bad idea, especially for large blocks
+ Cow::Owned(format!("{{\n {}\n}}", other_stuff.join("\n ")))
+ } else {
+ expr_block(cx, &arms[0].body, Some(other_stuff.join("\n ")), "..")
+ };
+ span_help_and_lint(cx,
+ WHILE_LET_LOOP,
+ expr.span,
+ "this loop could be written as a `while let` loop",
+ &format!("try\nwhile let {} = {} {}",
+ snippet(cx, arms[0].pats[0].span, ".."),
+ snippet(cx, matchexpr.span, ".."),
+ loop_body));
+ }
+ }
+ _ => (),
}
}
}
if let ExprMatch(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
let pat = &arms[0].pats[0].node;
if let (&PatEnum(ref path, Some(ref pat_args)),
- &ExprMethodCall(method_name, _, ref method_args)) =
- (pat, &match_expr.node) {
+ &ExprMethodCall(method_name, _, ref method_args)) = (pat, &match_expr.node) {
let iter_expr = &method_args[0];
if let Some(lhs_constructor) = path.segments.last() {
if method_name.node.as_str() == "next" &&
- match_trait_method(cx, match_expr, &["core", "iter", "Iterator"]) &&
- lhs_constructor.identifier.name.as_str() == "Some" &&
- !is_iterator_used_after_while_let(cx, iter_expr) {
+ match_trait_method(cx, match_expr, &["core", "iter", "Iterator"]) &&
+ lhs_constructor.identifier.name.as_str() == "Some" &&
+ !is_iterator_used_after_while_let(cx, iter_expr) {
let iterator = snippet(cx, method_args[0].span, "_");
let loop_var = snippet(cx, pat_args[0].span, "_");
- span_help_and_lint(cx, WHILE_LET_ON_ITERATOR, expr.span,
+ span_help_and_lint(cx,
+ WHILE_LET_ON_ITERATOR,
+ expr.span,
"this loop could be written as a `for` loop",
- &format!("try\nfor {} in {} {{...}}",
- loop_var,
- iterator));
+ &format!("try\nfor {} in {} {{...}}", loop_var, iterator));
}
}
}
if let StmtSemi(ref expr, _) = stmt.node {
if let ExprMethodCall(ref method, _, ref args) = expr.node {
if args.len() == 1 && method.node.as_str() == "collect" &&
- match_trait_method(cx, expr, &["core", "iter", "Iterator"]) {
- span_lint(cx, UNUSED_COLLECT, expr.span, &format!(
- "you are collect()ing an iterator and throwing away the result. \
- Consider using an explicit for loop to exhaust the iterator"));
+ match_trait_method(cx, expr, &["core", "iter", "Iterator"]) {
+ span_lint(cx,
+ UNUSED_COLLECT,
+ expr.span,
+ &format!("you are collect()ing an iterator and throwing away the result. Consider \
+ using an explicit for loop to exhaust the iterator"));
}
}
}
}
fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) {
- // check for looping over a range and then indexing a sequence with it
- // -> the iteratee must be a range literal
- if let ExprRange(Some(ref l), _) = arg.node {
- // Range should start with `0`
- if let ExprLit(ref lit) = l.node {
- if let LitInt(0, _) = lit.node {
-
- // the var must be a single name
- if let PatIdent(_, ref ident, _) = pat.node {
- let mut visitor = VarVisitor { cx: cx, var: ident.node.name,
- indexed: HashSet::new(), nonindex: false };
- walk_expr(&mut visitor, body);
- // linting condition: we only indexed one variable
- if visitor.indexed.len() == 1 {
- let indexed = visitor.indexed.into_iter().next().expect(
- "Len was nonzero, but no contents found");
- if visitor.nonindex {
- span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!(
- "the loop variable `{}` is used to index `{}`. Consider using \
- `for ({}, item) in {}.iter().enumerate()` or similar iterators",
- ident.node.name, indexed, ident.node.name, indexed));
- } else {
- span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!(
- "the loop variable `{}` is only used to index `{}`. \
- Consider using `for item in &{}` or similar iterators",
- ident.node.name, indexed, indexed));
- }
+ check_for_loop_range(cx, pat, arg, body, expr);
+ check_for_loop_reverse_range(cx, arg, expr);
+ check_for_loop_explicit_iter(cx, arg, expr);
+ check_for_loop_explicit_counter(cx, arg, body, expr);
+}
+
+/// Check for looping over a range and then indexing a sequence with it.
+/// The iteratee must be a range literal.
+fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) {
+ if let ExprRange(Some(ref l), ref r) = arg.node {
+ // the var must be a single name
+ if let PatIdent(_, ref ident, _) = pat.node {
+ let mut visitor = VarVisitor {
+ cx: cx,
+ var: ident.node.name,
+ indexed: HashSet::new(),
+ nonindex: false,
+ };
+ walk_expr(&mut visitor, body);
+ // linting condition: we only indexed one variable
+ if visitor.indexed.len() == 1 {
+ let indexed = visitor.indexed
+ .into_iter()
+ .next()
+ .expect("Len was nonzero, but no contents found");
+
+ let starts_at_zero = is_integer_literal(l, 0);
+
+ let skip: Cow<_> = if starts_at_zero {
+ "".into()
+ }
+ else {
+ format!(".skip({})", snippet(cx, l.span, "..")).into()
+ };
+
+ let take: Cow<_> = if let Some(ref r) = *r {
+ if !is_len_call(&r, &indexed) {
+ format!(".take({})", snippet(cx, r.span, "..")).into()
+ }
+ else {
+ "".into()
}
+ } else {
+ "".into()
+ };
+
+ if visitor.nonindex {
+ span_lint(cx,
+ NEEDLESS_RANGE_LOOP,
+ expr.span,
+ &format!("the loop variable `{}` is used to index `{}`. \
+ Consider using `for ({}, item) in {}.iter().enumerate(){}{}` or similar iterators",
+ ident.node.name,
+ indexed,
+ ident.node.name,
+ indexed,
+ take,
+ skip));
+ } else {
+ let repl = if starts_at_zero && take.is_empty() {
+ format!("&{}", indexed)
+ }
+ else {
+ format!("{}.iter(){}{}", indexed, take, skip)
+ };
+
+ span_lint(cx,
+ NEEDLESS_RANGE_LOOP,
+ expr.span,
+ &format!("the loop variable `{}` is only used to index `{}`. \
+ Consider using `for item in {}` or similar iterators",
+ ident.node.name,
+ indexed,
+ repl));
}
}
}
}
+}
+fn is_len_call(expr: &Expr, var: &Name) -> bool {
+ if_let_chain! {[
+ let ExprMethodCall(method, _, ref len_args) = expr.node,
+ len_args.len() == 1,
+ method.node.as_str() == "len",
+ let ExprPath(_, ref path) = len_args[0].node,
+ path.segments.len() == 1,
+ &path.segments[0].identifier.name == var
+ ], {
+ return true;
+ }}
+
+ false
+}
+
+fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) {
// if this for loop is iterating over a two-sided range...
if let ExprRange(Some(ref start_expr), Some(ref stop_expr)) = arg.node {
// ...and both sides are compile-time constant integers...
// who think that this will iterate from the larger value to the
// smaller value.
if start_idx > stop_idx {
- span_help_and_lint(cx, REVERSE_RANGE_LOOP, expr.span,
- "this range is empty so this for loop will never run",
- &format!("Consider using `({}..{}).rev()` if you are attempting to \
- iterate over this range in reverse", stop_idx, start_idx));
+ span_help_and_lint(cx,
+ REVERSE_RANGE_LOOP,
+ expr.span,
+ "this range is empty so this for loop will never run",
+ &format!("Consider using `({}..{}).rev()` if you are attempting to iterate \
+ over this range in reverse",
+ stop_idx,
+ start_idx));
} else if start_idx == stop_idx {
// if they are equal, it's also problematic - this loop
// will never run.
- span_lint(cx, REVERSE_RANGE_LOOP, expr.span,
- "this range is empty so this for loop will never run");
+ span_lint(cx,
+ REVERSE_RANGE_LOOP,
+ expr.span,
+ "this range is empty so this for loop will never run");
}
}
}
}
+}
+fn check_for_loop_explicit_iter(cx: &LateContext, arg: &Expr, expr: &Expr) {
if let ExprMethodCall(ref method, _, ref args) = arg.node {
// just the receiver, no arguments
if args.len() == 1 {
if method_name.as_str() == "iter" || method_name.as_str() == "iter_mut" {
if is_ref_iterable_type(cx, &args[0]) {
let object = snippet(cx, args[0].span, "_");
- span_lint(cx, EXPLICIT_ITER_LOOP, expr.span, &format!(
- "it is more idiomatic to loop over `&{}{}` instead of `{}.{}()`",
- if method_name.as_str() == "iter_mut" { "mut " } else { "" },
- object, object, method_name));
+ span_lint(cx,
+ EXPLICIT_ITER_LOOP,
+ expr.span,
+ &format!("it is more idiomatic to loop over `&{}{}` instead of `{}.{}()`",
+ if method_name.as_str() == "iter_mut" {
+ "mut "
+ } else {
+ ""
+ },
+ object,
+ object,
+ method_name));
}
- }
- // check for looping over Iterator::next() which is not what you want
- else if method_name.as_str() == "next" &&
- match_trait_method(cx, arg, &["core", "iter", "Iterator"]) {
- span_lint(cx, ITER_NEXT_LOOP, expr.span,
- "you are iterating over `Iterator::next()` which is an Option; \
- this will compile but is probably not what you want");
+ } else if method_name.as_str() == "next" && match_trait_method(cx, arg, &["core", "iter", "Iterator"]) {
+ span_lint(cx,
+ ITER_NEXT_LOOP,
+ expr.span,
+ "you are iterating over `Iterator::next()` which is an Option; this will compile but is \
+ probably not what you want");
}
}
}
+}
+
+fn check_for_loop_explicit_counter(cx: &LateContext, arg: &Expr, body: &Expr, expr: &Expr) {
// Look for variables that are incremented once per loop iteration.
- let mut visitor = IncrementVisitor { cx: cx, states: HashMap::new(), depth: 0, done: false };
+ let mut visitor = IncrementVisitor {
+ cx: cx,
+ states: HashMap::new(),
+ depth: 0,
+ done: false,
+ };
walk_expr(&mut visitor, body);
// For each candidate, check the parent block to see if
// it's initialized to zero at the start of the loop.
let map = &cx.tcx.map;
- let parent_scope = map.get_enclosing_scope(expr.id).and_then(|id| map.get_enclosing_scope(id) );
+ let parent_scope = map.get_enclosing_scope(expr.id).and_then(|id| map.get_enclosing_scope(id));
if let Some(parent_id) = parent_scope {
if let NodeBlock(block) = map.get(parent_id) {
- for (id, _) in visitor.states.iter().filter( |&(_,v)| *v == VarState::IncrOnce) {
- let mut visitor2 = InitializeVisitor { cx: cx, end_expr: expr, var_id: id.clone(),
- state: VarState::IncrOnce, name: None,
- depth: 0,
- past_loop: false };
+ for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) {
+ let mut visitor2 = InitializeVisitor {
+ cx: cx,
+ end_expr: expr,
+ var_id: id.clone(),
+ state: VarState::IncrOnce,
+ name: None,
+ depth: 0,
+ past_loop: false,
+ };
walk_block(&mut visitor2, block);
if visitor2.state == VarState::Warn {
if let Some(name) = visitor2.name {
- span_lint(cx, EXPLICIT_COUNTER_LOOP, expr.span,
- &format!("the variable `{0}` is used as a loop counter. Consider \
- using `for ({0}, item) in {1}.enumerate()` \
- or similar iterators",
- name, snippet(cx, arg.span, "_")));
+ span_lint(cx,
+ EXPLICIT_COUNTER_LOOP,
+ expr.span,
+ &format!("the variable `{0}` is used as a loop counter. Consider using `for ({0}, \
+ item) in {1}.enumerate()` or similar iterators",
+ name,
+ snippet(cx, arg.span, "_")));
}
}
}
struct VarVisitor<'v, 't: 'v> {
cx: &'v LateContext<'v, 't>, // context reference
- var: Name, // var name to look for as index
- indexed: HashSet<Name>, // indexed variables
- nonindex: bool, // has the var been used otherwise?
+ var: Name, // var name to look for as index
+ indexed: HashSet<Name>, // indexed variables
+ nonindex: bool, // has the var been used otherwise?
}
impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> {
fn is_iterator_used_after_while_let(cx: &LateContext, iter_expr: &Expr) -> bool {
let def_id = match var_def_id(cx, iter_expr) {
Some(id) => id,
- None => return false
+ None => return false,
};
let mut visitor = VarUsedAfterLoopVisitor {
cx: cx,
def_id: def_id,
iter_expr_id: iter_expr.id,
past_while_let: false,
- var_used_after_while_let: false
+ var_used_after_while_let: false,
};
if let Some(enclosing_block) = get_enclosing_block(cx, def_id) {
walk_block(&mut visitor, enclosing_block);
def_id: NodeId,
iter_expr_id: NodeId,
past_while_let: bool,
- var_used_after_while_let: bool
+ var_used_after_while_let: bool,
}
-impl <'v, 't> Visitor<'v> for VarUsedAfterLoopVisitor<'v, 't> {
+impl<'v, 't> Visitor<'v> for VarUsedAfterLoopVisitor<'v, 't> {
fn visit_expr(&mut self, expr: &'v Expr) {
if self.past_while_let {
if Some(self.def_id) == var_def_id(self.cx, expr) {
// no walk_ptrs_ty: calling iter() on a reference can make sense because it
// will allow further borrows afterwards
let ty = cx.tcx.expr_ty(e);
- is_iterable_array(ty) ||
- match_type(cx, ty, &VEC_PATH) ||
- match_type(cx, ty, &LL_PATH) ||
- match_type(cx, ty, &["std", "collections", "hash", "map", "HashMap"]) ||
- match_type(cx, ty, &["std", "collections", "hash", "set", "HashSet"]) ||
- match_type(cx, ty, &["collections", "vec_deque", "VecDeque"]) ||
- match_type(cx, ty, &["collections", "binary_heap", "BinaryHeap"]) ||
- match_type(cx, ty, &["collections", "btree", "map", "BTreeMap"]) ||
- match_type(cx, ty, &["collections", "btree", "set", "BTreeSet"])
+ is_iterable_array(ty) || match_type(cx, ty, &VEC_PATH) || match_type(cx, ty, &LL_PATH) ||
+ match_type(cx, ty, &HASHMAP_PATH) || match_type(cx, ty, &["std", "collections", "hash", "set", "HashSet"]) ||
+ match_type(cx, ty, &["collections", "vec_deque", "VecDeque"]) ||
+ match_type(cx, ty, &["collections", "binary_heap", "BinaryHeap"]) ||
+ match_type(cx, ty, &["collections", "btree", "map", "BTreeMap"]) ||
+ match_type(cx, ty, &["collections", "btree", "set", "BTreeSet"])
}
fn is_iterable_array(ty: ty::Ty) -> bool {
// IntoIterator is currently only implemented for array sizes <= 32 in rustc
match ty.sty {
ty::TyArray(_, 0...32) => true,
- _ => false
+ _ => false,
}
}
/// If a block begins with a statement (possibly a `let` binding) and has an expression, return it.
fn extract_expr_from_first_stmt(block: &Block) -> Option<&Expr> {
- if block.stmts.is_empty() { return None; }
+ if block.stmts.is_empty() {
+ return None;
+ }
if let StmtDecl(ref decl, _) = block.stmts[0].node {
if let DeclLocal(ref local) = decl.node {
- if let Some(ref expr) = local.init { Some(expr) } else { None }
- } else { None }
- } else { None }
+ if let Some(ref expr) = local.init {
+ Some(expr)
+ } else {
+ None
+ }
+ } else {
+ None
+ }
+ } else {
+ None
+ }
}
/// If a block begins with an expression (with or without semicolon), return it.
fn extract_first_expr(block: &Block) -> Option<&Expr> {
match block.expr {
Some(ref expr) => Some(expr),
- None if !block.stmts.is_empty() => match block.stmts[0].node {
- StmtExpr(ref expr, _) | StmtSemi(ref expr, _) => Some(expr),
- _ => None,
- },
+ None if !block.stmts.is_empty() => {
+ match block.stmts[0].node {
+ StmtExpr(ref expr, _) | StmtSemi(ref expr, _) => Some(expr),
+ _ => None,
+ }
+ }
_ => None,
}
}
match expr.node {
ExprBreak(None) => true,
// there won't be a `let <pat> = break` and so we can safely ignore the StmtDecl case
- ExprBlock(ref b) => match extract_first_expr(b) {
- Some(ref subexpr) => is_break_expr(subexpr),
- None => false,
- },
+ ExprBlock(ref b) => {
+ match extract_first_expr(b) {
+ Some(ref subexpr) => is_break_expr(subexpr),
+ None => false,
+ }
+ }
_ => false,
}
}
// at the start of the loop.
#[derive(PartialEq)]
enum VarState {
- Initial, // Not examined yet
- IncrOnce, // Incremented exactly once, may be a loop counter
- Declared, // Declared but not (yet) initialized to zero
+ Initial, // Not examined yet
+ IncrOnce, // Incremented exactly once, may be a loop counter
+ Declared, // Declared but not (yet) initialized to zero
Warn,
- DontWarn
+ DontWarn,
}
// Scan a for loop for variables that are incremented exactly once.
struct IncrementVisitor<'v, 't: 'v> {
- cx: &'v LateContext<'v, 't>, // context reference
- states: HashMap<NodeId, VarState>, // incremented variables
- depth: u32, // depth of conditional expressions
- done: bool
+ cx: &'v LateContext<'v, 't>, // context reference
+ states: HashMap<NodeId, VarState>, // incremented variables
+ depth: u32, // depth of conditional expressions
+ done: bool,
}
impl<'v, 't> Visitor<'v> for IncrementVisitor<'v, 't> {
let state = self.states.entry(def_id).or_insert(VarState::Initial);
match parent.node {
- ExprAssignOp(op, ref lhs, ref rhs) =>
+ ExprAssignOp(op, ref lhs, ref rhs) => {
if lhs.id == expr.id {
if op.node == BiAdd && is_integer_literal(rhs, 1) {
*state = match *state {
VarState::Initial if self.depth == 0 => VarState::IncrOnce,
- _ => VarState::DontWarn
+ _ => VarState::DontWarn,
};
- }
- else {
+ } else {
// Assigned some other value
*state = VarState::DontWarn;
}
- },
+ }
+ }
ExprAssign(ref lhs, _) if lhs.id == expr.id => *state = VarState::DontWarn,
- ExprAddrOf(mutability,_) if mutability == MutMutable => *state = VarState::DontWarn,
- _ => ()
+ ExprAddrOf(mutability, _) if mutability == MutMutable => *state = VarState::DontWarn,
+ _ => (),
}
}
- }
- // Give up if there are nested loops
- else if is_loop(expr) {
+ } else if is_loop(expr) {
self.states.clear();
self.done = true;
return;
- }
- // Keep track of whether we're inside a conditional expression
- else if is_conditional(expr) {
+ } else if is_conditional(expr) {
self.depth += 1;
walk_expr(self, expr);
self.depth -= 1;
// Check whether a variable is initialized to zero at the start of a loop.
struct InitializeVisitor<'v, 't: 'v> {
cx: &'v LateContext<'v, 't>, // context reference
- end_expr: &'v Expr, // the for loop. Stop scanning here.
+ end_expr: &'v Expr, // the for loop. Stop scanning here.
var_id: NodeId,
state: VarState,
name: Option<Name>,
- depth: u32, // depth of conditional expressions
- past_loop: bool
+ depth: u32, // depth of conditional expressions
+ past_loop: bool,
}
impl<'v, 't> Visitor<'v> for InitializeVisitor<'v, 't> {
} else {
VarState::Declared
}
- }
- else {
+ } else {
VarState::Declared
}
}
VarState::Warn
} else {
VarState::DontWarn
- }}
- ExprAddrOf(mutability,_) if mutability == MutMutable => self.state = VarState::DontWarn,
- _ => ()
+ }
+ }
+ ExprAddrOf(mutability, _) if mutability == MutMutable => self.state = VarState::DontWarn,
+ _ => (),
}
}
self.state = VarState::DontWarn;
return;
}
- }
- // If there are other loops between the declaration and the target loop, give up
- else if !self.past_loop && is_loop(expr) {
+ } else if !self.past_loop && is_loop(expr) {
self.state = VarState::DontWarn;
return;
- }
- // Keep track of whether we're inside a conditional expression
- else if is_conditional(expr) {
+ } else if is_conditional(expr) {
self.depth += 1;
walk_expr(self, expr);
self.depth -= 1;
fn var_def_id(cx: &LateContext, expr: &Expr) -> Option<NodeId> {
if let Some(path_res) = cx.tcx.def_map.borrow().get(&expr.id) {
if let DefLocal(_, node_id) = path_res.base_def {
- return Some(node_id)
+ return Some(node_id);
}
}
None
fn is_loop(expr: &Expr) -> bool {
match expr.node {
- ExprLoop(..) | ExprWhile(..) => true,
- _ => false
+ ExprLoop(..) | ExprWhile(..) => true,
+ _ => false,
}
}
fn is_conditional(expr: &Expr) -> bool {
match expr.node {
ExprIf(..) | ExprMatch(..) => true,
- _ => false
+ _ => false,
}
}
///
/// **Why is this bad?** It makes the code less readable.
///
-/// **Known problems:** False negative: The lint currently misses mapping `Clone::clone` directly. Issue #436 is tracking this.
+/// **Known problems:** None
///
/// **Example:** `x.map(|e| e.clone());`
declare_lint!(pub MAP_CLONE, Warn,
ExprPath(_, ref path) => {
if match_path(path, &CLONE_PATH) {
let type_name = get_type_name(cx, expr, &args[0]).unwrap_or("_");
- span_help_and_lint(cx, MAP_CLONE, expr.span, &format!(
- "you seem to be using .map() to clone the contents of an {}, consider \
- using `.cloned()`", type_name),
- &format!("try\n{}.cloned()", snippet(cx, args[0].span, "..")));
+ span_help_and_lint(cx,
+ MAP_CLONE,
+ expr.span,
+ &format!("you seem to be using .map() to clone the contents of an \
+ {}, consider using `.cloned()`",
+ type_name),
+ &format!("try\n{}.cloned()", snippet(cx, args[0].span, "..")));
}
}
_ => (),
fn expr_eq_ident(expr: &Expr, id: Ident) -> bool {
match expr.node {
ExprPath(None, ref path) => {
- let arg_segment = [PathSegment { identifier: id, parameters: PathParameters::none() }];
+ let arg_segment = [PathSegment {
+ identifier: id,
+ parameters: PathParameters::none(),
+ }];
!path.global && path.segments[..] == arg_segment
}
_ => false,
fn only_derefs(cx: &LateContext, expr: &Expr, id: Ident) -> bool {
match expr.node {
- ExprUnary(UnDeref, ref subexpr) if !is_adjusted(cx, subexpr) => {
- only_derefs(cx, subexpr, id)
- }
+ ExprUnary(UnDeref, ref subexpr) if !is_adjusted(cx, subexpr) => only_derefs(cx, subexpr, id),
_ => expr_eq_ident(expr, id),
}
}
use rustc::lint::*;
-use rustc_front::hir::*;
+use rustc::middle::const_eval::ConstVal::{Int, Uint};
+use rustc::middle::const_eval::EvalHint::ExprTypeChecked;
+use rustc::middle::const_eval::{eval_const_expr_partial, ConstVal};
use rustc::middle::ty;
+use rustc_front::hir::*;
+use std::cmp::Ordering;
use syntax::ast::Lit_::LitBool;
use syntax::codemap::Span;
-use utils::{snippet, span_lint, span_help_and_lint, in_external_macro, expr_block};
+use utils::{COW_PATH, OPTION_PATH, RESULT_PATH};
+use utils::{match_type, snippet, span_lint, span_note_and_lint, span_lint_and_then, in_external_macro, expr_block};
/// **What it does:** This lint checks for matches with a single arm where an `if let` will usually suffice. It is `Warn` by default.
///
declare_lint!(pub SINGLE_MATCH, Warn,
"a match statement with a single nontrivial arm (i.e, where the other arm \
is `_ => {}`) is used; recommends `if let` instead");
-/// **What it does:** This lint checks for matches where all arms match a reference, suggesting to remove the reference and deref the matched expression instead. It is `Warn` by default.
+
+/// **What it does:** This lint checks for matches where all arms match a reference, suggesting to remove the reference and deref the matched expression instead. It also checks for `if let &foo = bar` blocks. It is `Warn` by default.
///
/// **Why is this bad?** It just makes the code less readable. That reference destructuring adds nothing to the code.
///
/// }
/// ```
declare_lint!(pub MATCH_REF_PATS, Warn,
- "a match has all arms prefixed with `&`; the match expression can be \
+ "a match or `if let` has all arms prefixed with `&`; the match expression can be \
dereferenced instead");
+
/// **What it does:** This lint checks for matches where match expression is a `bool`. It suggests to replace the expression with an `if...else` block. It is `Warn` by default.
///
/// **Why is this bad?** It makes the code less readable.
declare_lint!(pub MATCH_BOOL, Warn,
"a match on boolean expression; recommends `if..else` block instead");
+/// **What it does:** This lint checks for overlapping match arms. It is `Warn` by default.
+///
+/// **Why is this bad?** It is likely to be an error and if not, makes the code less obvious.
+///
+/// **Known problems:** None
+///
+/// **Example:**
+///
+/// ```
+/// let x = 5;
+/// match x {
+/// 1 ... 10 => println!("1 ... 10"),
+/// 5 ... 15 => println!("5 ... 15"),
+/// _ => (),
+/// }
+/// ```
+declare_lint!(pub MATCH_OVERLAPPING_ARM, Warn,
+ "a match has overlapping arms");
+
#[allow(missing_copy_implementations)]
pub struct MatchPass;
impl LateLintPass for MatchPass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
- if in_external_macro(cx, expr.span) { return; }
+ if in_external_macro(cx, expr.span) {
+ return;
+ }
if let ExprMatch(ref ex, ref arms, MatchSource::Normal) = expr.node {
- // check preconditions for SINGLE_MATCH
- // only two arms
- if arms.len() == 2 &&
- // both of the arms have a single pattern and no guard
- arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
- arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
- // and the second pattern is a `_` wildcard: this is not strictly necessary,
- // since the exhaustiveness check will ensure the last one is a catch-all,
- // but in some cases, an explicit match is preferred to catch situations
- // when an enum is extended, so we don't consider these cases
- arms[1].pats[0].node == PatWild &&
- // we don't want any content in the second arm (unit or empty block)
- is_unit_expr(&arms[1].body) &&
- // finally, MATCH_BOOL doesn't apply here
- (cx.tcx.expr_ty(ex).sty != ty::TyBool || cx.current_level(MATCH_BOOL) == Allow)
- {
- span_help_and_lint(cx, SINGLE_MATCH, expr.span,
- "you seem to be trying to use match for destructuring a \
- single pattern. Consider using `if let`",
- &format!("try\nif let {} = {} {}",
- snippet(cx, arms[0].pats[0].span, ".."),
- snippet(cx, ex.span, ".."),
- expr_block(cx, &arms[0].body, None, "..")));
- }
+ check_single_match(cx, ex, arms, expr);
+ check_match_bool(cx, ex, arms, expr);
+ check_overlapping_arms(cx, ex, arms);
+ }
+ if let ExprMatch(ref ex, ref arms, source) = expr.node {
+ check_match_ref_pats(cx, ex, arms, source, expr);
+ }
+ }
+}
- // check preconditions for MATCH_BOOL
- // type of expression == bool
- if cx.tcx.expr_ty(ex).sty == ty::TyBool {
- if arms.len() == 2 && arms[0].pats.len() == 1 { // no guards
- let exprs = if let PatLit(ref arm_bool) = arms[0].pats[0].node {
- if let ExprLit(ref lit) = arm_bool.node {
- match lit.node {
- LitBool(true) => Some((&*arms[0].body, &*arms[1].body)),
- LitBool(false) => Some((&*arms[1].body, &*arms[0].body)),
- _ => None,
- }
- } else { None }
- } else { None };
- if let Some((ref true_expr, ref false_expr)) = exprs {
- if !is_unit_expr(true_expr) {
- if !is_unit_expr(false_expr) {
- span_help_and_lint(cx, MATCH_BOOL, expr.span,
- "you seem to be trying to match on a boolean expression. \
- Consider using an if..else block:",
- &format!("try\nif {} {} else {}",
- snippet(cx, ex.span, "b"),
- expr_block(cx, true_expr, None, ".."),
- expr_block(cx, false_expr, None, "..")));
- } else {
- span_help_and_lint(cx, MATCH_BOOL, expr.span,
- "you seem to be trying to match on a boolean expression. \
- Consider using an if..else block:",
- &format!("try\nif {} {}",
- snippet(cx, ex.span, "b"),
- expr_block(cx, true_expr, None, "..")));
- }
- } else if !is_unit_expr(false_expr) {
- span_help_and_lint(cx, MATCH_BOOL, expr.span,
- "you seem to be trying to match on a boolean expression. \
- Consider using an if..else block:",
- &format!("try\nif !{} {}",
- snippet(cx, ex.span, "b"),
- expr_block(cx, false_expr, None, "..")));
- } else {
- span_lint(cx, MATCH_BOOL, expr.span,
- "you seem to be trying to match on a boolean expression. \
- Consider using an if..else block");
- }
+fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
+ if arms.len() == 2 &&
+ arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
+ arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
+ is_unit_expr(&arms[1].body) {
+ let ty = cx.tcx.expr_ty(ex);
+ if ty.sty != ty::TyBool || cx.current_level(MATCH_BOOL) == Allow {
+ check_single_match_single_pattern(cx, ex, arms, expr);
+ check_single_match_opt_like(cx, ex, arms, expr, ty);
+ }
+ }
+}
+
+fn check_single_match_single_pattern(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
+ if arms[1].pats[0].node == PatWild {
+ span_lint_and_then(cx,
+ SINGLE_MATCH,
+ expr.span,
+ "you seem to be trying to use match for destructuring a single pattern. \
+ Consider using `if let`", |db| {
+ db.span_suggestion(expr.span, "try this",
+ format!("if let {} = {} {}",
+ snippet(cx, arms[0].pats[0].span, ".."),
+ snippet(cx, ex.span, ".."),
+ expr_block(cx, &arms[0].body, None, "..")));
+ });
+ }
+}
+
+fn check_single_match_opt_like(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr, ty: ty::Ty) {
+ // list of candidate Enums we know will never get any more membre
+ let candidates = &[
+ (&COW_PATH, "Borrowed"),
+ (&COW_PATH, "Cow::Borrowed"),
+ (&COW_PATH, "Cow::Owned"),
+ (&COW_PATH, "Owned"),
+ (&OPTION_PATH, "None"),
+ (&RESULT_PATH, "Err"),
+ (&RESULT_PATH, "Ok"),
+ ];
+
+ let path = match arms[1].pats[0].node {
+ PatEnum(ref path, _) => path.to_string(),
+ PatIdent(BindByValue(MutImmutable), ident, None) => ident.node.to_string(),
+ _ => return
+ };
+
+ for &(ty_path, pat_path) in candidates {
+ if &path == pat_path && match_type(cx, ty, ty_path) {
+ span_lint_and_then(cx,
+ SINGLE_MATCH,
+ expr.span,
+ "you seem to be trying to use match for destructuring a single pattern. \
+ Consider using `if let`", |db| {
+ db.span_suggestion(expr.span, "try this",
+ format!("if let {} = {} {}",
+ snippet(cx, arms[0].pats[0].span, ".."),
+ snippet(cx, ex.span, ".."),
+ expr_block(cx, &arms[0].body, None, "..")));
+ });
+ }
+ }
+}
+
+fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
+ // type of expression == bool
+ if cx.tcx.expr_ty(ex).sty == ty::TyBool {
+ let sugg = if arms.len() == 2 && arms[0].pats.len() == 1 {
+ // no guards
+ let exprs = if let PatLit(ref arm_bool) = arms[0].pats[0].node {
+ if let ExprLit(ref lit) = arm_bool.node {
+ match lit.node {
+ LitBool(true) => Some((&*arms[0].body, &*arms[1].body)),
+ LitBool(false) => Some((&*arms[1].body, &*arms[0].body)),
+ _ => None,
+ }
+ } else {
+ None
+ }
+ } else {
+ None
+ };
+
+ if let Some((ref true_expr, ref false_expr)) = exprs {
+ if !is_unit_expr(true_expr) {
+ if !is_unit_expr(false_expr) {
+ Some(format!("if {} {} else {}",
+ snippet(cx, ex.span, "b"),
+ expr_block(cx, true_expr, None, ".."),
+ expr_block(cx, false_expr, None, "..")))
} else {
- span_lint(cx, MATCH_BOOL, expr.span,
- "you seem to be trying to match on a boolean expression. \
- Consider using an if..else block");
+ Some(format!("if {} {}",
+ snippet(cx, ex.span, "b"),
+ expr_block(cx, true_expr, None, "..")))
}
+ } else if !is_unit_expr(false_expr) {
+ Some(format!("try\nif !{} {}",
+ snippet(cx, ex.span, "b"),
+ expr_block(cx, false_expr, None, "..")))
} else {
- span_lint(cx, MATCH_BOOL, expr.span,
- "you seem to be trying to match on a boolean expression. \
- Consider using an if..else block");
+ None
}
+ } else {
+ None
}
+ } else {
+ None
+ };
+
+ span_lint_and_then(cx,
+ MATCH_BOOL,
+ expr.span,
+ "you seem to be trying to match on a boolean expression. Consider using \
+ an if..else block:", move |db| {
+ if let Some(ref sugg) = sugg {
+ db.span_suggestion(expr.span, "try this", sugg.clone());
+ }
+ });
+ }
+}
+
+fn check_overlapping_arms(cx: &LateContext, ex: &Expr, arms: &[Arm]) {
+ if arms.len() >= 2 && cx.tcx.expr_ty(ex).is_integral() {
+ let ranges = all_ranges(cx, arms);
+ let overlap = match type_ranges(&ranges) {
+ TypedRanges::IntRanges(ranges) => overlapping(&ranges).map(|(start, end)| (start.span, end.span)),
+ TypedRanges::UintRanges(ranges) => overlapping(&ranges).map(|(start, end)| (start.span, end.span)),
+ TypedRanges::None => None,
+ };
+
+ if let Some((start, end)) = overlap {
+ span_note_and_lint(cx,
+ MATCH_OVERLAPPING_ARM,
+ start,
+ "some ranges overlap",
+ end,
+ "overlaps with this");
}
- if let ExprMatch(ref ex, ref arms, source) = expr.node {
- // check preconditions for MATCH_REF_PATS
- if has_only_ref_pats(arms) {
- if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
- let template = match_template(cx, expr.span, source, "", inner);
- span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
- "you don't need to add `&` to both the expression \
- and the patterns: use `{}`", template));
- } else {
- let template = match_template(cx, expr.span, source, "*", ex);
- span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
- "instead of prefixing all patterns with `&`, you can dereference the \
- expression: `{}`", template));
- }
+ }
+}
+
+fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: MatchSource, expr: &Expr) {
+ if has_only_ref_pats(arms) {
+ if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
+ let template = match_template(cx, expr.span, source, "", inner);
+ span_lint(cx,
+ MATCH_REF_PATS,
+ expr.span,
+ &format!("you don't need to add `&` to both the expression and the patterns: use `{}`",
+ template));
+ } else {
+ let template = match_template(cx, expr.span, source, "*", ex);
+ span_lint(cx,
+ MATCH_REF_PATS,
+ expr.span,
+ &format!("instead of prefixing all patterns with `&`, you can dereference the expression: `{}`",
+ template));
+ }
+ }
+}
+
+/// Get all arms that are unbounded PatRange-s.
+fn all_ranges(cx: &LateContext, arms: &[Arm]) -> Vec<SpannedRange<ConstVal>> {
+ arms.iter()
+ .filter_map(|arm| {
+ if let Arm { ref pats, guard: None, .. } = *arm {
+ Some(pats.iter().filter_map(|pat| {
+ if_let_chain! {[
+ let PatRange(ref lhs, ref rhs) = pat.node,
+ let Ok(lhs) = eval_const_expr_partial(cx.tcx, &lhs, ExprTypeChecked, None),
+ let Ok(rhs) = eval_const_expr_partial(cx.tcx, &rhs, ExprTypeChecked, None)
+ ], {
+ return Some(SpannedRange { span: pat.span, node: (lhs, rhs) });
+ }}
+
+ if_let_chain! {[
+ let PatLit(ref value) = pat.node,
+ let Ok(value) = eval_const_expr_partial(cx.tcx, &value, ExprTypeChecked, None)
+ ], {
+ return Some(SpannedRange { span: pat.span, node: (value.clone(), value) });
+ }}
+
+ None
+ }))
+ } else {
+ None
}
+ })
+ .flat_map(IntoIterator::into_iter)
+ .collect()
+}
+
+#[derive(Debug, Eq, PartialEq)]
+pub struct SpannedRange<T> {
+ pub span: Span,
+ pub node: (T, T),
+}
+
+#[derive(Debug)]
+enum TypedRanges {
+ IntRanges(Vec<SpannedRange<i64>>),
+ UintRanges(Vec<SpannedRange<u64>>),
+ None,
+}
+
+/// Get all `Int` ranges or all `Uint` ranges. Mixed types are an error anyway and other types than
+/// `Uint` and `Int` probably don't make sense.
+fn type_ranges(ranges: &[SpannedRange<ConstVal>]) -> TypedRanges {
+ if ranges.is_empty() {
+ TypedRanges::None
+ } else {
+ match ranges[0].node {
+ (Int(_), Int(_)) => {
+ TypedRanges::IntRanges(ranges.iter()
+ .filter_map(|range| {
+ if let (Int(start), Int(end)) = range.node {
+ Some(SpannedRange {
+ span: range.span,
+ node: (start, end),
+ })
+ } else {
+ None
+ }
+ })
+ .collect())
+ }
+ (Uint(_), Uint(_)) => {
+ TypedRanges::UintRanges(ranges.iter()
+ .filter_map(|range| {
+ if let (Uint(start), Uint(end)) = range.node {
+ Some(SpannedRange {
+ span: range.span,
+ node: (start, end),
+ })
+ } else {
+ None
+ }
+ })
+ .collect())
+ }
+ _ => TypedRanges::None,
}
}
}
}
fn has_only_ref_pats(arms: &[Arm]) -> bool {
- let mapped = arms.iter().flat_map(|a| &a.pats).map(|p| match p.node {
- PatRegion(..) => Some(true), // &-patterns
- PatWild => Some(false), // an "anything" wildcard is also fine
- _ => None, // any other pattern is not fine
- }).collect::<Option<Vec<bool>>>();
+ let mapped = arms.iter()
+ .flat_map(|a| &a.pats)
+ .map(|p| {
+ match p.node {
+ PatRegion(..) => Some(true), // &-patterns
+ PatWild => Some(false), // an "anything" wildcard is also fine
+ _ => None, // any other pattern is not fine
+ }
+ })
+ .collect::<Option<Vec<bool>>>();
// look for Some(v) where there's at least one true element
mapped.map_or(false, |v| v.iter().any(|el| *el))
}
-fn match_template(cx: &LateContext,
- span: Span,
- source: MatchSource,
- op: &str,
- expr: &Expr) -> String {
+fn match_template(cx: &LateContext, span: Span, source: MatchSource, op: &str, expr: &Expr) -> String {
let expr_snippet = snippet(cx, expr.span, "..");
match source {
- MatchSource::Normal => {
- format!("match {}{} {{ ...", op, expr_snippet)
+ MatchSource::Normal => format!("match {}{} {{ ...", op, expr_snippet),
+ MatchSource::IfLetDesugar { .. } => format!("if let ... = {}{} {{", op, expr_snippet),
+ MatchSource::WhileLetDesugar => format!("while let ... = {}{} {{", op, expr_snippet),
+ MatchSource::ForLoopDesugar => cx.sess().span_bug(span, "for loop desugared to match with &-patterns!"),
+ }
+}
+
+pub fn overlapping<T>(ranges: &[SpannedRange<T>]) -> Option<(&SpannedRange<T>, &SpannedRange<T>)>
+ where T: Copy + Ord
+{
+ #[derive(Copy, Clone, Debug, Eq, PartialEq)]
+ enum Kind<'a, T: 'a> {
+ Start(T, &'a SpannedRange<T>),
+ End(T, &'a SpannedRange<T>),
+ }
+
+ impl<'a, T: Copy> Kind<'a, T> {
+ fn range(&self) -> &'a SpannedRange<T> {
+ match *self {
+ Kind::Start(_, r) | Kind::End(_, r) => r,
+ }
}
- MatchSource::IfLetDesugar { .. } => {
- format!("if let ... = {}{} {{", op, expr_snippet)
+
+ fn value(self) -> T {
+ match self {
+ Kind::Start(t, _) | Kind::End(t, _) => t,
+ }
}
- MatchSource::WhileLetDesugar => {
- format!("while let ... = {}{} {{", op, expr_snippet)
+ }
+
+ impl<'a, T: Copy + Ord> PartialOrd for Kind<'a, T> {
+ fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+ Some(self.cmp(other))
}
- MatchSource::ForLoopDesugar => {
- cx.sess().span_bug(span, "for loop desugared to match with &-patterns!")
+ }
+
+ impl<'a, T: Copy + Ord> Ord for Kind<'a, T> {
+ fn cmp(&self, other: &Self) -> Ordering {
+ self.value().cmp(&other.value())
}
}
+
+ let mut values = Vec::with_capacity(2 * ranges.len());
+
+ for r in ranges {
+ values.push(Kind::Start(r.node.0, &r));
+ values.push(Kind::End(r.node.1, &r));
+ }
+
+ values.sort();
+
+ for (a, b) in values.iter().zip(values.iter().skip(1)) {
+ match (a, b) {
+ (&Kind::Start(_, ra), &Kind::End(_, rb)) => {
+ if ra.node != rb.node {
+ return Some((ra, rb));
+ }
+ }
+ (&Kind::End(a, _), &Kind::Start(b, _)) if a != b => (),
+ _ => return Some((&a.range(), &b.range())),
+ }
+ }
+
+ None
}
use std::iter;
use std::borrow::Cow;
-use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args,
+use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, match_trait_method,
walk_ptrs_ty_depth, walk_ptrs_ty};
use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
use utils::MethodArgs;
/// **Example:** `x.map(|a| a + 1).unwrap_or(0)`
declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn,
"using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as \
- `map_or(a, f)`)");
+ `map_or(a, f)`");
/// **What it does:** This lint `Warn`s on `_.map(_).unwrap_or_else(_)`.
///
/// **Example:** `x.map(|a| a + 1).unwrap_or_else(some_function)`
declare_lint!(pub 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)`)");
+ `map_or_else(g, f)`");
+
+/// **What it does:** This lint `Warn`s on `_.filter(_).next()`.
+///
+/// **Why is this bad?** Readability, this can be written more concisely as `_.find(_)`.
+///
+/// **Known problems:** None.
+///
+/// **Example:** `iter.filter(|x| x == 0).next()`
+declare_lint!(pub FILTER_NEXT, Warn,
+ "using `filter(p).next()`, which is more succinctly expressed as `.find(p)`");
+
+/// **What it does:** This lint `Warn`s on an iterator search (such as `find()`, `position()`, or
+/// `rposition()`) followed by a call to `is_some()`.
+///
+/// **Why is this bad?** Readability, this can be written more concisely as `_.any(_)`.
+///
+/// **Known problems:** None.
+///
+/// **Example:** `iter.find(|x| x == 0).is_some()`
+declare_lint!(pub SEARCH_IS_SOME, Warn,
+ "using an iterator search followed by `is_some()`, which is more succinctly \
+ expressed as a call to `any()`");
impl LintPass for MethodsPass {
fn get_lints(&self) -> LintArray {
- lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING,
- SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, OK_EXPECT, OPTION_MAP_UNWRAP_OR,
+ lint_array!(OPTION_UNWRAP_USED,
+ RESULT_UNWRAP_USED,
+ STR_TO_STRING,
+ STRING_TO_STRING,
+ SHOULD_IMPLEMENT_TRAIT,
+ WRONG_SELF_CONVENTION,
+ WRONG_PUB_SELF_CONVENTION,
+ OK_EXPECT,
+ OPTION_MAP_UNWRAP_OR,
OPTION_MAP_UNWRAP_OR_ELSE)
}
}
if let ExprMethodCall(_, _, _) = expr.node {
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
lint_unwrap(cx, expr, arglists[0]);
- }
- else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
+ } else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
lint_to_string(cx, expr, arglists[0]);
- }
- else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
+ } else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
lint_ok_expect(cx, expr, arglists[0]);
- }
- else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
+ } else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
- }
- else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
+ } else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
+ } else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
+ lint_filter_next(cx, expr, arglists[0]);
+ } else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
+ lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
+ } else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
+ lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
+ } else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
+ lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
}
}
}
let is_copy = is_copy(cx, &ty, &item);
for &(prefix, self_kinds) in &CONVENTIONS {
if name.as_str().starts_with(prefix) &&
- !self_kinds.iter().any(|k| k.matches(&sig.explicit_self.node, is_copy)) {
+ !self_kinds.iter().any(|k| k.matches(&sig.explicit_self.node, is_copy)) {
let lint = if item.vis == Visibility::Public {
WRONG_PUB_SELF_CONVENTION
} else {
WRONG_SELF_CONVENTION
};
- span_lint(cx, lint, sig.explicit_self.span, &format!(
- "methods called `{}*` usually take {}; consider choosing a less \
- ambiguous name", prefix,
- &self_kinds.iter().map(|k| k.description()).collect::<Vec<_>>().join(" or ")));
+ span_lint(cx,
+ lint,
+ sig.explicit_self.span,
+ &format!("methods called `{}*` usually take {}; consider choosing a less \
+ ambiguous name",
+ prefix,
+ &self_kinds.iter()
+ .map(|k| k.description())
+ .collect::<Vec<_>>()
+ .join(" or ")));
}
}
}
}
}
-#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
+#[allow(ptr_arg)]
+// Type of MethodArgs is potentially a Vec
/// lint use of `unwrap()` for `Option`s and `Result`s
fn lint_unwrap(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs) {
let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&unwrap_args[0]));
- if match_type(cx, obj_ty, &OPTION_PATH) {
- span_lint(cx, OPTION_UNWRAP_USED, expr.span,
- "used unwrap() on an Option value. If you don't want to handle the None case \
- gracefully, consider using expect() to provide a better panic message");
- }
- else if match_type(cx, obj_ty, &RESULT_PATH) {
- span_lint(cx, RESULT_UNWRAP_USED, expr.span,
- "used unwrap() on a Result value. Graceful handling of Err values is preferred");
+ let mess = if match_type(cx, obj_ty, &OPTION_PATH) {
+ Some((OPTION_UNWRAP_USED, "an Option", "None"))
+ } else if match_type(cx, obj_ty, &RESULT_PATH) {
+ Some((RESULT_UNWRAP_USED, "a Result", "Err"))
+ } else {
+ None
+ };
+
+ if let Some((lint, kind, none_value)) = mess {
+ span_lint(cx,
+ lint,
+ expr.span,
+ &format!("used unwrap() on {} value. If you don't want to handle the {} case gracefully, consider \
+ using expect() to provide a better panic
+ message",
+ kind,
+ none_value));
}
}
-#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
+#[allow(ptr_arg)]
+// Type of MethodArgs is potentially a Vec
/// lint use of `to_string()` for `&str`s and `String`s
fn lint_to_string(cx: &LateContext, expr: &Expr, to_string_args: &MethodArgs) {
let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&to_string_args[0]));
if obj_ty.sty == ty::TyStr {
let mut arg_str = snippet(cx, to_string_args[0].span, "_");
if ptr_depth > 1 {
- arg_str = Cow::Owned(format!(
- "({}{})",
- iter::repeat('*').take(ptr_depth - 1).collect::<String>(),
- arg_str));
+ arg_str = Cow::Owned(format!("({}{})", iter::repeat('*').take(ptr_depth - 1).collect::<String>(), arg_str));
}
- span_lint(cx, STR_TO_STRING, expr.span,
- &format!("`{}.to_owned()` is faster", arg_str));
- }
- else if match_type(cx, obj_ty, &STRING_PATH) {
- span_lint(cx, STRING_TO_STRING, expr.span,
+ span_lint(cx, STR_TO_STRING, expr.span, &format!("`{}.to_owned()` is faster", arg_str));
+ } else if match_type(cx, obj_ty, &STRING_PATH) {
+ span_lint(cx,
+ STRING_TO_STRING,
+ expr.span,
"`String.to_string()` is a no-op; use `clone()` to make a copy");
}
}
-#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
+#[allow(ptr_arg)]
+// Type of MethodArgs is potentially a Vec
/// lint use of `ok().expect()` for `Result`s
fn lint_ok_expect(cx: &LateContext, expr: &Expr, ok_args: &MethodArgs) {
// lint if the caller of `ok()` is a `Result`
let result_type = cx.tcx.expr_ty(&ok_args[0]);
if let Some(error_type) = get_error_type(cx, result_type) {
if has_debug_impl(error_type, cx) {
- span_lint(cx, OK_EXPECT, expr.span,
- "called `ok().expect()` on a Result value. You can call `expect` \
- directly on the `Result`");
+ span_lint(cx,
+ OK_EXPECT,
+ expr.span,
+ "called `ok().expect()` on a Result value. You can call `expect` directly on the `Result`");
}
}
}
}
-#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
+#[allow(ptr_arg)]
+// Type of MethodArgs is potentially a Vec
/// lint use of `map().unwrap_or()` for `Option`s
-fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs,
- map_args: &MethodArgs) {
+fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr, map_args: &MethodArgs, unwrap_args: &MethodArgs) {
// lint if the caller of `map()` is an `Option`
if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) {
// lint message
- let msg = "called `map(f).unwrap_or(a)` on an Option value. This can be done more \
- directly by calling `map_or(a, f)` instead";
+ let msg = "called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling \
+ `map_or(a, f)` instead";
// get snippets for args to map() and unwrap_or()
let map_snippet = snippet(cx, map_args[1].span, "..");
let unwrap_snippet = snippet(cx, unwrap_args[1].span, "..");
// lint, with note if neither arg is > 1 line and both map() and
// unwrap_or() have the same span
- let multiline = map_snippet.lines().count() > 1
- || unwrap_snippet.lines().count() > 1;
+ let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1;
let same_span = map_args[1].span.expn_id == unwrap_args[1].span.expn_id;
if same_span && !multiline {
- span_note_and_lint(
- cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span,
- &format!("replace this with map_or({1}, {0})", map_snippet, unwrap_snippet)
- );
- }
- else if same_span && multiline {
+ span_note_and_lint(cx,
+ OPTION_MAP_UNWRAP_OR,
+ expr.span,
+ msg,
+ expr.span,
+ &format!("replace `map({0}).unwrap_or({1})` with `map_or({1}, {0})`",
+ map_snippet,
+ unwrap_snippet));
+ } else if same_span && multiline {
span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg);
};
}
}
-#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
+#[allow(ptr_arg)]
+// Type of MethodArgs is potentially a Vec
/// lint use of `map().unwrap_or_else()` for `Option`s
-fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs,
- map_args: &MethodArgs) {
+fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, map_args: &MethodArgs, unwrap_args: &MethodArgs) {
// lint if the caller of `map()` is an `Option`
if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) {
// lint message
- let msg = "called `map(f).unwrap_or_else(g)` on an Option value. This can be done more \
- directly by calling `map_or_else(g, f)` instead";
+ let msg = "called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling \
+ `map_or_else(g, f)` instead";
// get snippets for args to map() and unwrap_or_else()
let map_snippet = snippet(cx, map_args[1].span, "..");
let unwrap_snippet = snippet(cx, unwrap_args[1].span, "..");
// lint, with note if neither arg is > 1 line and both map() and
// unwrap_or_else() have the same span
- let multiline = map_snippet.lines().count() > 1
- || unwrap_snippet.lines().count() > 1;
+ let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1;
let same_span = map_args[1].span.expn_id == unwrap_args[1].span.expn_id;
if same_span && !multiline {
- span_note_and_lint(
- cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg, expr.span,
- &format!("replace this with map_or_else({1}, {0})", map_snippet, unwrap_snippet)
- );
- }
- else if same_span && multiline {
+ span_note_and_lint(cx,
+ OPTION_MAP_UNWRAP_OR_ELSE,
+ expr.span,
+ msg,
+ expr.span,
+ &format!("replace `map({0}).unwrap_or_else({1})` with `with map_or_else({1}, {0})`",
+ map_snippet,
+ unwrap_snippet));
+ } else if same_span && multiline {
span_lint(cx, OPTION_MAP_UNWRAP_OR_ELSE, expr.span, msg);
};
}
}
+#[allow(ptr_arg)]
+// Type of MethodArgs is potentially a Vec
+/// lint use of `filter().next() for Iterators`
+fn lint_filter_next(cx: &LateContext, expr: &Expr, filter_args: &MethodArgs) {
+ // lint if caller of `.filter().next()` is an Iterator
+ if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) {
+ let msg = "called `filter(p).next()` on an Iterator. This is more succinctly expressed by calling `.find(p)` \
+ instead.";
+ let filter_snippet = snippet(cx, filter_args[1].span, "..");
+ if filter_snippet.lines().count() <= 1 {
+ // add note if not multi-line
+ span_note_and_lint(cx,
+ FILTER_NEXT,
+ expr.span,
+ msg,
+ expr.span,
+ &format!("replace `filter({0}).next()` with `find({0})`", filter_snippet));
+ } else {
+ span_lint(cx, FILTER_NEXT, expr.span, msg);
+ }
+ }
+}
+
+#[allow(ptr_arg)]
+// Type of MethodArgs is potentially a Vec
+/// lint searching an Iterator followed by `is_some()`
+fn lint_search_is_some(cx: &LateContext, expr: &Expr, search_method: &str, search_args: &MethodArgs,
+ is_some_args: &MethodArgs) {
+ // lint if caller of search is an Iterator
+ if match_trait_method(cx, &*is_some_args[0], &["core", "iter", "Iterator"]) {
+ let msg = format!("called `is_some()` after searching an iterator with {}. This is more succinctly expressed \
+ by calling `any()`.",
+ search_method);
+ let search_snippet = snippet(cx, search_args[1].span, "..");
+ if search_snippet.lines().count() <= 1 {
+ // add note if not multi-line
+ span_note_and_lint(cx,
+ SEARCH_IS_SOME,
+ expr.span,
+ &msg,
+ expr.span,
+ &format!("replace `{0}({1}).is_some()` with `any({1})`", search_method, search_snippet));
+ } else {
+ span_lint(cx, SEARCH_IS_SOME, expr.span, &msg);
+ }
+ }
+}
+
// Given a `Result<T, E>` type, return its error type (`E`)
fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
if !match_type(cx, ty, &RESULT_PATH) {
let no_ref_ty = walk_ptrs_ty(ty);
let debug = match cx.tcx.lang_items.debug_trait() {
Some(debug) => debug,
- None => return false
+ None => return false,
};
let debug_def = cx.tcx.lookup_trait_def(debug);
let mut debug_impl_exists = false;
debug_impl_exists
}
-const CONVENTIONS: [(&'static str, &'static [SelfKind]); 5] = [
- ("into_", &[ValueSelf]),
- ("to_", &[RefSelf]),
- ("as_", &[RefSelf, RefMutSelf]),
- ("is_", &[RefSelf, NoSelf]),
- ("from_", &[NoSelf]),
-];
-
-const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [
- ("add", 2, ValueSelf, AnyType, "std::ops::Add"),
- ("sub", 2, ValueSelf, AnyType, "std::ops::Sub"),
- ("mul", 2, ValueSelf, AnyType, "std::ops::Mul"),
- ("div", 2, ValueSelf, AnyType, "std::ops::Div"),
- ("rem", 2, ValueSelf, AnyType, "std::ops::Rem"),
- ("shl", 2, ValueSelf, AnyType, "std::ops::Shl"),
- ("shr", 2, ValueSelf, AnyType, "std::ops::Shr"),
- ("bitand", 2, ValueSelf, AnyType, "std::ops::BitAnd"),
- ("bitor", 2, ValueSelf, AnyType, "std::ops::BitOr"),
- ("bitxor", 2, ValueSelf, AnyType, "std::ops::BitXor"),
- ("neg", 1, ValueSelf, AnyType, "std::ops::Neg"),
- ("not", 1, ValueSelf, AnyType, "std::ops::Not"),
- ("drop", 1, RefMutSelf, UnitType, "std::ops::Drop"),
- ("index", 2, RefSelf, RefType, "std::ops::Index"),
- ("index_mut", 2, RefMutSelf, RefType, "std::ops::IndexMut"),
- ("deref", 1, RefSelf, RefType, "std::ops::Deref"),
- ("deref_mut", 1, RefMutSelf, RefType, "std::ops::DerefMut"),
- ("clone", 1, RefSelf, AnyType, "std::clone::Clone"),
- ("borrow", 1, RefSelf, RefType, "std::borrow::Borrow"),
- ("borrow_mut", 1, RefMutSelf, RefType, "std::borrow::BorrowMut"),
- ("as_ref", 1, RefSelf, RefType, "std::convert::AsRef"),
- ("as_mut", 1, RefMutSelf, RefType, "std::convert::AsMut"),
- ("eq", 2, RefSelf, BoolType, "std::cmp::PartialEq"),
- ("cmp", 2, RefSelf, AnyType, "std::cmp::Ord"),
- ("default", 0, NoSelf, AnyType, "std::default::Default"),
- ("hash", 2, RefSelf, UnitType, "std::hash::Hash"),
- ("next", 1, RefMutSelf, AnyType, "std::iter::Iterator"),
- ("into_iter", 1, ValueSelf, AnyType, "std::iter::IntoIterator"),
- ("from_iter", 1, NoSelf, AnyType, "std::iter::FromIterator"),
- ("from_str", 1, NoSelf, AnyType, "std::str::FromStr"),
-];
+const CONVENTIONS: [(&'static str, &'static [SelfKind]); 5] = [("into_", &[ValueSelf]),
+ ("to_", &[RefSelf]),
+ ("as_", &[RefSelf, RefMutSelf]),
+ ("is_", &[RefSelf, NoSelf]),
+ ("from_", &[NoSelf])];
+
+const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [("add",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::Add"),
+ ("sub",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::Sub"),
+ ("mul",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::Mul"),
+ ("div",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::Div"),
+ ("rem",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::Rem"),
+ ("shl",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::Shl"),
+ ("shr",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::Shr"),
+ ("bitand",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::BitAnd"),
+ ("bitor",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::BitOr"),
+ ("bitxor",
+ 2,
+ ValueSelf,
+ AnyType,
+ "std::ops::BitXor"),
+ ("neg",
+ 1,
+ ValueSelf,
+ AnyType,
+ "std::ops::Neg"),
+ ("not",
+ 1,
+ ValueSelf,
+ AnyType,
+ "std::ops::Not"),
+ ("drop",
+ 1,
+ RefMutSelf,
+ UnitType,
+ "std::ops::Drop"),
+ ("index",
+ 2,
+ RefSelf,
+ RefType,
+ "std::ops::Index"),
+ ("index_mut",
+ 2,
+ RefMutSelf,
+ RefType,
+ "std::ops::IndexMut"),
+ ("deref",
+ 1,
+ RefSelf,
+ RefType,
+ "std::ops::Deref"),
+ ("deref_mut",
+ 1,
+ RefMutSelf,
+ RefType,
+ "std::ops::DerefMut"),
+ ("clone",
+ 1,
+ RefSelf,
+ AnyType,
+ "std::clone::Clone"),
+ ("borrow",
+ 1,
+ RefSelf,
+ RefType,
+ "std::borrow::Borrow"),
+ ("borrow_mut",
+ 1,
+ RefMutSelf,
+ RefType,
+ "std::borrow::BorrowMut"),
+ ("as_ref",
+ 1,
+ RefSelf,
+ RefType,
+ "std::convert::AsRef"),
+ ("as_mut",
+ 1,
+ RefMutSelf,
+ RefType,
+ "std::convert::AsMut"),
+ ("eq",
+ 2,
+ RefSelf,
+ BoolType,
+ "std::cmp::PartialEq"),
+ ("cmp",
+ 2,
+ RefSelf,
+ AnyType,
+ "std::cmp::Ord"),
+ ("default",
+ 0,
+ NoSelf,
+ AnyType,
+ "std::default::Default"),
+ ("hash",
+ 2,
+ RefSelf,
+ UnitType,
+ "std::hash::Hash"),
+ ("next",
+ 1,
+ RefMutSelf,
+ AnyType,
+ "std::iter::Iterator"),
+ ("into_iter",
+ 1,
+ ValueSelf,
+ AnyType,
+ "std::iter::IntoIterator"),
+ ("from_iter",
+ 1,
+ NoSelf,
+ AnyType,
+ "std::iter::FromIterator"),
+ ("from_str",
+ 1,
+ NoSelf,
+ AnyType,
+ "std::str::FromStr")];
#[derive(Clone, Copy)]
enum SelfKind {
(&RefMutSelf, &SelfValue(_)) => allow_value_for_ref,
(&NoSelf, &SelfStatic) => true,
(_, &SelfExplicit(ref ty, _)) => self.matches_explicit_type(ty, allow_value_for_ref),
- _ => false
+ _ => false,
}
}
(&RefMutSelf, &TyRptr(_, MutTy { mutbl: Mutability::MutMutable, .. })) => true,
(&RefSelf, &TyPath(..)) => allow_value_for_ref,
(&RefMutSelf, &TyPath(..)) => allow_value_for_ref,
- _ => false
+ _ => false,
}
}
(&UnitType, &DefaultReturn(_)) => true,
(&UnitType, &Return(ref ty)) if ty.node == TyTup(vec![].into()) => true,
(&BoolType, &Return(ref ty)) if is_bool(ty) => true,
- (&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![].into()) => true,
+ (&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![].into()) => true,
(&RefType, &Return(ref ty)) => {
- if let TyRptr(_, _) = ty.node { true } else { false }
+ if let TyRptr(_, _) = ty.node {
+ true
+ } else {
+ false
+ }
}
- _ => false
+ _ => false,
}
}
}
use utils::{match_def_path, span_lint};
use self::MinMax::{Min, Max};
-/// **What it does:** This lint checks for expressions where `std::cmp::min` and `max` are used to clamp values, but switched so that the result is constant.
+/// **What it does:** This lint checks for expressions where `std::cmp::min` and `max` are used to clamp values, but switched so that the result is constant. It is `Warn` by default.
///
/// **Why is this bad?** This is in all probability not the intended outcome. At the least it hurts readability of the code.
///
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let Some((outer_max, outer_c, oe)) = min_max(cx, expr) {
if let Some((inner_max, inner_c, _)) = min_max(cx, oe) {
- if outer_max == inner_max { return; }
+ if outer_max == inner_max {
+ return;
+ }
match (outer_max, outer_c.partial_cmp(&inner_c)) {
(_, None) | (Max, Some(Less)) | (Min, Some(Greater)) => (),
_ => {
- span_lint(cx, MIN_MAX, expr.span,
- "this min/max combination leads to constant result")
+ span_lint(cx, MIN_MAX, expr.span, "this min/max combination leads to constant result");
}
}
}
None
}
}
- } else { None }
- } else { None }
- }
+ } else {
+ None
+ }
+ } else {
+ None
+ }
+}
-fn fetch_const(args: &[P<Expr>], m: MinMax) ->
- Option<(MinMax, Constant, &Expr)> {
- if args.len() != 2 { return None }
+fn fetch_const(args: &[P<Expr>], m: MinMax) -> Option<(MinMax, Constant, &Expr)> {
+ if args.len() != 2 {
+ return None;
+ }
if let Some(c) = constant_simple(&args[0]) {
- if let None = constant_simple(&args[1]) { // otherwise ignore
+ if let None = constant_simple(&args[1]) {
+ // otherwise ignore
Some((m, c, &args[1]))
- } else { None }
+ } else {
+ None
+ }
} else {
if let Some(c) = constant_simple(&args[1]) {
Some((m, c, &args[0]))
- } else { None }
+ } else {
+ None
+ }
}
}
fn check_fn(&mut self, cx: &LateContext, k: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) {
if let FnKind::Closure = k {
// Does not apply to closures
- return
+ return;
}
for ref arg in &decl.inputs {
if let PatIdent(BindByRef(_), _, _) = arg.pat.node {
span_lint(cx,
- TOPLEVEL_REF_ARG,
- arg.pat.span,
- "`ref` directly on a function argument is ignored. Consider using a reference type instead."
- );
+ TOPLEVEL_REF_ARG,
+ arg.pat.span,
+ "`ref` directly on a function argument is ignored. Consider using a reference type instead.");
}
}
}
}
fn check_nan(cx: &LateContext, path: &Path, span: Span) {
- path.segments.last().map(|seg| if seg.identifier.name.as_str() == "NAN" {
- span_lint(cx, CMP_NAN, span,
- "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
+ path.segments.last().map(|seg| {
+ if seg.identifier.name.as_str() == "NAN" {
+ span_lint(cx,
+ CMP_NAN,
+ span,
+ "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
+ }
});
}
if let ExprBinary(ref cmp, ref left, ref right) = expr.node {
let op = cmp.node;
if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) {
- if is_allowed(cx, left) || is_allowed(cx, right) { return; }
+ if is_allowed(cx, left) || is_allowed(cx, right) {
+ return;
+ }
if let Some(name) = get_item_name(cx, expr) {
let name = name.as_str();
- if name == "eq" || name == "ne" || name == "is_nan" ||
- name.starts_with("eq_") ||
- name.ends_with("_eq") {
+ if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") ||
+ name.ends_with("_eq") {
return;
}
}
- span_lint(cx, FLOAT_CMP, expr.span, &format!(
- "{}-comparison of f32 or f64 detected. Consider changing this to \
- `abs({} - {}) < epsilon` for some suitable value of epsilon",
- binop_to_string(op), snippet(cx, left.span, ".."),
- snippet(cx, right.span, "..")));
+ span_lint(cx,
+ FLOAT_CMP,
+ expr.span,
+ &format!("{}-comparison of f32 or f64 detected. Consider changing this to `abs({} - {}) < \
+ epsilon` for some suitable value of epsilon",
+ binop_to_string(op),
+ snippet(cx, left.span, ".."),
+ snippet(cx, right.span, "..")));
}
}
}
let res = eval_const_expr_partial(cx.tcx, expr, ExprTypeChecked, None);
if let Ok(Float(val)) = res {
val == 0.0 || val == ::std::f64::INFINITY || val == ::std::f64::NEG_INFINITY
- } else { false }
+ } else {
+ false
+ }
}
fn is_float(cx: &LateContext, expr: &Expr) -> bool {
fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, op: Span) {
let snip = match expr.node {
ExprMethodCall(Spanned{node: ref name, ..}, _, ref args) if args.len() == 1 => {
- if name.as_str() == "to_string" ||
- name.as_str() == "to_owned" && is_str_arg(cx, args) {
- snippet(cx, args[0].span, "..")
- } else {
- return
- }
+ if name.as_str() == "to_string" || name.as_str() == "to_owned" && is_str_arg(cx, args) {
+ snippet(cx, args[0].span, "..")
+ } else {
+ return;
+ }
}
ExprCall(ref path, ref v) if v.len() == 1 => {
if let ExprPath(None, ref path) = path.node {
- if match_path(path, &["String", "from_str"]) ||
- match_path(path, &["String", "from"]) {
- snippet(cx, v[0].span, "..")
- } else {
- return
- }
+ if match_path(path, &["String", "from_str"]) || match_path(path, &["String", "from"]) {
+ snippet(cx, v[0].span, "..")
+ } else {
+ return;
+ }
} else {
- return
+ return;
}
}
- _ => return
+ _ => return,
};
if left {
- span_lint(cx, CMP_OWNED, expr.span, &format!(
- "this creates an owned instance just for comparison. Consider using \
- `{} {} {}` to compare without allocation", snip,
- snippet(cx, op, "=="), snippet(cx, other_span, "..")));
+ span_lint(cx,
+ CMP_OWNED,
+ expr.span,
+ &format!("this creates an owned instance just for comparison. Consider using `{} {} {}` to \
+ compare without allocation",
+ snip,
+ snippet(cx, op, "=="),
+ snippet(cx, other_span, "..")));
} else {
- span_lint(cx, CMP_OWNED, expr.span, &format!(
- "this creates an owned instance just for comparison. Consider using \
- `{} {} {}` to compare without allocation",
- snippet(cx, other_span, ".."), snippet(cx, op, "=="), snip));
+ span_lint(cx,
+ CMP_OWNED,
+ expr.span,
+ &format!("this creates an owned instance just for comparison. Consider using `{} {} {}` to \
+ compare without allocation",
+ snippet(cx, other_span, ".."),
+ snippet(cx, op, "=="),
+ snip));
}
}
fn is_str_arg(cx: &LateContext, args: &[P<Expr>]) -> bool {
- args.len() == 1 && if let ty::TyStr =
- walk_ptrs_ty(cx.tcx.expr_ty(&args[0])).sty { true } else { false }
+ args.len() == 1 &&
+ if let ty::TyStr = walk_ptrs_ty(cx.tcx.expr_ty(&args[0])).sty {
+ true
+ } else {
+ false
+ }
}
/// **What it does:** This lint checks for getting the remainder of a division by one. It is `Warn` by default.
}
}
-/// **What it does:** This lint checks for patterns in the form `name @ _`.
+/// **What it does:** This lint checks for patterns in the form `name @ _`. It is `Warn` by default.
///
/// **Why is this bad?** It's almost always more readable to just use direct bindings.
///
fn check_pat(&mut self, cx: &LateContext, pat: &Pat) {
if let PatIdent(_, ref ident, Some(ref right)) = pat.node {
if right.node == PatWild {
- cx.span_lint(REDUNDANT_PATTERN, pat.span, &format!(
- "the `{} @ _` pattern can be written as just `{}`",
- ident.node.name, ident.node.name));
+ cx.span_lint(REDUNDANT_PATTERN,
+ pat.span,
+ &format!("the `{} @ _` pattern can be written as just `{}`",
+ ident.node.name,
+ ident.node.name));
}
}
}
}
impl LateLintPass for UsedUnderscoreBinding {
+ #[allow(unused_attributes)]
+ #[rustfmt_skip]
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
- if in_attributes_expansion(cx, expr) { // Don't lint things expanded by #[derive(...)], etc
+ if in_attributes_expansion(cx, expr) {
+ // Don't lint things expanded by #[derive(...)], etc
return;
}
let needs_lint = match expr.node {
ExprPath(_, ref path) => {
- let ident = path.segments.last()
+ let ident = path.segments
+ .last()
.expect("path should always have at least one segment")
.identifier;
- ident.name.as_str().chars().next() == Some('_') //starts with '_'
- && ident.name.as_str().chars().skip(1).next() != Some('_') //doesn't start with "__"
- && ident.name != ident.unhygienic_name //not in bang macro
- && is_used(cx, expr)
- },
+ ident.name.as_str().chars().next() == Some('_') && // starts with '_'
+ ident.name.as_str().chars().skip(1).next() != Some('_') && // doesn't start with "__"
+ ident.name != ident.unhygienic_name && is_used(cx, expr) // not in bang macro
+ }
ExprField(_, spanned) => {
let name = spanned.node.as_str();
- name.chars().next() == Some('_')
- && name.chars().skip(1).next() != Some('_')
- },
- _ => false
+ name.chars().next() == Some('_') && name.chars().skip(1).next() != Some('_')
+ }
+ _ => false,
};
if needs_lint {
- cx.span_lint(USED_UNDERSCORE_BINDING, expr.span,
- "used binding which is prefixed with an underscore. A leading underscore \
- signals that a binding will not be used.");
+ cx.span_lint(USED_UNDERSCORE_BINDING,
+ expr.span,
+ "used binding which is prefixed with an underscore. A leading underscore signals that a \
+ binding will not be used.");
}
}
}
match parent.node {
ExprAssign(_, ref rhs) => **rhs == *expr,
ExprAssignOp(_, _, ref rhs) => **rhs == *expr,
- _ => is_used(cx, &parent)
+ _ => is_used(cx, &parent),
}
- }
- else {
+ } else {
true
}
}
-//use rustc_front::hir::*;
-
use rustc::lint::*;
+use std::collections::HashMap;
+
use syntax::ast::*;
+use syntax::codemap::Span;
+use syntax::visit::FnKind;
-use utils::span_lint;
+use utils::{span_lint, span_help_and_lint};
/// **What it does:** This lint `Warn`s on struct field patterns bound to wildcards.
///
declare_lint!(pub UNNEEDED_FIELD_PATTERN, Warn,
"Struct fields are bound to a wildcard instead of using `..`");
+/// **What it does:** This lint `Warn`s on function arguments having the similar names differing by an underscore
+///
+/// **Why is this bad?** It affects code readability
+///
+/// **Known problems:** None.
+///
+/// **Example:** `fn foo(a: i32, _a: i32) {}`
+declare_lint!(pub DUPLICATE_UNDERSCORE_ARGUMENT, Warn,
+ "Function arguments having names which only differ by an underscore");
+
#[derive(Copy, Clone)]
pub struct MiscEarly;
impl LintPass for MiscEarly {
fn get_lints(&self) -> LintArray {
- lint_array!(UNNEEDED_FIELD_PATTERN)
+ lint_array!(UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT)
}
}
impl EarlyLintPass for MiscEarly {
fn check_pat(&mut self, cx: &EarlyContext, pat: &Pat) {
- if let PatStruct(_, ref pfields, _) = pat.node {
+ if let PatStruct(ref npat, ref pfields, _) = pat.node {
let mut wilds = 0;
+ let type_name = match npat.segments.last() {
+ Some(elem) => format!("{}", elem.identifier.name),
+ None => String::new(),
+ };
for field in pfields {
if field.node.pat.node == PatWild {
}
}
if !pfields.is_empty() && wilds == pfields.len() {
- span_lint(cx, UNNEEDED_FIELD_PATTERN, pat.span,
- "All the struct fields are matched to a wildcard pattern, \
- consider using `..`.");
+ span_help_and_lint(cx,
+ UNNEEDED_FIELD_PATTERN,
+ pat.span,
+ "All the struct fields are matched to a wildcard pattern, consider using `..`.",
+ &format!("Try with `{} {{ .. }}` instead", type_name));
return;
}
if wilds > 0 {
+ let mut normal = vec![];
+
+ for field in pfields {
+ if field.node.pat.node != PatWild {
+ if let Ok(n) = cx.sess().codemap().span_to_snippet(field.span) {
+ normal.push(n);
+ }
+ }
+ }
for field in pfields {
if field.node.pat.node == PatWild {
- span_lint(cx, UNNEEDED_FIELD_PATTERN, field.span,
- "You matched a field with a wildcard pattern. \
- Consider using `..` instead");
+ wilds -= 1;
+ if wilds > 0 {
+ span_lint(cx,
+ UNNEEDED_FIELD_PATTERN,
+ field.span,
+ "You matched a field with a wildcard pattern. Consider using `..` instead");
+ } else {
+ span_help_and_lint(cx,
+ UNNEEDED_FIELD_PATTERN,
+ field.span,
+ "You matched a field with a wildcard pattern. Consider using `..` \
+ instead",
+ &format!("Try with `{} {{ {}, .. }}`",
+ type_name,
+ normal[..].join(", ")));
+ }
+ }
+ }
+ }
+ }
+ }
+
+ fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) {
+ let mut registered_names: HashMap<String, Span> = HashMap::new();
+
+ for ref arg in &decl.inputs {
+ if let PatIdent(_, sp_ident, None) = arg.pat.node {
+ let arg_name = sp_ident.node.to_string();
+
+ if arg_name.starts_with("_") {
+ if let Some(correspondance) = registered_names.get(&arg_name[1..]) {
+ span_lint(cx,
+ DUPLICATE_UNDERSCORE_ARGUMENT,
+ *correspondance,
+ &format!("`{}` already exists, having another argument having almost the same \
+ name makes code comprehension and documentation more difficult",
+ arg_name[1..].to_owned()));
}
+ } else {
+ registered_names.insert(arg_name, arg.pat.span.clone());
}
}
}
}
fn check_ty(&mut self, cx: &LateContext, ty: &Ty) {
- unwrap_mut(ty).and_then(unwrap_mut).map_or((), |_| span_lint(cx, MUT_MUT,
- ty.span, "generally you want to avoid `&mut &mut _` if possible"))
+ unwrap_mut(ty).and_then(unwrap_mut).map_or((), |_| {
+ span_lint(cx, MUT_MUT, ty.span, "generally you want to avoid `&mut &mut _` if possible");
+ });
}
}
fn check_expr_mut(cx: &LateContext, expr: &Expr) {
- if in_external_macro(cx, expr.span) { return; }
+ if in_external_macro(cx, expr.span) {
+ return;
+ }
fn unwrap_addr(expr: &Expr) -> Option<&Expr> {
match expr.node {
ExprAddrOf(MutMutable, ref e) => Some(e),
- _ => None
+ _ => None,
}
}
unwrap_addr(expr).map_or((), |e| {
- unwrap_addr(e).map_or_else(
- || {
- if let TyRef(_, TypeAndMut{mutbl: MutMutable, ..}) =
- cx.tcx.expr_ty(e).sty {
- span_lint(cx, MUT_MUT, expr.span,
- "this expression mutably borrows a mutable reference. \
- Consider reborrowing")
- }
- },
- |_| {
- span_lint(cx, MUT_MUT, expr.span,
- "generally you want to avoid `&mut &mut _` if possible")
- }
- )
+ unwrap_addr(e).map_or_else(|| {
+ if let TyRef(_, TypeAndMut{mutbl: MutMutable, ..}) = cx.tcx.expr_ty(e).sty {
+ span_lint(cx,
+ MUT_MUT,
+ expr.span,
+ "this expression mutably borrows a mutable reference. Consider \
+ reborrowing");
+ }
+ },
+ |_| {
+ span_lint(cx,
+ MUT_MUT,
+ expr.span,
+ "generally you want to avoid `&mut &mut _` if possible");
+ })
})
}
fn unwrap_mut(ty: &Ty) -> Option<&Ty> {
match ty.node {
TyRptr(_, MutTy{ ty: ref pty, mutbl: MutMutable }) => Some(pty),
- _ => None
+ _ => None,
}
}
use rustc::middle::ty::{TypeAndMut, TypeVariants, MethodCall, TyS};
use syntax::ptr::P;
-/// **What it does:** This lint detects giving a mutable reference to a function that only requires an immutable reference.
+/// **What it does:** This lint detects giving a mutable reference to a function that only requires an immutable reference. It is `Warn` by default.
///
/// **Why is this bad?** The immutable reference rules out all other references to the value. Also the code misleads about the intent of the call site.
///
match borrowed_table.node_types.get(&fn_expr.id) {
Some(function_type) => {
if let ExprPath(_, ref path) = fn_expr.node {
- check_arguments(cx, &arguments, function_type,
- &format!("{}", path));
+ check_arguments(cx, &arguments, function_type, &format!("{}", path));
}
}
None => unreachable!(), // A function with unknown type is called.
- // If this happened the compiler would have aborted the
- // compilation long ago.
+ // If this happened the compiler would have aborted the
+ // compilation long ago.
};
ExprMethodCall(ref name, _, ref arguments) => {
let method_call = MethodCall::expr(e.id);
match borrowed_table.method_map.get(&method_call) {
- Some(method_type) => check_arguments(cx, &arguments, method_type.ty,
- &format!("{}", name.node.as_str())),
+ Some(method_type) => {
+ check_arguments(cx, &arguments, method_type.ty, &format!("{}", name.node.as_str()))
+ }
None => unreachable!(), // Just like above, this should never happen.
};
}
TypeVariants::TyRef(_, TypeAndMut {mutbl: MutImmutable, ..}) |
TypeVariants::TyRawPtr(TypeAndMut {mutbl: MutImmutable, ..}) => {
if let ExprAddrOf(MutMutable, _) = argument.node {
- span_lint(cx, UNNECESSARY_MUT_PASSED,
- argument.span, &format!("The function/method \"{}\" \
- doesn't need a mutable reference",
- name));
+ span_lint(cx,
+ UNNECESSARY_MUT_PASSED,
+ argument.span,
+ &format!("The function/method \"{}\" doesn't need a mutable reference", name));
}
}
_ => {}
if match_type(cx, ty, &MUTEX_PATH) {
let mutex_param = &subst.types.get(ParamSpace::TypeSpace, 0).sty;
if let Some(atomic_name) = get_atomic_name(mutex_param) {
- let msg = format!("Consider using an {} instead of a \
- Mutex here. If you just want the \
- locking behaviour and not the internal \
- type, consider using Mutex<()>.",
+ let msg = format!("Consider using an {} instead of a Mutex here. If you just want the locking \
+ behaviour and not the internal type, consider using Mutex<()>.",
atomic_name);
match *mutex_param {
- ty::TyUint(t) if t != ast::TyUs =>
- span_lint(cx, MUTEX_INTEGER, expr.span, &msg),
- ty::TyInt(t) if t != ast::TyIs =>
- span_lint(cx, MUTEX_INTEGER, expr.span, &msg),
- _ => span_lint(cx, MUTEX_ATOMIC, expr.span, &msg)
- }
+ ty::TyUint(t) if t != ast::TyUs => span_lint(cx, MUTEX_INTEGER, expr.span, &msg),
+ ty::TyInt(t) if t != ast::TyIs => span_lint(cx, MUTEX_INTEGER, expr.span, &msg),
+ _ => span_lint(cx, MUTEX_ATOMIC, expr.span, &msg),
+ };
}
}
}
ty::TyUint(_) => Some("AtomicUsize"),
ty::TyInt(_) => Some("AtomicIsize"),
ty::TyRawPtr(_) => Some("AtomicPtr"),
- _ => None
+ _ => None,
}
}
if let ExprIf(ref pred, ref then_block, Some(ref else_expr)) = e.node {
match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) {
(Some(true), Some(true)) => {
- span_lint(cx, NEEDLESS_BOOL, e.span,
+ span_lint(cx,
+ NEEDLESS_BOOL,
+ e.span,
"this if-then-else expression will always return true");
}
(Some(false), Some(false)) => {
- span_lint(cx, NEEDLESS_BOOL, e.span,
+ span_lint(cx,
+ NEEDLESS_BOOL,
+ e.span,
"this if-then-else expression will always return false");
}
(Some(true), Some(false)) => {
let pred_snip = snippet(cx, pred.span, "..");
- let hint = if pred_snip == ".." { "its predicate".into() } else {
+ let hint = if pred_snip == ".." {
+ "its predicate".into()
+ } else {
format!("`{}`", pred_snip)
};
- span_lint(cx, NEEDLESS_BOOL, e.span, &format!(
- "you can reduce this if-then-else expression to just {}", hint));
+ span_lint(cx,
+ NEEDLESS_BOOL,
+ e.span,
+ &format!("you can reduce this if-then-else expression to just {}", hint));
}
(Some(false), Some(true)) => {
let pred_snip = snippet(cx, pred.span, "..");
- let hint = if pred_snip == ".." { "`!` and its predicate".into() } else {
+ let hint = if pred_snip == ".." {
+ "`!` and its predicate".into()
+ } else {
format!("`!{}`", pred_snip)
};
- span_lint(cx, NEEDLESS_BOOL, e.span, &format!(
- "you can reduce this if-then-else expression to just {}", hint));
+ span_lint(cx,
+ NEEDLESS_BOOL,
+ e.span,
+ &format!("you can reduce this if-then-else expression to just {}", hint));
}
- _ => ()
+ _ => (),
}
}
}
fn fetch_bool_block(block: &Block) -> Option<bool> {
if block.stmts.is_empty() {
block.expr.as_ref().and_then(|e| fetch_bool_expr(e))
- } else { None }
+ } else {
+ None
+ }
}
fn fetch_bool_expr(expr: &Expr) -> Option<bool> {
match expr.node {
ExprBlock(ref block) => fetch_bool_block(block),
- ExprLit(ref lit_ptr) => if let LitBool(value) = lit_ptr.node {
- Some(value) } else { None },
- _ => None
+ ExprLit(ref lit_ptr) => {
+ if let LitBool(value) = lit_ptr.node {
+ Some(value)
+ } else {
+ None
+ }
+ }
+ _ => None,
}
}
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let ExprMethodCall(ref name, _, _) = expr.node {
if name.node.as_str() == "as_slice" && check_paths(cx, expr) {
- span_lint(cx, UNSTABLE_AS_SLICE, expr.span,
- "used as_slice() from the 'convert' nightly feature. Use &[..] \
- instead");
+ span_lint(cx,
+ UNSTABLE_AS_SLICE,
+ expr.span,
+ "used as_slice() from the 'convert' nightly feature. Use &[..] instead");
}
if name.node.as_str() == "as_mut_slice" && check_paths(cx, expr) {
- span_lint(cx, UNSTABLE_AS_MUT_SLICE, expr.span,
- "used as_mut_slice() from the 'convert' nightly feature. Use &mut [..] \
- instead");
+ span_lint(cx,
+ UNSTABLE_AS_MUT_SLICE,
+ expr.span,
+ "used as_mut_slice() from the 'convert' nightly feature. Use &mut [..] instead");
}
}
}
let ty = cx.tcx.expr_ty(expr);
if let TyStruct(def, _) = ty.sty {
if fields.len() == def.struct_variant().fields.len() {
- span_lint(cx, NEEDLESS_UPDATE, base.span,
- "struct update has no effect, all the fields \
- in the struct have already been specified");
+ span_lint(cx,
+ NEEDLESS_UPDATE,
+ base.span,
+ "struct update has no effect, all the fields in the struct have already been specified");
}
}
}
let def = cx.tcx.def_map.borrow().get(&callee.id).map(|d| d.full_def());
match def {
Some(DefStruct(..)) |
- Some(DefVariant(..)) => {
- args.iter().all(|arg| has_no_effect(cx, arg))
- }
+ Some(DefVariant(..)) => args.iter().all(|arg| has_no_effect(cx, arg)),
_ => false,
}
}
fn check_stmt(&mut self, cx: &LateContext, stmt: &Stmt) {
if let StmtSemi(ref expr, _) = stmt.node {
if has_no_effect(cx, expr) {
- span_lint(cx, NO_EFFECT, stmt.span,
- "statement with no effect");
+ span_lint(cx, NO_EFFECT, stmt.span, "statement with no effect");
}
}
}
fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
if let ExprMethodCall(ref name, _, ref arguments) = e.node {
let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&arguments[0]));
- if name.node.as_str() == "open" && match_type(cx, obj_ty, &OPEN_OPTIONS_PATH){
+ if name.node.as_str() == "open" && match_type(cx, obj_ty, &OPEN_OPTIONS_PATH) {
let mut options = Vec::new();
get_open_options(cx, &arguments[0], &mut options);
check_open_options(cx, &options, e.span);
}
}
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
enum Argument {
True,
False,
- Unknown
+ Unknown,
}
#[derive(Debug)]
Read,
Truncate,
Create,
- Append
+ Append,
}
fn get_open_options(cx: &LateContext, argument: &Expr, options: &mut Vec<(OpenOption, Argument)>) {
if let ExprMethodCall(ref name, _, ref arguments) = argument.node {
let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&arguments[0]));
-
+
// Only proceed if this is a call on some object of type std::fs::OpenOptions
if match_type(cx, obj_ty, &OPEN_OPTIONS_PATH) && arguments.len() >= 2 {
-
+
let argument_option = match arguments[1].node {
ExprLit(ref span) => {
if let Spanned {node: LitBool(lit), ..} = **span {
- if lit {Argument::True} else {Argument::False}
+ if lit {
+ Argument::True
+ } else {
+ Argument::False
+ }
} else {
return; // The function is called with a literal
// which is not a boolean literal. This is theoretically
// possible, but not very likely.
}
}
- _ => {
- Argument::Unknown
- }
+ _ => Argument::Unknown,
};
-
+
match &*name.node.as_str() {
"create" => {
options.push((OpenOption::Create, argument_option));
}
_ => {}
}
-
+
get_open_options(cx, &arguments[0], options);
}
}
}
-fn check_for_duplicates(cx: &LateContext, options: &[(OpenOption, Argument)], span: Span) {
+fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span: Span) {
+ let (mut create, mut append, mut truncate, mut read, mut write) = (false, false, false, false, false);
+ let (mut create_arg, mut append_arg, mut truncate_arg, mut read_arg, mut write_arg) = (false,
+ false,
+ false,
+ false,
+ false);
// This code is almost duplicated (oh, the irony), but I haven't found a way to unify it.
- if options.iter().filter(|o| if let (OpenOption::Create, _) = **o {true} else {false}).count() > 1 {
- span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"create\" \
- is called more than once");
- }
- if options.iter().filter(|o| if let (OpenOption::Append, _) = **o {true} else {false}).count() > 1 {
- span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"append\" \
- is called more than once");
- }
- if options.iter().filter(|o| if let (OpenOption::Truncate, _) = **o {true} else {false}).count() > 1 {
- span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"truncate\" \
- is called more than once");
- }
- if options.iter().filter(|o| if let (OpenOption::Read, _) = **o {true} else {false}).count() > 1 {
- span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"read\" \
- is called more than once");
- }
- if options.iter().filter(|o| if let (OpenOption::Write, _) = **o {true} else {false}).count() > 1 {
- span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "The method \"write\" \
- is called more than once");
+
+ for option in options {
+ match *option {
+ (OpenOption::Create, arg) => {
+ if create {
+ span_lint(cx,
+ NONSENSICAL_OPEN_OPTIONS,
+ span,
+ "The method \"create\" is called more than once");
+ } else {
+ create = true
+ }
+ create_arg = create_arg || (arg == Argument::True);;
+ }
+ (OpenOption::Append, arg) => {
+ if append {
+ span_lint(cx,
+ NONSENSICAL_OPEN_OPTIONS,
+ span,
+ "The method \"append\" is called more than once");
+ } else {
+ append = true
+ }
+ append_arg = append_arg || (arg == Argument::True);;
+ }
+ (OpenOption::Truncate, arg) => {
+ if truncate {
+ span_lint(cx,
+ NONSENSICAL_OPEN_OPTIONS,
+ span,
+ "The method \"truncate\" is called more than once");
+ } else {
+ truncate = true
+ }
+ truncate_arg = truncate_arg || (arg == Argument::True);
+ }
+ (OpenOption::Read, arg) => {
+ if read {
+ span_lint(cx,
+ NONSENSICAL_OPEN_OPTIONS,
+ span,
+ "The method \"read\" is called more than once");
+ } else {
+ read = true
+ }
+ read_arg = read_arg || (arg == Argument::True);;
+ }
+ (OpenOption::Write, arg) => {
+ if write {
+ span_lint(cx,
+ NONSENSICAL_OPEN_OPTIONS,
+ span,
+ "The method \"write\" is called more than once");
+ } else {
+ write = true
+ }
+ write_arg = write_arg || (arg == Argument::True);;
+ }
+ }
}
-}
-fn check_for_inconsistencies(cx: &LateContext, options: &[(OpenOption, Argument)], span: Span) {
- // Truncate + read makes no sense.
- if options.iter().filter(|o| if let (OpenOption::Read, Argument::True) = **o {true} else {false}).count() > 0 &&
- options.iter().filter(|o| if let (OpenOption::Truncate, Argument::True) = **o {true} else {false}).count() > 0 {
+ if read && truncate && read_arg && truncate_arg {
span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "File opened with \"truncate\" and \"read\"");
}
-
- // Append + truncate makes no sense.
- if options.iter().filter(|o| if let (OpenOption::Append, Argument::True) = **o {true} else {false}).count() > 0 &&
- options.iter().filter(|o| if let (OpenOption::Truncate, Argument::True) = **o {true} else {false}).count() > 0 {
- span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "File opened with \"append\" and \"truncate\"");
+ if append && truncate && append_arg && truncate_arg {
+ span_lint(cx,
+ NONSENSICAL_OPEN_OPTIONS,
+ span,
+ "File opened with \"append\" and \"truncate\"");
}
}
-
-fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span: Span) {
- check_for_duplicates(cx, options, span);
- check_for_inconsistencies(cx, options, span);
-}
use utils::{span_lint, snippet};
-/// **What it does:** This lint checks for operations where precedence may be unclear and `Warn`'s about them by default, suggesting to add parentheses. Currently it catches the following:
+/// **What it does:** This lint checks for operations where precedence may be unclear and `Warn`s about them by default, suggesting to add parentheses. Currently it catches the following:
/// * mixed usage of arithmetic and bit shifting/combining operators without parentheses
/// * a "negative" numeric literal (which is really a unary `-` followed by a numeric literal) followed by a method call
///
impl EarlyLintPass for Precedence {
fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
if let ExprBinary(Spanned { node: op, ..}, ref left, ref right) = expr.node {
- if !is_bit_op(op) { return; }
+ if !is_bit_op(op) {
+ return;
+ }
match (is_arith_expr(left), is_arith_expr(right)) {
- (true, true) => span_lint(cx, PRECEDENCE, expr.span,
- &format!("operator precedence can trip the unwary. \
- Consider parenthesizing your expression:\
- `({}) {} ({})`", snippet(cx, left.span, ".."),
- op.to_string(), snippet(cx, right.span, ".."))),
- (true, false) => span_lint(cx, PRECEDENCE, expr.span,
- &format!("operator precedence can trip the unwary. \
- Consider parenthesizing your expression:\
- `({}) {} {}`", snippet(cx, left.span, ".."),
- op.to_string(), snippet(cx, right.span, ".."))),
- (false, true) => span_lint(cx, PRECEDENCE, expr.span,
- &format!("operator precedence can trip the unwary. \
- Consider parenthesizing your expression:\
- `{} {} ({})`", snippet(cx, left.span, ".."),
- op.to_string(), snippet(cx, right.span, ".."))),
+ (true, true) => {
+ span_lint(cx,
+ PRECEDENCE,
+ expr.span,
+ &format!("operator precedence can trip the unwary. Consider parenthesizing your \
+ expression:`({}) {} ({})`",
+ snippet(cx, left.span, ".."),
+ op.to_string(),
+ snippet(cx, right.span, "..")));
+ }
+ (true, false) => {
+ span_lint(cx,
+ PRECEDENCE,
+ expr.span,
+ &format!("operator precedence can trip the unwary. Consider parenthesizing your \
+ expression:`({}) {} {}`",
+ snippet(cx, left.span, ".."),
+ op.to_string(),
+ snippet(cx, right.span, "..")));
+ }
+ (false, true) => {
+ span_lint(cx,
+ PRECEDENCE,
+ expr.span,
+ &format!("operator precedence can trip the unwary. Consider parenthesizing your \
+ expression:`{} {} ({})`",
+ snippet(cx, left.span, ".."),
+ op.to_string(),
+ snippet(cx, right.span, "..")));
+ }
_ => (),
}
}
if let Some(slf) = args.first() {
if let ExprLit(ref lit) = slf.node {
match lit.node {
- LitInt(..) | LitFloat(..) | LitFloatUnsuffixed(..) =>
- span_lint(cx, PRECEDENCE, expr.span, &format!(
- "unary minus has lower precedence than \
- method call. Consider adding parentheses \
- to clarify your intent: -({})",
- snippet(cx, rhs.span, ".."))),
- _ => ()
+ LitInt(..) | LitFloat(..) | LitFloatUnsuffixed(..) => {
+ span_lint(cx,
+ PRECEDENCE,
+ expr.span,
+ &format!("unary minus has lower precedence than method call. Consider \
+ adding parentheses to clarify your intent: -({})",
+ snippet(cx, rhs.span, "..")));
+ }
+ _ => (),
}
}
}
fn is_arith_expr(expr: &Expr) -> bool {
match expr.node {
ExprBinary(Spanned { node: op, ..}, _, _) => is_arith_op(op),
- _ => false
+ _ => false,
}
}
fn is_bit_op(op: BinOp_) -> bool {
match op {
BiBitXor | BiBitAnd | BiBitOr | BiShl | BiShr => true,
- _ => false
+ _ => false,
}
}
fn is_arith_op(op: BinOp_) -> bool {
match op {
BiAdd | BiSub | BiMul | BiDiv | BiRem => true,
- _ => false
+ _ => false,
}
}
if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&arg.ty.id) {
if let ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = ty.sty {
if match_type(cx, ty, &VEC_PATH) {
- span_lint(cx, PTR_ARG, arg.ty.span,
- "writing `&Vec<_>` instead of `&[_]` involves one more reference \
- and cannot be used with non-Vec-based slices. Consider changing \
- the type to `&[...]`");
+ span_lint(cx,
+ PTR_ARG,
+ arg.ty.span,
+ "writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used \
+ with non-Vec-based slices. Consider changing the type to `&[...]`");
} else if match_type(cx, ty, &STRING_PATH) {
- span_lint(cx, PTR_ARG, arg.ty.span,
- "writing `&String` instead of `&str` involves a new object \
- where a slice will do. Consider changing the type to `&str`");
+ span_lint(cx,
+ PTR_ARG,
+ arg.ty.span,
+ "writing `&String` instead of `&str` involves a new object where a slice will do. \
+ Consider changing the type to `&str`");
}
}
}
impl LateLintPass for StepByZero {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
- if let ExprMethodCall(Spanned { node: ref name, .. }, _,
- ref args) = expr.node {
+ if let ExprMethodCall(Spanned { node: ref name, .. }, _, ref args) = expr.node {
// Range with step_by(0).
- if name.as_str() == "step_by" && args.len() == 2 &&
- is_range(cx, &args[0]) && is_integer_literal(&args[1], 0) {
- cx.span_lint(RANGE_STEP_BY_ZERO, expr.span,
- "Range::step_by(0) produces an infinite iterator. \
- Consider using `std::iter::repeat()` instead")
- }
-
- // x.iter().zip(0..x.len())
- else if name.as_str() == "zip" && args.len() == 2 {
+ if name.as_str() == "step_by" && args.len() == 2 && is_range(cx, &args[0]) &&
+ is_integer_literal(&args[1], 0) {
+ cx.span_lint(RANGE_STEP_BY_ZERO,
+ expr.span,
+ "Range::step_by(0) produces an infinite iterator. Consider using `std::iter::repeat()` \
+ instead")
+ } else if name.as_str() == "zip" && args.len() == 2 {
let iter = &args[0].node;
let zip_arg = &args[1].node;
if_let_chain! {
use rustc::lint::*;
use syntax::ast::*;
-//use reexport::*;
+// use reexport::*;
use syntax::codemap::{Span, Spanned};
use syntax::visit::FnKind;
self.check_final_expr(cx, &arm.body);
}
}
- _ => { }
+ _ => {}
}
}
fn emit_return_lint(&mut self, cx: &EarlyContext, spans: (Span, Span)) {
- if in_external_macro(cx, spans.1) {return;}
- span_lint_and_then(cx, NEEDLESS_RETURN, spans.0,
- "unneeded return statement",
- || {
+ if in_external_macro(cx, spans.1) {
+ return;
+ }
+ span_lint_and_then(cx, NEEDLESS_RETURN, spans.0, "unneeded return statement", |db| {
if let Some(snippet) = snippet_opt(cx, spans.1) {
- cx.sess().span_suggestion(spans.0,
- "remove `return` as shown:",
- snippet);
+ db.span_suggestion(spans.0, "remove `return` as shown:", snippet);
}
});
}
}
fn emit_let_lint(&mut self, cx: &EarlyContext, lint_span: Span, note_span: Span) {
- if in_external_macro(cx, note_span) {return;}
- span_lint(cx, LET_AND_RETURN, lint_span,
- "returning the result of a let binding from a block. \
- Consider returning the expression directly.");
+ if in_external_macro(cx, note_span) {
+ return;
+ }
+ let mut db = span_lint(cx,
+ LET_AND_RETURN,
+ lint_span,
+ "returning the result of a let binding from a block. Consider returning the \
+ expression directly.");
if cx.current_level(LET_AND_RETURN) != Level::Allow {
- cx.sess().span_note(note_span,
- "this expression can be directly returned");
+ db.span_note(note_span, "this expression can be directly returned");
}
}
}
}
impl EarlyLintPass for ReturnPass {
- fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, _: &FnDecl,
- block: &Block, _: Span, _: NodeId) {
+ fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, _: &FnDecl, block: &Block, _: Span, _: NodeId) {
self.check_block_return(cx, block);
}
use rustc::lint::*;
use rustc::middle::def::Def::{DefVariant, DefStruct};
-use utils::{is_from_for_desugar, in_external_macro, snippet, span_lint, span_note_and_lint};
+use utils::{is_from_for_desugar, in_external_macro, snippet, span_lint, span_note_and_lint, DiagnosticWrapper};
/// **What it does:** This lint checks for bindings that shadow other bindings already in scope, while just changing reference level or mutability. It is `Allow` by default.
///
fn get_lints(&self) -> LintArray {
lint_array!(SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED)
}
-
}
impl LateLintPass for ShadowPass {
- fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl,
- block: &Block, _: Span, _: NodeId) {
- if in_external_macro(cx, block.span) { return; }
+ fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl, block: &Block, _: Span, _: NodeId) {
+ if in_external_macro(cx, block.span) {
+ return;
+ }
check_fn(cx, decl, block);
}
}
for stmt in &block.stmts {
match stmt.node {
StmtDecl(ref decl, _) => check_decl(cx, decl, bindings),
- StmtExpr(ref e, _) | StmtSemi(ref e, _) =>
- check_expr(cx, e, bindings)
+ StmtExpr(ref e, _) | StmtSemi(ref e, _) => check_expr(cx, e, bindings),
}
}
- if let Some(ref o) = block.expr { check_expr(cx, o, bindings); }
+ if let Some(ref o) = block.expr {
+ check_expr(cx, o, bindings);
+ }
bindings.truncate(len);
}
fn check_decl(cx: &LateContext, decl: &Decl, bindings: &mut Vec<(Name, Span)>) {
- if in_external_macro(cx, decl.span) { return; }
- if is_from_for_desugar(decl) { return; }
+ if in_external_macro(cx, decl.span) {
+ return;
+ }
+ if is_from_for_desugar(decl) {
+ return;
+ }
if let DeclLocal(ref local) = decl.node {
let Local{ ref pat, ref ty, ref init, span, .. } = **local;
- if let Some(ref t) = *ty { check_ty(cx, t, bindings) }
+ if let Some(ref t) = *ty {
+ check_ty(cx, t, bindings)
+ }
if let Some(ref o) = *init {
check_expr(cx, o, bindings);
check_pat(cx, pat, &Some(o), span, bindings);
fn is_binding(cx: &LateContext, pat: &Pat) -> bool {
match cx.tcx.def_map.borrow().get(&pat.id).map(|d| d.full_def()) {
Some(DefVariant(..)) | Some(DefStruct(..)) => false,
- _ => true
+ _ => true,
}
}
-fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span,
- bindings: &mut Vec<(Name, Span)>) {
- //TODO: match more stuff / destructuring
+fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, bindings: &mut Vec<(Name, Span)>) {
+ // TODO: match more stuff / destructuring
match pat.node {
PatIdent(_, ref ident, ref inner) => {
let name = ident.node.unhygienic_name;
bindings.push((name, ident.span));
}
}
- if let Some(ref p) = *inner { check_pat(cx, p, init, span, bindings); }
+ if let Some(ref p) = *inner {
+ check_pat(cx, p, init, span, bindings);
+ }
}
- //PatEnum(Path, Option<Vec<P<Pat>>>),
- PatStruct(_, ref pfields, _) =>
+ //
PatEnum(Path, Option<Vec<P<Pat>>>),
+ PatStruct(_, ref pfields, _) => {
if let Some(ref init_struct) = *init {
if let ExprStruct(_, ref efields, _) = init_struct.node {
for field in pfields {
let name = field.node.name;
let efield = efields.iter()
- .find(|ref f| f.name.node == name)
- .map(|f| &*f.expr);
+ .find(|ref f| f.name.node == name)
+ .map(|f| &*f.expr);
check_pat(cx, &field.node.pat, &efield, span, bindings);
}
} else {
for field in pfields {
check_pat(cx, &field.node.pat, &None, span, bindings);
}
- },
- PatTup(ref inner) =>
+ }
+ }
+ PatTup(ref inner) => {
if let Some(ref init_tup) = *init {
if let ExprTup(ref tup) = init_tup.node {
for (i, p) in inner.iter().enumerate() {
for p in inner {
check_pat(cx, p, &None, span, bindings);
}
- },
+ }
+ }
PatBox(ref inner) => {
if let Some(ref initp) = *init {
if let ExprBox(ref inner_init) = initp.node {
check_pat(cx, inner, init, span, bindings);
}
}
- PatRegion(ref inner, _) =>
- check_pat(cx, inner, init, span, bindings),
- //PatVec(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>),
+ PatRegion(ref inner, _) => check_pat(cx, inner, init, span, bindings),
+ // PatVec(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>),
_ => (),
}
}
-fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init:
- &Option<T>, prev_span: Span) where T: Deref<Target=Expr> {
- fn note_orig(cx: &LateContext, lint: &'static Lint, span: Span) {
+fn lint_shadow<T>(cx: &LateContext, name: Name, span: Span, lspan: Span, init: &Option<T>, prev_span: Span)
+ where T: Deref<Target = Expr>
+{
+ fn note_orig(cx: &LateContext, mut db: DiagnosticWrapper, lint: &'static Lint, span: Span) {
if cx.current_level(lint) != Level::Allow {
- cx.sess().span_note(span, "previous binding is here");
+ db.span_note(span, "previous binding is here");
}
}
if let Some(ref expr) = *init {
if is_self_shadow(name, expr) {
- span_lint(cx, SHADOW_SAME, span, &format!(
- "{} is shadowed by itself in {}",
- snippet(cx, lspan, "_"),
- snippet(cx, expr.span, "..")));
- note_orig(cx, SHADOW_SAME, prev_span);
+ let db = span_lint(cx,
+ SHADOW_SAME,
+ span,
+ &format!("{} is shadowed by itself in {}",
+ snippet(cx, lspan, "_"),
+ snippet(cx, expr.span, "..")));
+ note_orig(cx, db, SHADOW_SAME, prev_span);
} else {
if contains_self(name, expr) {
- span_note_and_lint(cx, SHADOW_REUSE, lspan, &format!(
- "{} is shadowed by {} which reuses the original value",
- snippet(cx, lspan, "_"),
- snippet(cx, expr.span, "..")),
- expr.span, "initialization happens here");
- note_orig(cx, SHADOW_REUSE, prev_span);
+ let db = span_note_and_lint(cx,
+ SHADOW_REUSE,
+ lspan,
+ &format!("{} is shadowed by {} which reuses the original value",
+ snippet(cx, lspan, "_"),
+ snippet(cx, expr.span, "..")),
+ expr.span,
+ "initialization happens here");
+ note_orig(cx, db, SHADOW_REUSE, prev_span);
} else {
- span_note_and_lint(cx, SHADOW_UNRELATED, lspan, &format!(
- "{} is shadowed by {}",
- snippet(cx, lspan, "_"),
- snippet(cx, expr.span, "..")),
- expr.span, "initialization happens here");
- note_orig(cx, SHADOW_UNRELATED, prev_span);
+ let db = span_note_and_lint(cx,
+ SHADOW_UNRELATED,
+ lspan,
+ &format!("{} is shadowed by {}",
+ snippet(cx, lspan, "_"),
+ snippet(cx, expr.span, "..")),
+ expr.span,
+ "initialization happens here");
+ note_orig(cx, db, SHADOW_UNRELATED, prev_span);
}
}
} else {
- span_lint(cx, SHADOW_UNRELATED, span, &format!(
- "{} shadows a previous declaration", snippet(cx, lspan, "_")));
- note_orig(cx, SHADOW_UNRELATED, prev_span);
+ let db = span_lint(cx,
+ SHADOW_UNRELATED,
+ span,
+ &format!("{} shadows a previous declaration", snippet(cx, lspan, "_")));
+ note_orig(cx, db, SHADOW_UNRELATED, prev_span);
}
}
fn check_expr(cx: &LateContext, expr: &Expr, bindings: &mut Vec<(Name, Span)>) {
- if in_external_macro(cx, expr.span) { return; }
+ if in_external_macro(cx, expr.span) {
+ return;
+ }
match expr.node {
- ExprUnary(_, ref e) | ExprField(ref e, _) |
- ExprTupField(ref e, _) | ExprAddrOf(_, ref e) | ExprBox(ref e)
- => { check_expr(cx, e, bindings) }
- ExprBlock(ref block) | ExprLoop(ref block, _) =>
- { check_block(cx, block, bindings) }
- //ExprCall
- //ExprMethodCall
- ExprVec(ref v) | ExprTup(ref v) =>
- for ref e in v { check_expr(cx, e, bindings) },
+ ExprUnary(_, ref e) |
+ ExprField(ref e, _) |
+ ExprTupField(ref e, _) |
+ ExprAddrOf(_, ref e) |
+ ExprBox(ref e) => check_expr(cx, e, bindings),
+ ExprBlock(ref block) | ExprLoop(ref block, _) => check_block(cx, block, bindings),
+ // ExprCall
+ // ExprMethodCall
+ ExprVec(ref v) | ExprTup(ref v) => {
+ for ref e in v {
+ check_expr(cx, e, bindings)
+ }
+ }
ExprIf(ref cond, ref then, ref otherwise) => {
check_expr(cx, cond, bindings);
check_block(cx, then, bindings);
- if let Some(ref o) = *otherwise { check_expr(cx, o, bindings); }
+ if let Some(ref o) = *otherwise {
+ check_expr(cx, o, bindings);
+ }
}
ExprWhile(ref cond, ref block, _) => {
check_expr(cx, cond, bindings);
for ref arm in arms {
for ref pat in &arm.pats {
check_pat(cx, &pat, &Some(&**init), pat.span, bindings);
- //This is ugly, but needed to get the right type
+ // This is ugly, but needed to get the right type
if let Some(ref guard) = arm.guard {
check_expr(cx, guard, bindings);
}
}
}
}
- _ => ()
+ _ => (),
}
}
}
TyPtr(MutTy{ ty: ref mty, .. }) |
TyRptr(_, MutTy{ ty: ref mty, .. }) => check_ty(cx, mty, bindings),
- TyTup(ref tup) => { for ref t in tup { check_ty(cx, t, bindings) } }
+ TyTup(ref tup) => {
+ for ref t in tup {
+ check_ty(cx, t, bindings)
+ }
+ }
TyTypeof(ref expr) => check_expr(cx, expr, bindings),
_ => (),
}
match expr.node {
ExprBox(ref inner) |
ExprAddrOf(_, ref inner) => is_self_shadow(name, inner),
- ExprBlock(ref block) => block.stmts.is_empty() && block.expr.as_ref().
- map_or(false, |ref e| is_self_shadow(name, e)),
- ExprUnary(op, ref inner) => (UnDeref == op) &&
- is_self_shadow(name, inner),
+ ExprBlock(ref block) => {
+ block.stmts.is_empty() && block.expr.as_ref().map_or(false, |ref e| is_self_shadow(name, e))
+ }
+ ExprUnary(op, ref inner) => (UnDeref == op) && is_self_shadow(name, inner),
ExprPath(_, ref path) => path_eq_name(name, path),
_ => false,
}
}
fn path_eq_name(name: Name, path: &Path) -> bool {
- !path.global && path.segments.len() == 1 &&
- path.segments[0].identifier.unhygienic_name == name
+ !path.global && path.segments.len() == 1 && path.segments[0].identifier.unhygienic_name == name
}
struct ContainsSelf {
name: Name,
- result: bool
+ result: bool,
}
impl<'v> Visitor<'v> for ContainsSelf {
}
fn contains_self(name: Name, expr: &Expr) -> bool {
- let mut cs = ContainsSelf { name: name, result: false };
+ let mut cs = ContainsSelf {
+ name: name,
+ result: false,
+ };
cs.visit_expr(expr);
cs.result
}
use rustc_front::hir::*;
use syntax::codemap::Spanned;
-use eq_op::is_exp_equal;
-use utils::{match_type, span_lint, walk_ptrs_ty, get_parent_expr};
+use utils::{is_exp_equal, match_type, span_lint, walk_ptrs_ty, get_parent_expr};
use utils::STRING_PATH;
-/// **What it does:** This lint matches code of the form `x = x + y` (without `let`!)
+/// **What it does:** This lint matches code of the form `x = x + y` (without `let`!). It is `Allow` by default.
///
/// **Why is this bad?** Because this expression needs another copy as opposed to `x.push_str(y)` (in practice LLVM will usually elide it, though). Despite [llogiq](https://github.com/llogiq)'s reservations, this lint also is `allow` by default, as some people opine that it's more readable.
///
if let Some(ref p) = parent {
if let ExprAssign(ref target, _) = p.node {
// avoid duplicate matches
- if is_exp_equal(cx, target, left) { return; }
+ if is_exp_equal(cx, target, left) {
+ return;
+ }
}
}
}
- span_lint(cx, STRING_ADD, e.span,
- "you added something to a string. \
- Consider using `String::push_str()` instead")
+ span_lint(cx,
+ STRING_ADD,
+ e.span,
+ "you added something to a string. Consider using `String::push_str()` instead");
}
} else if let ExprAssign(ref target, ref src) = e.node {
if is_string(cx, target) && is_add(cx, src, target) {
- span_lint(cx, STRING_ADD_ASSIGN, e.span,
- "you assigned the result of adding something to this string. \
- Consider using `String::push_str()` instead")
+ span_lint(cx,
+ STRING_ADD_ASSIGN,
+ e.span,
+ "you assigned the result of adding something to this string. Consider using \
+ `String::push_str()` instead");
}
}
}
fn is_add(cx: &LateContext, src: &Expr, target: &Expr) -> bool {
match src.node {
- ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) =>
- is_exp_equal(cx, target, left),
- ExprBlock(ref block) => block.stmts.is_empty() &&
- block.expr.as_ref().map_or(false,
- |expr| is_add(cx, expr, target)),
- _ => false
+ ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) => is_exp_equal(cx, target, left),
+ ExprBlock(ref block) => {
+ block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| is_add(cx, expr, target))
+ }
+ _ => false,
}
}
match target.node {
ExprField(ref base, _) | ExprTupField(ref base, _) => {
if is_temporary(base) && !is_adjusted(cx, base) {
- span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span,
- "assignment to temporary");
+ span_lint(cx, TEMPORARY_ASSIGNMENT, expr.span, "assignment to temporary");
}
}
- _ => ()
+ _ => (),
}
}
}
let to_ty = cx.tcx.expr_ty(e);
if from_ty == to_ty {
- cx.span_lint(USELESS_TRANSMUTE, e.span,
+ cx.span_lint(USELESS_TRANSMUTE,
+ e.span,
&format!("transmute from a type (`{}`) to itself", from_ty));
}
}
use syntax::ast::UintTy::*;
use syntax::ast::FloatTy::*;
-use utils::{match_type, snippet, span_lint, span_help_and_lint};
-use utils::{is_from_for_desugar, in_macro, in_external_macro};
-use utils::{LL_PATH, VEC_PATH};
+use utils::*;
/// Handles all the linting of funky types
#[allow(missing_copy_implementations)]
pub struct TypePass;
-/// **What it does:** This lint checks for use of `Box<Vec<_>>` anywhere in the code.
+/// **What it does:** This lint checks for use of `Box<Vec<_>>` anywhere in the code. It is `Warn` by default.
///
/// **Why is this bad?** `Vec` already keeps its contents in a separate area on the heap. So if you `Box` it, you just add another level of indirection without any benefit whatsoever.
///
/// **Example:** `struct X { values: Box<Vec<Foo>> }`
declare_lint!(pub BOX_VEC, Warn,
"usage of `Box<Vec<T>>`, vector elements are already on the heap");
-/// **What it does:** This lint checks for usage of any `LinkedList`, suggesting to use a `Vec` or a `VecDeque` (formerly called `RingBuf`).
+
+/// **What it does:** This lint checks for usage of any `LinkedList`, suggesting to use a `Vec` or a `VecDeque` (formerly called `RingBuf`). It is `Warn` by default.
///
/// **Why is this bad?** Gankro says:
///
impl LateLintPass for TypePass {
fn check_ty(&mut self, cx: &LateContext, ast_ty: &Ty) {
+ if in_macro(cx, ast_ty.span) {
+ return;
+ }
if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) {
if let ty::TyBox(ref inner) = ty.sty {
if match_type(cx, inner, &VEC_PATH) {
- span_help_and_lint(
- cx, BOX_VEC, ast_ty.span,
- "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
- "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.");
+ span_help_and_lint(cx,
+ BOX_VEC,
+ ast_ty.span,
+ "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
+ "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.");
}
- }
- else if match_type(cx, ty, &LL_PATH) {
- span_help_and_lint(
- cx, LINKEDLIST, ast_ty.span,
- "I see you're using a LinkedList! Perhaps you meant some other data structure?",
- "a VecDeque might work");
+ } else if match_type(cx, ty, &LL_PATH) {
+ span_help_and_lint(cx,
+ LINKEDLIST,
+ ast_ty.span,
+ "I see you're using a LinkedList! Perhaps you meant some other data structure?",
+ "a VecDeque might work");
}
}
}
if let DeclLocal(ref local) = decl.node {
let bindtype = &cx.tcx.pat_ty(&local.pat).sty;
if *bindtype == ty::TyTuple(vec![]) {
- if in_external_macro(cx, decl.span) ||
- in_macro(cx, local.pat.span) { return; }
- if is_from_for_desugar(decl) { return; }
- span_lint(cx, LET_UNIT_VALUE, decl.span, &format!(
- "this let-binding has unit value. Consider omitting `let {} =`",
- snippet(cx, local.pat.span, "..")));
+ if in_external_macro(cx, decl.span) || in_macro(cx, local.pat.span) {
+ return;
+ }
+ if is_from_for_desugar(decl) {
+ return;
+ }
+ span_lint(cx,
+ LET_UNIT_VALUE,
+ decl.span,
+ &format!("this let-binding has unit value. Consider omitting `let {} =`",
+ snippet(cx, local.pat.span, "..")));
}
}
}
impl LateLintPass for UnitCmp {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
- if in_macro(cx, expr.span) { return; }
+ if in_macro(cx, expr.span) {
+ return;
+ }
if let ExprBinary(ref cmp, ref left, _) = expr.node {
let op = cmp.node;
let sty = &cx.tcx.expr_ty(left).sty;
if *sty == ty::TyTuple(vec![]) && is_comparison_binop(op) {
let result = match op {
BiEq | BiLe | BiGe => "true",
- _ => "false"
+ _ => "false",
};
- span_lint(cx, UNIT_CMP, expr.span, &format!(
- "{}-comparison of unit values detected. This will always be {}",
- binop_to_string(op), result));
+ span_lint(cx,
+ UNIT_CMP,
+ expr.span,
+ &format!("{}-comparison of unit values detected. This will always be {}",
+ binop_to_string(op),
+ result));
}
}
}
/// Will return 0 if the type is not an int or uint variant
fn int_ty_to_nbits(typ: &ty::TyS) -> usize {
let n = match typ.sty {
- ty::TyInt(i) => 4 << (i as usize),
+ ty::TyInt(i) => 4 << (i as usize),
ty::TyUint(u) => 4 << (u as usize),
- _ => 0
+ _ => 0,
};
// n == 4 is the usize/isize case
- if n == 4 { ::std::usize::BITS } else { n }
+ if n == 4 {
+ ::std::usize::BITS
+ } else {
+ n
+ }
}
fn is_isize_or_usize(typ: &ty::TyS) -> bool {
match typ.sty {
ty::TyInt(TyIs) | ty::TyUint(TyUs) => true,
- _ => false
+ _ => false,
}
}
fn span_precision_loss_lint(cx: &LateContext, expr: &Expr, cast_from: &ty::TyS, cast_to_f64: bool) {
- let mantissa_nbits = if cast_to_f64 {52} else {23};
+ let mantissa_nbits = if cast_to_f64 {
+ 52
+ } else {
+ 23
+ };
let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64;
let arch_dependent_str = "on targets with 64-bit wide pointers ";
- let from_nbits_str = if arch_dependent {"64".to_owned()}
- else if is_isize_or_usize(cast_from) {"32 or 64".to_owned()}
- else {int_ty_to_nbits(cast_from).to_string()};
- span_lint(cx, CAST_PRECISION_LOSS, expr.span,
- &format!("casting {0} to {1} causes a loss of precision {2}\
- ({0} is {3} bits wide, but {1}'s mantissa is only {4} bits wide)",
- cast_from, if cast_to_f64 {"f64"} else {"f32"},
- if arch_dependent {arch_dependent_str} else {""},
- from_nbits_str, mantissa_nbits));
+ let from_nbits_str = if arch_dependent {
+ "64".to_owned()
+ } else if is_isize_or_usize(cast_from) {
+ "32 or 64".to_owned()
+ } else {
+ int_ty_to_nbits(cast_from).to_string()
+ };
+ span_lint(cx,
+ CAST_PRECISION_LOSS,
+ expr.span,
+ &format!("casting {0} to {1} causes a loss of precision {2}({0} is {3} bits wide, but {1}'s mantissa \
+ is only {4} bits wide)",
+ cast_from,
+ if cast_to_f64 {
+ "f64"
+ } else {
+ "f32"
+ },
+ if arch_dependent {
+ arch_dependent_str
+ } else {
+ ""
+ },
+ from_nbits_str,
+ mantissa_nbits));
}
enum ArchSuffix {
- _32, _64, None
+ _32,
+ _64,
+ None,
}
fn check_truncation_and_wrapping(cx: &LateContext, expr: &Expr, cast_from: &ty::TyS, cast_to: &ty::TyS) {
let arch_32_suffix = " on targets with 32-bit wide pointers";
let cast_unsigned_to_signed = !cast_from.is_signed() && cast_to.is_signed();
let (from_nbits, to_nbits) = (int_ty_to_nbits(cast_from), int_ty_to_nbits(cast_to));
- let (span_truncation, suffix_truncation, span_wrap, suffix_wrap) =
- match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
- (true, true) | (false, false) => (
- to_nbits < from_nbits,
- ArchSuffix::None,
- to_nbits == from_nbits && cast_unsigned_to_signed,
+ let (span_truncation, suffix_truncation, span_wrap, suffix_wrap) = match (is_isize_or_usize(cast_from),
+ is_isize_or_usize(cast_to)) {
+ (true, true) | (false, false) => {
+ (to_nbits < from_nbits,
+ ArchSuffix::None,
+ to_nbits == from_nbits && cast_unsigned_to_signed,
+ ArchSuffix::None)
+ }
+ (true, false) => {
+ (to_nbits <= 32,
+ if to_nbits == 32 {
+ ArchSuffix::_64
+ } else {
ArchSuffix::None
- ),
- (true, false) => (
- to_nbits <= 32,
- if to_nbits == 32 {ArchSuffix::_64} else {ArchSuffix::None},
- to_nbits <= 32 && cast_unsigned_to_signed,
+ },
+ to_nbits <= 32 && cast_unsigned_to_signed,
+ ArchSuffix::_32)
+ }
+ (false, true) => {
+ (from_nbits == 64,
+ ArchSuffix::_32,
+ cast_unsigned_to_signed,
+ if from_nbits == 64 {
+ ArchSuffix::_64
+ } else {
ArchSuffix::_32
- ),
- (false, true) => (
- from_nbits == 64,
- ArchSuffix::_32,
- cast_unsigned_to_signed,
- if from_nbits == 64 {ArchSuffix::_64} else {ArchSuffix::_32}
- ),
- };
+ })
+ }
+ };
if span_truncation {
- span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span,
- &format!("casting {} to {} may truncate the value{}",
- cast_from, cast_to,
- match suffix_truncation {
- ArchSuffix::_32 => arch_32_suffix,
- ArchSuffix::_64 => arch_64_suffix,
- ArchSuffix::None => "" }));
+ span_lint(cx,
+ CAST_POSSIBLE_TRUNCATION,
+ expr.span,
+ &format!("casting {} to {} may truncate the value{}",
+ cast_from,
+ cast_to,
+ match suffix_truncation {
+ ArchSuffix::_32 => arch_32_suffix,
+ ArchSuffix::_64 => arch_64_suffix,
+ ArchSuffix::None => "",
+ }));
}
if span_wrap {
- span_lint(cx, CAST_POSSIBLE_WRAP, expr.span,
- &format!("casting {} to {} may wrap around the value{}",
- cast_from, cast_to,
- match suffix_wrap {
- ArchSuffix::_32 => arch_32_suffix,
- ArchSuffix::_64 => arch_64_suffix,
- ArchSuffix::None => "" }));
+ span_lint(cx,
+ CAST_POSSIBLE_WRAP,
+ expr.span,
+ &format!("casting {} to {} may wrap around the value{}",
+ cast_from,
+ cast_to,
+ match suffix_wrap {
+ ArchSuffix::_32 => arch_32_suffix,
+ ArchSuffix::_64 => arch_64_suffix,
+ ArchSuffix::None => "",
+ }));
}
}
match (cast_from.is_integral(), cast_to.is_integral()) {
(true, false) => {
let from_nbits = int_ty_to_nbits(cast_from);
- let to_nbits = if let ty::TyFloat(TyF32) = cast_to.sty {32} else {64};
+ let to_nbits = if let ty::TyFloat(TyF32) = cast_to.sty {
+ 32
+ } else {
+ 64
+ };
if is_isize_or_usize(cast_from) || from_nbits >= to_nbits {
span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64);
}
}
(false, true) => {
- span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span,
- &format!("casting {} to {} may truncate the value",
- cast_from, cast_to));
+ span_lint(cx,
+ CAST_POSSIBLE_TRUNCATION,
+ expr.span,
+ &format!("casting {} to {} may truncate the value", cast_from, cast_to));
if !cast_to.is_signed() {
- span_lint(cx, CAST_SIGN_LOSS, expr.span,
- &format!("casting {} to {} may lose the sign of the value",
- cast_from, cast_to));
+ span_lint(cx,
+ CAST_SIGN_LOSS,
+ expr.span,
+ &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to));
}
}
(true, true) => {
if cast_from.is_signed() && !cast_to.is_signed() {
- span_lint(cx, CAST_SIGN_LOSS, expr.span,
- &format!("casting {} to {} may lose the sign of the value",
- cast_from, cast_to));
+ span_lint(cx,
+ CAST_SIGN_LOSS,
+ expr.span,
+ &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to));
}
check_truncation_and_wrapping(cx, expr, cast_from, cast_to);
}
(false, false) => {
- if let (&ty::TyFloat(TyF64),
- &ty::TyFloat(TyF32)) = (&cast_from.sty, &cast_to.sty) {
- span_lint(cx, CAST_POSSIBLE_TRUNCATION,
- expr.span,
- "casting f64 to f32 may truncate the value");
+ if let (&ty::TyFloat(TyF64), &ty::TyFloat(TyF32)) = (&cast_from.sty, &cast_to.sty) {
+ span_lint(cx,
+ CAST_POSSIBLE_TRUNCATION,
+ expr.span,
+ "casting f64 to f32 may truncate the value");
}
}
}
ItemStatic(ref ty, _, _) |
ItemConst(ref ty, _) => check_type(cx, ty),
// functions, enums, structs, impls and traits are covered
- _ => ()
+ _ => (),
}
}
TypeTraitItem(_, Some(ref ty)) => check_type(cx, ty),
MethodTraitItem(MethodSig { ref decl, .. }, None) => check_fndecl(cx, decl),
// methods with default impl are covered by check_fn
- _ => ()
+ _ => (),
}
}
ImplItemKind::Const(ref ty, _) |
ImplItemKind::Type(ref ty) => check_type(cx, ty),
// methods are covered by check_fn
- _ => ()
+ _ => (),
}
}
}
fn check_type(cx: &LateContext, ty: &Ty) {
- if in_macro(cx, ty.span) { return; }
+ if in_macro(cx, ty.span) {
+ return;
+ }
let score = {
- let mut visitor = TypeComplexityVisitor { score: 0, nest: 1 };
+ let mut visitor = TypeComplexityVisitor {
+ score: 0,
+ nest: 1,
+ };
visitor.visit_ty(ty);
visitor.score
};
// println!("{:?} --> {}", ty, score);
if score > 250 {
- span_lint(cx, TYPE_COMPLEXITY, ty.span, &format!(
- "very complex type used. Consider factoring parts into `type` definitions"));
+ span_lint(cx,
+ TYPE_COMPLEXITY,
+ ty.span,
+ &format!("very complex type used. Consider factoring parts into `type` definitions"));
}
}
TyBareFn(..) |
TyPolyTraitRef(..) => (50 * self.nest, 1),
- _ => (0, 0)
+ _ => (0, 0),
};
self.score += add_score;
self.nest += sub_nest;
}
}
-fn escape<T: Iterator<Item=char>>(s: T) -> String {
+fn escape<T: Iterator<Item = char>>(s: T) -> String {
let mut result = String::new();
for c in s {
if c as u32 > 0x7F {
- for d in c.escape_unicode() { result.push(d) };
+ for d in c.escape_unicode() {
+ result.push(d)
+ }
} else {
result.push(c);
}
fn check_str(cx: &LateContext, span: Span) {
let string = snippet(cx, span, "");
if string.contains('\u{200B}') {
- span_help_and_lint(cx, ZERO_WIDTH_SPACE, span,
- "zero-width space detected",
- &format!("Consider replacing the string with:\n\"{}\"",
- string.replace("\u{200B}", "\\u{200B}")));
+ span_help_and_lint(cx,
+ ZERO_WIDTH_SPACE,
+ span,
+ "zero-width space detected",
+ &format!("Consider replacing the string with:\n\"{}\"",
+ string.replace("\u{200B}", "\\u{200B}")));
}
if string.chars().any(|c| c as u32 > 0x7F) {
- span_help_and_lint(cx, NON_ASCII_LITERAL, span,
- "literal non-ASCII character detected",
- &format!("Consider replacing the string with:\n\"{}\"",
- if cx.current_level(UNICODE_NOT_NFC) == Level::Allow {
- escape(string.chars())
- } else {
- escape(string.nfc())
- }));
+ span_help_and_lint(cx,
+ NON_ASCII_LITERAL,
+ span,
+ "literal non-ASCII character detected",
+ &format!("Consider replacing the string with:\n\"{}\"",
+ if cx.current_level(UNICODE_NOT_NFC) == Level::Allow {
+ escape(string.chars())
+ } else {
+ escape(string.nfc())
+ }));
}
- if cx.current_level(NON_ASCII_LITERAL) == Level::Allow &&
- string.chars().zip(string.nfc()).any(|(a, b)| a != b) {
- span_help_and_lint(cx, UNICODE_NOT_NFC, span,
- "non-nfc unicode sequence detected",
- &format!("Consider replacing the string with:\n\"{}\"",
- string.nfc().collect::<String>()));
+ if cx.current_level(NON_ASCII_LITERAL) == Level::Allow && string.chars().zip(string.nfc()).any(|(a, b)| a != b) {
+ span_help_and_lint(cx,
+ UNICODE_NOT_NFC,
+ span,
+ "non-nfc unicode sequence detected",
+ &format!("Consider replacing the string with:\n\"{}\"", string.nfc().collect::<String>()));
}
}
use std::borrow::Cow;
use syntax::ast::Lit_::*;
use syntax::ast;
+use syntax::errors::DiagnosticBuilder;
use syntax::ptr::P;
+use consts::constant;
use rustc::session::Session;
use std::str::FromStr;
+use std::ops::{Deref, DerefMut};
pub type MethodArgs = HirVec<P<Expr>>;
// module DefPaths for certain structs/enums we check for
+pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"];
+pub const BTREEMAP_PATH: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"];
+pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"];
+pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"];
+pub const HASHMAP_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"];
+pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
+pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
+pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"];
pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"];
pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"];
pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"];
-pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"];
-pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
-pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"];
-pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
-pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"];
-pub const BEGIN_UNWIND:[&'static str; 3] = ["std", "rt", "begin_unwind"];
+pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"];
/// Produce a nested chain of if-lets and ifs from the patterns:
///
};
}
-/// returns true if this expn_info was expanded by any macro
+/// Returns true if the two spans come from differing expansions (i.e. one is from a macro and one
+/// isn't).
+pub fn differing_macro_contexts(sp1: Span, sp2: Span) -> bool {
+ sp1.expn_id != sp2.expn_id
+}
+/// Returns true if this `expn_info` was expanded by any macro.
pub fn in_macro<T: LintContext>(cx: &T, span: Span) -> bool {
- cx.sess().codemap().with_expn_info(span.expn_id,
- |info| info.is_some())
+ cx.sess().codemap().with_expn_info(span.expn_id, |info| info.is_some())
}
-/// returns true if the macro that expanded the crate was outside of
-/// the current crate or was a compiler plugin
+/// Returns true if the macro that expanded the crate was outside of the current crate or was a
+/// compiler plugin.
pub fn in_external_macro<T: LintContext>(cx: &T, span: Span) -> bool {
- /// invokes in_macro with the expansion info of the given span
- /// slightly heavy, try to use this after other checks have already happened
+ /// Invokes in_macro with the expansion info of the given span slightly heavy, try to use this
+ /// after other checks have already happened.
fn in_macro_ext<T: LintContext>(cx: &T, opt_info: Option<&ExpnInfo>) -> bool {
// no ExpnInfo = no macro
opt_info.map_or(false, |info| {
if let ExpnFormat::MacroAttribute(..) = info.callee.format {
- // these are all plugins
- return true;
+ // these are all plugins
+ return true;
}
// no span for the callee = external macro
info.callee.span.map_or(true, |span| {
// no snippet = external macro or compiler-builtin expansion
- cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code|
- // macro doesn't start with "macro_rules"
- // = compiler plugin
- !code.starts_with("macro_rules")
- )
+ cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| !code.starts_with("macro_rules"))
})
})
}
- cx.sess().codemap().with_expn_info(span.expn_id,
- |info| in_macro_ext(cx, info))
+ cx.sess().codemap().with_expn_info(span.expn_id, |info| in_macro_ext(cx, info))
}
-/// check if a DefId's path matches the given absolute type path
-/// usage e.g. with
-/// `match_def_path(cx, id, &["core", "option", "Option"])`
+/// Check if a `DefId`'s path matches the given absolute type path usage.
+///
+/// # Examples
+/// ```
+/// match_def_path(cx, id, &["core", "option", "Option"])
+/// ```
pub fn match_def_path(cx: &LateContext, def_id: DefId, path: &[&str]) -> bool {
- cx.tcx.with_path(def_id, |iter| iter.zip(path)
- .all(|(nm, p)| nm.name().as_str() == *p))
+ cx.tcx.with_path(def_id, |iter| {
+ iter.zip(path)
+ .all(|(nm, p)| nm.name().as_str() == *p)
+ })
}
-/// check if type is struct or enum type with given def path
+/// Check if type is struct or enum type with given def path.
pub fn match_type(cx: &LateContext, ty: ty::Ty, path: &[&str]) -> bool {
match ty.sty {
- ty::TyEnum(ref adt, _) | ty::TyStruct(ref adt, _) => {
- match_def_path(cx, adt.did, path)
- }
- _ => {
- false
- }
+ ty::TyEnum(ref adt, _) | ty::TyStruct(ref adt, _) => match_def_path(cx, adt.did, path),
+ _ => false,
}
}
-/// check if method call given in "expr" belongs to given trait
+/// Check if the method call given in `expr` belongs to given trait.
pub fn match_impl_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool {
let method_call = ty::MethodCall::expr(expr.id);
- let trt_id = cx.tcx.tables
- .borrow().method_map.get(&method_call)
- .and_then(|callee| cx.tcx.impl_of_method(callee.def_id));
+ let trt_id = cx.tcx
+ .tables
+ .borrow()
+ .method_map
+ .get(&method_call)
+ .and_then(|callee| cx.tcx.impl_of_method(callee.def_id));
if let Some(trt_id) = trt_id {
match_def_path(cx, trt_id, path)
} else {
}
}
-/// check if method call given in "expr" belongs to given trait
+/// Check if the method call given in `expr` belongs to given trait.
pub fn match_trait_method(cx: &LateContext, expr: &Expr, path: &[&str]) -> bool {
let method_call = ty::MethodCall::expr(expr.id);
- let trt_id = cx.tcx.tables
- .borrow().method_map.get(&method_call)
- .and_then(|callee| cx.tcx.trait_of_item(callee.def_id));
+
+ let trt_id = cx.tcx
+ .tables
+ .borrow()
+ .method_map
+ .get(&method_call)
+ .and_then(|callee| cx.tcx.trait_of_item(callee.def_id));
if let Some(trt_id) = trt_id {
match_def_path(cx, trt_id, path)
} else {
}
}
-/// match a Path against a slice of segment string literals, e.g.
-/// `match_path(path, &["std", "rt", "begin_unwind"])`
+/// Match a `Path` against a slice of segment string literals.
+///
+/// # Examples
+/// ```
+/// match_path(path, &["std", "rt", "begin_unwind"])
+/// ```
pub fn match_path(path: &Path, segments: &[&str]) -> bool {
- path.segments.iter().rev().zip(segments.iter().rev()).all(
- |(a, b)| a.identifier.name.as_str() == *b)
+ path.segments.iter().rev().zip(segments.iter().rev()).all(|(a, b)| a.identifier.name.as_str() == *b)
}
-/// match a Path against a slice of segment string literals, e.g.
-/// `match_path(path, &["std", "rt", "begin_unwind"])`
+/// Match a `Path` against a slice of segment string literals, e.g.
+///
+/// # Examples
+/// ```
+/// match_path(path, &["std", "rt", "begin_unwind"])
+/// ```
pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool {
- path.segments.iter().rev().zip(segments.iter().rev()).all(
- |(a, b)| a.identifier.name.as_str() == *b)
+ path.segments.iter().rev().zip(segments.iter().rev()).all(|(a, b)| a.identifier.name.as_str() == *b)
}
-/// match an Expr against a chain of methods, and return the matched Exprs. For example, if `expr`
-/// represents the `.baz()` in `foo.bar().baz()`, `matched_method_chain(expr, &["bar", "baz"])`
-/// will return a Vec containing the Exprs for `.bar()` and `.baz()`
+/// Match an `Expr` against a chain of methods, and return the matched `Expr`s.
+///
+/// For example, if `expr` represents the `.baz()` in `foo.bar().baz()`,
+/// `matched_method_chain(expr, &["bar", "baz"])` will return a `Vec` containing the `Expr`s for
+/// `.bar()` and `.baz()`
pub fn method_chain_args<'a>(expr: &'a Expr, methods: &[&str]) -> Option<Vec<&'a MethodArgs>> {
let mut current = expr;
let mut matched = Vec::with_capacity(methods.len());
- for method_name in methods.iter().rev() { // method chains are stored last -> first
+ for method_name in methods.iter().rev() {
+ // method chains are stored last -> first
if let ExprMethodCall(ref name, _, ref args) = current.node {
if name.node.as_str() == *method_name {
matched.push(args); // build up `matched` backwards
current = &args[0] // go to parent expression
- }
- else {
+ } else {
return None;
}
- }
- else {
+ } else {
return None;
}
}
}
-/// get the name of the item the expression is in, if available
+/// Get the name of the item the expression is in, if available.
pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option<Name> {
let parent_id = cx.tcx.map.get_parent(expr.id);
match cx.tcx.map.find(parent_id) {
Some(NodeItem(&Item{ ref name, .. })) |
Some(NodeTraitItem(&TraitItem{ ref name, .. })) |
- Some(NodeImplItem(&ImplItem{ ref name, .. })) => {
- Some(*name)
- }
+ Some(NodeImplItem(&ImplItem{ ref name, .. })) => Some(*name),
_ => None,
}
}
-/// checks if a `let` decl is from a for loop desugaring
+/// Checks if a `let` decl is from a `for` loop desugaring.
pub fn is_from_for_desugar(decl: &Decl) -> bool {
if_let_chain! {
[
}
-/// convert a span to a code snippet if available, otherwise use default, e.g.
-/// `snippet(cx, expr.span, "..")`
+/// Convert a span to a code snippet if available, otherwise use default.
+///
+/// # Example
+/// ```
+/// snippet(cx, expr.span, "..")
+/// ```
pub fn snippet<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow<'a, str> {
cx.sess().codemap().span_to_snippet(span).map(From::from).unwrap_or(Cow::Borrowed(default))
}
-/// Converts a span to a code snippet. Returns None if not available.
+/// Convert a span to a code snippet. Returns `None` if not available.
pub fn snippet_opt<T: LintContext>(cx: &T, span: Span) -> Option<String> {
cx.sess().codemap().span_to_snippet(span).ok()
}
-/// convert a span (from a block) to a code snippet if available, otherwise use default, e.g.
-/// `snippet(cx, expr.span, "..")`
-/// This trims the code of indentation, except for the first line
-/// Use it for blocks or block-like things which need to be printed as such
+/// Convert a span (from a block) to a code snippet if available, otherwise use default.
+/// This trims the code of indentation, except for the first line. Use it for blocks or block-like
+/// things which need to be printed as such.
+///
+/// # Example
+/// ```
+/// snippet(cx, expr.span, "..")
+/// ```
pub fn snippet_block<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow<'a, str> {
let snip = snippet(cx, span, default);
trim_multiline(snip, true)
}
-/// Like snippet_block, but add braces if the expr is not an ExprBlock
-/// Also takes an Option<String> which can be put inside the braces
-pub fn expr_block<'a, T: LintContext>(cx: &T, expr: &Expr,
- option: Option<String>,
- default: &'a str) -> Cow<'a, str> {
+/// Like `snippet_block`, but add braces if the expr is not an `ExprBlock`.
+/// Also takes an `Option<String>` which can be put inside the braces.
+pub fn expr_block<'a, T: LintContext>(cx: &T, expr: &Expr, option: Option<String>, default: &'a str) -> Cow<'a, str> {
let code = snippet_block(cx, expr.span, default);
- let string = option.map_or("".to_owned(), |s| s);
+ let string = option.unwrap_or_default();
if let ExprBlock(_) = expr.node {
Cow::Owned(format!("{}{}", code, string))
} else if string.is_empty() {
}
}
-/// Trim indentation from a multiline string
-/// with possibility of ignoring the first line
+/// Trim indentation from a multiline string with possibility of ignoring the first line.
pub fn trim_multiline(s: Cow<str>, ignore_first: bool) -> Cow<str> {
let s_space = trim_multiline_inner(s, ignore_first, ' ');
let s_tab = trim_multiline_inner(s_space, ignore_first, '\t');
}
fn trim_multiline_inner(s: Cow<str>, ignore_first: bool, ch: char) -> Cow<str> {
- let x = s.lines().skip(ignore_first as usize)
- .filter_map(|l| { if l.len() > 0 { // ignore empty lines
- Some(l.char_indices()
- .find(|&(_,x)| x != ch)
- .unwrap_or((l.len(), ch)).0)
- } else {None}})
- .min().unwrap_or(0);
+ let x = s.lines()
+ .skip(ignore_first as usize)
+ .filter_map(|l| {
+ if l.len() > 0 {
+ // ignore empty lines
+ Some(l.char_indices()
+ .find(|&(_, x)| x != ch)
+ .unwrap_or((l.len(), ch))
+ .0)
+ } else {
+ None
+ }
+ })
+ .min()
+ .unwrap_or(0);
if x > 0 {
- Cow::Owned(s.lines().enumerate().map(|(i,l)| if (ignore_first && i == 0) ||
- l.len() == 0 {
- l
- } else {
- l.split_at(x).1
- }).collect::<Vec<_>>()
- .join("\n"))
+ Cow::Owned(s.lines()
+ .enumerate()
+ .map(|(i, l)| {
+ if (ignore_first && i == 0) || l.len() == 0 {
+ l
+ } else {
+ l.split_at(x).1
+ }
+ })
+ .collect::<Vec<_>>()
+ .join("\n"))
} else {
s
}
}
-/// get a parent expr if any – this is useful to constrain a lint
+/// Get a parent expressions if any – this is useful to constrain a lint.
pub fn get_parent_expr<'c>(cx: &'c LateContext, e: &Expr) -> Option<&'c Expr> {
let map = &cx.tcx.map;
- let node_id : NodeId = e.id;
- let parent_id : NodeId = map.get_parent_node(node_id);
- if node_id == parent_id { return None; }
- map.find(parent_id).and_then(|node|
- if let NodeExpr(parent) = node { Some(parent) } else { None } )
+ let node_id: NodeId = e.id;
+ let parent_id: NodeId = map.get_parent_node(node_id);
+ if node_id == parent_id {
+ return None;
+ }
+ map.find(parent_id).and_then(|node| {
+ if let NodeExpr(parent) = node {
+ Some(parent)
+ } else {
+ None
+ }
+ })
}
pub fn get_enclosing_block<'c>(cx: &'c LateContext, node: NodeId) -> Option<&'c Block> {
match node {
NodeBlock(ref block) => Some(block),
NodeItem(&Item{ node: ItemFn(_, _, _, _, _, ref block), .. }) => Some(block),
- _ => None
+ _ => None,
}
- } else { None }
+ } else {
+ None
+ }
}
-#[cfg(not(feature="structured_logging"))]
-pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: Span, msg: &str) {
- cx.span_lint(lint, sp, msg);
- if cx.current_level(lint) != Level::Allow {
- cx.sess().fileline_help(sp, &format!("for further information visit \
- https://github.com/Manishearth/rust-clippy/wiki#{}",
- lint.name_lower()))
+pub struct DiagnosticWrapper<'a>(pub DiagnosticBuilder<'a>);
+
+impl<'a> Drop for DiagnosticWrapper<'a> {
+ fn drop(&mut self) {
+ self.0.emit();
+ }
+}
+
+impl<'a> DerefMut for DiagnosticWrapper<'a> {
+ fn deref_mut(&mut self) -> &mut DiagnosticBuilder<'a> {
+ &mut self.0
+ }
+}
+
+impl<'a> Deref for DiagnosticWrapper<'a> {
+ type Target = DiagnosticBuilder<'a>;
+ fn deref(&self) -> &DiagnosticBuilder<'a> {
+ &self.0
}
}
-#[cfg(feature="structured_logging")]
-pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: Span, msg: &str) {
- // lint.name / lint.desc is can give details of the lint
- // cx.sess().codemap() has all these nice functions for line/column/snippet details
- // http://doc.rust-lang.org/syntax/codemap/struct.CodeMap.html#method.span_to_string
- cx.span_lint(lint, sp, msg);
+pub fn span_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str) -> DiagnosticWrapper<'a> {
+ let mut db = cx.struct_span_lint(lint, sp, msg);
if cx.current_level(lint) != Level::Allow {
- cx.sess().fileline_help(sp, &format!("for further information visit \
- https://github.com/Manishearth/rust-clippy/wiki#{}",
- lint.name_lower()))
+ db.fileline_help(sp,
+ &format!("for further information visit https://github.com/Manishearth/rust-clippy/wiki#{}",
+ lint.name_lower()));
}
+ DiagnosticWrapper(db)
}
-pub fn span_help_and_lint<T: LintContext>(cx: &T, lint: &'static Lint, span: Span,
- msg: &str, help: &str) {
- cx.span_lint(lint, span, msg);
+pub fn span_help_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span, msg: &str, help: &str)
+ -> DiagnosticWrapper<'a> {
+ let mut db = cx.struct_span_lint(lint, span, msg);
if cx.current_level(lint) != Level::Allow {
- cx.sess().fileline_help(span, &format!("{}\nfor further information \
- visit https://github.com/Manishearth/rust-clippy/wiki#{}",
- help, lint.name_lower()))
+ db.fileline_help(span,
+ &format!("{}\nfor further information visit \
+ https://github.com/Manishearth/rust-clippy/wiki#{}",
+ help,
+ lint.name_lower()));
}
+ DiagnosticWrapper(db)
}
-pub fn span_note_and_lint<T: LintContext>(cx: &T, lint: &'static Lint, span: Span,
- msg: &str, note_span: Span, note: &str) {
- cx.span_lint(lint, span, msg);
+pub fn span_note_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, span: Span, msg: &str, note_span: Span,
+ note: &str)
+ -> DiagnosticWrapper<'a> {
+ let mut db = cx.struct_span_lint(lint, span, msg);
if cx.current_level(lint) != Level::Allow {
if note_span == span {
- cx.sess().fileline_note(note_span, note)
+ db.fileline_note(note_span, note);
} else {
- cx.sess().span_note(note_span, note)
+ db.span_note(note_span, note);
}
- cx.sess().fileline_help(span, &format!("for further information visit \
- https://github.com/Manishearth/rust-clippy/wiki#{}",
- lint.name_lower()))
+ db.fileline_help(span,
+ &format!("for further information visit https://github.com/Manishearth/rust-clippy/wiki#{}",
+ lint.name_lower()));
}
+ DiagnosticWrapper(db)
}
-pub fn span_lint_and_then<T: LintContext, F>(cx: &T, lint: &'static Lint, sp: Span,
- msg: &str, f: F) where F: Fn() {
- cx.span_lint(lint, sp, msg);
+pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F)
+ -> DiagnosticWrapper<'a>
+ where F: Fn(&mut DiagnosticWrapper)
+{
+ let mut db = DiagnosticWrapper(cx.struct_span_lint(lint, sp, msg));
if cx.current_level(lint) != Level::Allow {
- f();
- cx.sess().fileline_help(sp, &format!("for further information visit \
- https://github.com/Manishearth/rust-clippy/wiki#{}",
- lint.name_lower()))
+ f(&mut db);
+ db.fileline_help(sp,
+ &format!("for further information visit https://github.com/Manishearth/rust-clippy/wiki#{}",
+ lint.name_lower()));
}
+ db
}
-/// return the base type for references and raw pointers
+/// Return the base type for references and raw pointers.
pub fn walk_ptrs_ty(ty: ty::Ty) -> ty::Ty {
match ty.sty {
ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => walk_ptrs_ty(tm.ty),
- _ => ty
+ _ => ty,
}
}
-/// return the base type for references and raw pointers, and count reference depth
+/// Return the base type for references and raw pointers, and count reference depth.
pub fn walk_ptrs_ty_depth(ty: ty::Ty) -> (ty::Ty, usize) {
fn inner(ty: ty::Ty, depth: usize) -> (ty::Ty, usize) {
match ty.sty {
ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => inner(tm.ty, depth + 1),
- _ => (ty, depth)
+ _ => (ty, depth),
}
}
inner(ty, 0)
}
-pub fn is_integer_literal(expr: &Expr, value: u64) -> bool
-{
+/// Check whether the given expression is a constant literal of the given value.
+pub fn is_integer_literal(expr: &Expr, value: u64) -> bool {
// FIXME: use constant folding
if let ExprLit(ref spanned) = expr.node {
if let LitInt(v, _) = spanned.node {
impl LimitStack {
pub fn new(limit: u64) -> LimitStack {
- LimitStack {
- stack: vec![limit],
- }
+ LimitStack { stack: vec![limit] }
}
pub fn limit(&self) -> u64 {
*self.stack.last().expect("there should always be a value in the stack")
}
pub fn push_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) {
let stack = &mut self.stack;
- parse_attrs(
- sess,
- attrs,
- name,
- |val| stack.push(val),
- );
+ parse_attrs(sess, attrs, name, |val| stack.push(val));
}
pub fn pop_attrs(&mut self, sess: &Session, attrs: &[ast::Attribute], name: &'static str) {
let stack = &mut self.stack;
- parse_attrs(
- sess,
- attrs,
- name,
- |val| assert_eq!(stack.pop(), Some(val)),
- );
+ parse_attrs(sess, attrs, name, |val| assert_eq!(stack.pop(), Some(val)));
}
}
fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'static str, mut f: F) {
for attr in attrs {
let attr = &attr.node;
- if attr.is_sugared_doc { continue; }
+ if attr.is_sugared_doc {
+ continue;
+ }
if let ast::MetaNameValue(ref key, ref value) = attr.value.node {
if *key == name {
if let LitStr(ref s, _) = value.node {
}
}
}
+
+pub fn is_exp_equal(cx: &LateContext, left: &Expr, right: &Expr) -> bool {
+ if let (Some(l), Some(r)) = (constant(cx, left), constant(cx, right)) {
+ if l == r {
+ return true;
+ }
+ }
+ match (&left.node, &right.node) {
+ (&ExprField(ref lfexp, ref lfident), &ExprField(ref rfexp, ref rfident)) => {
+ lfident.node == rfident.node && is_exp_equal(cx, lfexp, rfexp)
+ }
+ (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node,
+ (&ExprPath(ref lqself, ref lsubpath), &ExprPath(ref rqself, ref rsubpath)) => {
+ both(lqself, rqself, is_qself_equal) && is_path_equal(lsubpath, rsubpath)
+ }
+ (&ExprTup(ref ltup), &ExprTup(ref rtup)) => is_exps_equal(cx, ltup, rtup),
+ (&ExprVec(ref l), &ExprVec(ref r)) => is_exps_equal(cx, l, r),
+ (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => is_exp_equal(cx, lx, rx) && is_cast_ty_equal(lt, rt),
+ _ => false,
+ }
+}
+
+fn is_exps_equal(cx: &LateContext, left: &[P<Expr>], right: &[P<Expr>]) -> bool {
+ over(left, right, |l, r| is_exp_equal(cx, l, r))
+}
+
+fn is_path_equal(left: &Path, right: &Path) -> bool {
+ // The == of idents doesn't work with different contexts,
+ // we have to be explicit about hygiene
+ left.global == right.global &&
+ over(&left.segments,
+ &right.segments,
+ |l, r| l.identifier.name == r.identifier.name && l.parameters == r.parameters)
+}
+
+fn is_qself_equal(left: &QSelf, right: &QSelf) -> bool {
+ left.ty.node == right.ty.node && left.position == right.position
+}
+
+fn over<X, F>(left: &[X], right: &[X], mut eq_fn: F) -> bool
+ where F: FnMut(&X, &X) -> bool
+{
+ left.len() == right.len() && left.iter().zip(right).all(|(x, y)| eq_fn(x, y))
+}
+
+fn both<X, F>(l: &Option<X>, r: &Option<X>, mut eq_fn: F) -> bool
+ where F: FnMut(&X, &X) -> bool
+{
+ l.as_ref().map_or_else(|| r.is_none(), |x| r.as_ref().map_or(false, |y| eq_fn(x, y)))
+}
+
+fn is_cast_ty_equal(left: &Ty, right: &Ty) -> bool {
+ match (&left.node, &right.node) {
+ (&TyVec(ref lvec), &TyVec(ref rvec)) => is_cast_ty_equal(lvec, rvec),
+ (&TyPtr(ref lmut), &TyPtr(ref rmut)) => lmut.mutbl == rmut.mutbl && is_cast_ty_equal(&*lmut.ty, &*rmut.ty),
+ (&TyRptr(_, ref lrmut), &TyRptr(_, ref rrmut)) => {
+ lrmut.mutbl == rrmut.mutbl && is_cast_ty_equal(&*lrmut.ty, &*rrmut.ty)
+ }
+ (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) => {
+ both(lq, rq, is_qself_equal) && is_path_equal(lpath, rpath)
+ }
+ (&TyInfer, &TyInfer) => true,
+ _ => false,
+ }
+}
/// 0.0/0.0 with std::f32::NaN or std::f64::NaN, depending on the precision.
pub struct ZeroDivZeroPass;
-/// **What it does:** This lint checks for `0.0 / 0.0`
+/// **What it does:** This lint checks for `0.0 / 0.0`. It is `Warn` by default.
///
/// **Why is this bad?** It's less readable than `std::f32::NAN` or `std::f64::NAN`
///
-#![feature(plugin)]
+#![feature(plugin, deprecated)]
#![plugin(clippy)]
-#![deny(inline_always)]
+#![deny(inline_always, deprecated_semver)]
#[inline(always)] //~ERROR you have declared `#[inline(always)]` on `test_attr_lint`.
fn test_attr_lint() {
unreachable!();
}
+#[deprecated(since = "forever")] //~ERROR the since field must contain a semver-compliant version
+pub const SOME_CONST : u8 = 42;
+
+#[deprecated(since = "1")] //~ERROR the since field must contain a semver-compliant version
+pub const ANOTHER_CONST : u8 = 23;
+
+#[deprecated(since = "0.1.1")]
+pub const YET_ANOTHER_CONST : u8 = 0;
fn main() {
test_attr_lint();
#![deny(block_in_if_condition_stmt)]
#![allow(unused)]
+
+macro_rules! blocky {
+ () => {{true}}
+}
+
+fn macro_if() {
+ if blocky!() {
+ }
+}
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'
#![deny(clippy)]
#![allow(boxed_local)]
+macro_rules! boxit {
+ ($init:expr, $x:ty) => {
+ let _: Box<$x> = Box::new($init);
+ }
+}
+
+fn test_macro() {
+ boxit!(Vec::new(), Vec<u8>);
+}
pub fn test(foo: Box<Vec<bool>>) { //~ ERROR you seem to be trying to use `Box<Vec<T>>`
println!("{:?}", foo.get(0))
}
fn main(){
test(Box::new(Vec::new()));
test2(Box::new(|v| println!("{:?}", v)));
+ test_macro();
}
#![deny(cyclomatic_complexity)]
#![allow(unused)]
-fn main() { //~ ERROR: The function has a cyclomatic complexity of 28.
+
+fn main() { //~ERROR The function has a cyclomatic complexity of 28
if true {
println!("a");
}
--- /dev/null
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(duplicate_underscore_argument)]
+#[allow(dead_code, unused)]
+
+fn join_the_dark_side(darth: i32, _darth: i32) {} //~ERROR `darth` already exists
+fn join_the_light_side(knight: i32, _master: i32) {} // the Force is strong with this one
+
+fn main() {
+ join_the_dark_side(0, 0);
+ join_the_light_side(0, 0);
+}
\ No newline at end of file
--- /dev/null
+#![feature(plugin)]
+#![plugin(clippy)]
+#![allow(unused)]
+
+#![deny(map_entry)]
+
+use std::collections::{BTreeMap, HashMap};
+use std::hash::Hash;
+
+fn foo() {}
+
+fn insert_if_absent0<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
+ if !m.contains_key(&k) { m.insert(k, v); }
+ //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
+ //~| HELP Consider
+ //~| SUGGESTION m.entry(k).or_insert(v)
+}
+
+fn insert_if_absent1<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
+ if !m.contains_key(&k) { foo(); m.insert(k, v); }
+ //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
+ //~| HELP Consider
+ //~| SUGGESTION m.entry(k)
+}
+
+fn insert_if_absent2<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
+ if !m.contains_key(&k) { m.insert(k, v) } else { None };
+ //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
+ //~| HELP Consider
+ //~| SUGGESTION m.entry(k).or_insert(v)
+}
+
+fn insert_if_absent3<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
+ if !m.contains_key(&k) { foo(); m.insert(k, v) } else { None };
+ //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
+ //~| HELP Consider
+ //~| SUGGESTION m.entry(k)
+}
+
+fn insert_in_btreemap<K: Ord, V>(m: &mut BTreeMap<K, V>, k: K, v: V) {
+ if !m.contains_key(&k) { foo(); m.insert(k, v) } else { None };
+ //~^ ERROR usage of `contains_key` followed by `insert` on `BTreeMap`
+ //~| HELP Consider
+ //~| SUGGESTION m.entry(k)
+}
+
+fn insert_other_if_absent<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, o: K, v: V) {
+ if !m.contains_key(&k) { m.insert(o, v); }
+}
+
+fn main() {
+}
fn main() {
let mut vec = vec![1, 2, 3, 4];
let vec2 = vec![1, 2, 3, 4];
- for i in 0..vec.len() { //~ERROR the loop variable `i` is only used to index `vec`.
+ for i in 0..vec.len() {
+ //~^ ERROR `i` is only used to index `vec`. Consider using `for item in &vec`
println!("{}", vec[i]);
}
- for i in 0..vec.len() { //~ERROR the loop variable `i` is used to index `vec`.
+ for i in 0..vec.len() {
+ //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate()`
println!("{} {}", vec[i], i);
}
for i in 0..vec.len() { // not an error, indexing more than one variable
println!("{} {}", vec[i], vec2[i]);
}
- for i in 5..vec.len() { // not an error, not starting with 0
+ for i in 0..vec.len() {
+ //~^ ERROR `i` is only used to index `vec2`. Consider using `for item in vec2.iter().take(vec.len())`
+ println!("{}", vec2[i]);
+ }
+
+ for i in 5..vec.len() {
+ //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().skip(5)`
+ println!("{}", vec[i]);
+ }
+
+ for i in 5..10 {
+ //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
println!("{}", vec[i]);
}
+ for i in 5..vec.len() {
+ //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().skip(5)`
+ println!("{} {}", vec[i], i);
+ }
+
+ for i in 5..10 {
+ //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().take(10).skip(5)`
+ println!("{} {}", vec[i], i);
+ }
+
for i in 10..0 { //~ERROR this range is empty so this for loop will never run
println!("{}", i);
}
#![feature(plugin)]
#![plugin(clippy)]
-#![deny(needless_lifetimes)]
-#![allow(dead_code, unused_lifetimes)]
+#![deny(needless_lifetimes, unused_lifetimes)]
+#![allow(dead_code)]
fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) { }
//~^ERROR explicit lifetimes given
#![deny(clippy)]
#![allow(unused)]
+use std::borrow::Cow;
+
+enum Foo { Bar, Baz(u8) }
+use Foo::*;
+
fn single_match(){
let x = Some(1u8);
+
match x { //~ ERROR you seem to be trying to use match
//~^ HELP try
Some(y) => {
}
_ => ()
}
- // Not linted
- match x {
- Some(y) => println!("{:?}", y),
- None => ()
- }
+
let z = (1u8,1u8);
match z { //~ ERROR you seem to be trying to use match
//~^ HELP try
}
}
+fn single_match_know_enum() {
+ let x = Some(1u8);
+ let y : Result<_, i8> = Ok(1i8);
+
+ match x { //~ ERROR you seem to be trying to use match
+ //~^ HELP try
+ Some(y) => println!("{:?}", y),
+ None => ()
+ }
+
+ match y { //~ ERROR you seem to be trying to use match
+ //~^ HELP try
+ Ok(y) => println!("{:?}", y),
+ Err(..) => ()
+ }
+
+ let c = Cow::Borrowed("");
+
+ match c { //~ ERROR you seem to be trying to use match
+ //~^ HELP try
+ Cow::Borrowed(..) => println!("42"),
+ Cow::Owned(..) => (),
+ }
+
+ let z = Foo::Bar;
+ // no warning
+ match z {
+ Bar => println!("42"),
+ Baz(_) => (),
+ }
+
+ match z {
+ Baz(_) => println!("42"),
+ Bar => (),
+ }
+}
+
fn match_bool() {
let test: bool = true;
true => 1,
false => 0,
};
-
+
match test { //~ ERROR you seem to be trying to match on a boolean expression
true => (),
false => { println!("Noooo!"); }
};
-
+
match test { //~ ERROR you seem to be trying to match on a boolean expression
false => { println!("Noooo!"); }
_ => (),
};
-
+
match test { //~ ERROR you seem to be trying to match on a boolean expression
false => { println!("Noooo!"); }
true => { println!("Yes!"); }
// Not linted
match option {
1 ... 10 => (),
- 10 ... 20 => (),
+ 11 ... 20 => (),
_ => (),
};
}
}
}
+fn overlapping() {
+ const FOO : u64 = 2;
+
+ match 42 {
+ 0 ... 10 => println!("0 ... 10"), //~ERROR: some ranges overlap
+ 0 ... 11 => println!("0 ... 10"),
+ _ => (),
+ }
+
+ match 42 {
+ 0 ... 5 => println!("0 ... 5"), //~ERROR: some ranges overlap
+ 6 ... 7 => println!("6 ... 7"),
+ FOO ... 11 => println!("0 ... 10"),
+ _ => (),
+ }
+
+ match 42 {
+ 2 => println!("2"),
+ 0 ... 5 => println!("0 ... 5"), //~ERROR: some ranges overlap
+ _ => (),
+ }
+
+ match 42 {
+ 0 ... 10 => println!("0 ... 10"),
+ 11 ... 50 => println!("0 ... 10"),
+ _ => (),
+ }
+}
+
fn main() {
}
// Check OPTION_MAP_UNWRAP_OR
// single line case
let _ = opt.map(|x| x + 1) //~ ERROR called `map(f).unwrap_or(a)`
- //~| NOTE replace this
+ //~| NOTE replace `map(|x| x + 1).unwrap_or(0)`
.unwrap_or(0); // should lint even though this call is on a separate line
// multi line cases
let _ = opt.map(|x| { //~ ERROR called `map(f).unwrap_or(a)`
// Check OPTION_MAP_UNWRAP_OR_ELSE
// single line case
let _ = opt.map(|x| x + 1) //~ ERROR called `map(f).unwrap_or_else(g)`
- //~| NOTE replace this
+ //~| NOTE replace `map(|x| x + 1).unwrap_or_else(|| 0)`
.unwrap_or_else(|| 0); // should lint even though this call is on a separate line
// multi line cases
let _ = opt.map(|x| { //~ ERROR called `map(f).unwrap_or_else(g)`
}
+/// Struct to generate false positive for Iterator-based lints
+#[derive(Copy, Clone)]
+struct IteratorFalsePositives {
+ foo: u32,
+}
+
+impl IteratorFalsePositives {
+ fn filter(self) -> IteratorFalsePositives {
+ self
+ }
+
+ fn next(self) -> IteratorFalsePositives {
+ self
+ }
+
+ fn find(self) -> Option<u32> {
+ Some(self.foo)
+ }
+
+ fn position(self) -> Option<u32> {
+ Some(self.foo)
+ }
+
+ fn rposition(self) -> Option<u32> {
+ Some(self.foo)
+ }
+}
+
+/// Checks implementation of FILTER_NEXT lint
+fn filter_next() {
+ let v = vec![3, 2, 1, 0, -1, -2, -3];
+
+ // check single-line case
+ let _ = v.iter().filter(|&x| *x < 0).next();
+ //~^ ERROR called `filter(p).next()` on an Iterator.
+ //~| NOTE replace `filter(|&x| *x < 0).next()`
+
+ // check multi-line case
+ let _ = v.iter().filter(|&x| { //~ERROR called `filter(p).next()` on an Iterator.
+ *x < 0
+ }
+ ).next();
+
+ // check that we don't lint if the caller is not an Iterator
+ let foo = IteratorFalsePositives { foo: 0 };
+ let _ = foo.filter().next();
+}
+
+/// Checks implementation of SEARCH_IS_SOME lint
+fn search_is_some() {
+ let v = vec![3, 2, 1, 0, -1, -2, -3];
+
+ // check `find().is_some()`, single-line
+ let _ = v.iter().find(|&x| *x < 0).is_some();
+ //~^ ERROR called `is_some()` after searching
+ //~| NOTE replace `find(|&x| *x < 0).is_some()`
+
+ // check `find().is_some()`, multi-line
+ let _ = v.iter().find(|&x| { //~ERROR called `is_some()` after searching
+ *x < 0
+ }
+ ).is_some();
+
+ // check `position().is_some()`, single-line
+ let _ = v.iter().position(|&x| x < 0).is_some();
+ //~^ ERROR called `is_some()` after searching
+ //~| NOTE replace `position(|&x| x < 0).is_some()`
+
+ // check `position().is_some()`, multi-line
+ let _ = v.iter().position(|&x| { //~ERROR called `is_some()` after searching
+ x < 0
+ }
+ ).is_some();
+
+ // check `rposition().is_some()`, single-line
+ let _ = v.iter().rposition(|&x| x < 0).is_some();
+ //~^ ERROR called `is_some()` after searching
+ //~| NOTE replace `rposition(|&x| x < 0).is_some()`
+
+ // check `rposition().is_some()`, multi-line
+ let _ = v.iter().rposition(|&x| { //~ERROR called `is_some()` after searching
+ x < 0
+ }
+ ).is_some();
+
+ // check that we don't lint if the caller is not an Iterator
+ let foo = IteratorFalsePositives { foo: 0 };
+ let _ = foo.find().is_some();
+ let _ = foo.position().is_some();
+ let _ = foo.rposition().is_some();
+}
+
fn main() {
use std::io;
--- /dev/null
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(unneeded_field_pattern)]
+#[allow(dead_code, unused)]
+
+struct Foo {
+ a: i32,
+ b: i32,
+ c: i32,
+}
+
+fn main() {
+ let f = Foo { a: 0, b: 0, c: 0 };
+
+ match f {
+ Foo { a: _, b: 0, .. } => {} //~ERROR You matched a field with a wildcard pattern
+ //~^ HELP Try with `Foo { b: 0, .. }`
+ Foo { a: _, b: _, c: _ } => {} //~ERROR All the struct fields are matched to a
+ //~^ HELP Try with `Foo { .. }`
+ }
+ match f {
+ Foo { b: 0, .. } => {} // should be OK
+ Foo { .. } => {} // and the Force might be with this one
+ }
+}
unimplemented!()
}
+struct X { x: u32 }
+
+impl X {
+ fn self_ref_with_lifetime<'a>(&'a self) {}
+ fn explicit_self_with_lifetime<'a>(self: &'a Self) {}
+}
+
fn main() {
}
--- /dev/null
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(wrong_self_convention)]
+#![deny(wrong_pub_self_convention)]
+#![allow(dead_code)]
+
+fn main() {}
+
+#[derive(Clone, Copy)]
+struct Foo;
+
+impl Foo {
+
+ fn as_i32(self) {}
+ fn into_i32(self) {}
+ fn is_i32(self) {}
+ fn to_i32(self) {}
+ fn from_i32(self) {} //~ERROR: methods called `from_*` usually take no self
+
+ pub fn as_i64(self) {}
+ pub fn into_i64(self) {}
+ pub fn is_i64(self) {}
+ pub fn to_i64(self) {}
+ pub fn from_i64(self) {} //~ERROR: methods called `from_*` usually take no self
+
+}
+
+struct Bar;
+
+impl Bar {
+
+ fn as_i32(self) {} //~ERROR: methods called `as_*` usually take self by reference
+ fn into_i32(&self) {} //~ERROR: methods called `into_*` usually take self by value
+ fn is_i32(self) {} //~ERROR: methods called `is_*` usually take self by reference
+ fn to_i32(self) {} //~ERROR: methods called `to_*` usually take self by reference
+ fn from_i32(self) {} //~ERROR: methods called `from_*` usually take no self
+
+ pub fn as_i64(self) {} //~ERROR: methods called `as_*` usually take self by reference
+ pub fn into_i64(&self) {} //~ERROR: methods called `into_*` usually take self by value
+ pub fn is_i64(self) {} //~ERROR: methods called `is_*` usually take self by reference
+ pub fn to_i64(self) {} //~ERROR: methods called `to_*` usually take self by reference
+ pub fn from_i64(self) {} //~ERROR: methods called `from_*` usually take no self
+
+}
--- /dev/null
+#![allow(plugin_as_library)]
+#![feature(rustc_private)]
+
+extern crate clippy;
+extern crate syntax;
+
+#[test]
+fn test_overlapping() {
+ use clippy::matches::overlapping;
+ use syntax::codemap::DUMMY_SP;
+
+ let sp = |s, e| clippy::matches::SpannedRange { span: DUMMY_SP, node: (s, e) };
+
+ assert_eq!(None, overlapping::<u8>(&[]));
+ assert_eq!(None, overlapping(&[sp(1, 4)]));
+ assert_eq!(None, overlapping(&[sp(1, 4), sp(5, 6)]));
+ assert_eq!(None, overlapping(&[sp(1, 4), sp(5, 6), sp(10, 11)]));
+ assert_eq!(Some((&sp(1, 4), &sp(3, 6))), overlapping(&[sp(1, 4), sp(3, 6)]));
+ assert_eq!(Some((&sp(5, 6), &sp(6, 11))), overlapping(&[sp(1, 4), sp(5, 6), sp(6, 11)]));
+}
projects / rust.git / commitdiff