-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Home
Welcome to the rust-clippy wiki!
Here we aim to collect further explanations on the lints clippy provides. So without further ado:
Those lints are Deny by default:
cmp_nan
invalid_regex
out_of_bounds_indexing
zero_width_space
Those lints are Warn by default:
absurd_extreme_comparisons
approx_constant
bad_bit_mask
block_in_if_condition_expr
block_in_if_condition_stmt
bool_comparison
box_vec
boxed_local
char_lit_as_u
chars_next_cmp
clone_double_ref
clone_on_copy
cmp_owned
collapsible_if
cyclomatic_complexity
deprecated_semver
derive_hash_xor_eq
drop_ref
duplicate_underscore_argument
empty_loop
enum_variant_names
eq_op
expl_impl_clone_on_copy
explicit_counter_loop
explicit_iter_loop
extend_from_slice
filter_next
float_cmp
for_kv_map
for_loop_over_option
for_loop_over_result
identity_op
if_same_then_else
ifs_same_cond
ineffective_bit_mask
inline_always
items_after_statements
iter_next_loop
len_without_is_empty
len_zero
let_and_return
let_unit_value
linkedlist
map_clone
map_entry
match_bool
match_overlapping_arm
match_ref_pats
match_same_arms
min_max
modulo_one
mutex_atomic
needless_bool
needless_lifetimes
needless_range_loop
needless_return
needless_update
new_ret_no_self
no_effect
nonsensical_open_options
ok_expect
option_map_unwrap_or
option_map_unwrap_or_else
or_fun_call
panic_params
precedence
ptr_arg
range_step_by_zero
range_zip_with_len
redundant_closure
redundant_pattern
regex_macro
reverse_range_loop
search_is_some
should_implement_trait
single_char_pattern
single_match
str_to_string
string_lit_as_bytes
string_to_string
temporary_assignment
toplevel_ref_arg
trivial_regex
type_complexity
unit_cmp
unnecessary_mut_passed
unneeded_field_pattern
unstable_as_mut_slice
unstable_as_slice
unused_collect
unused_lifetimes
used_underscore_binding
useless_format
useless_transmute
useless_vec
while_let_loop
while_let_on_iterator
wrong_self_convention
zero_divided_by_zero
Those lints are Allow by default:
cast_possible_truncation
cast_possible_wrap
cast_precision_loss
cast_sign_loss
enum_glob_use
mut_mut
mutex_integer
non_ascii_literal
option_unwrap_used
print_stdout
result_unwrap_used
shadow_reuse
shadow_same
shadow_unrelated
single_match_else
string_add
string_add_assign
unicode_not_nfc
use_debug
wrong_pub_self_convention
Clippy works as a plugin to the compiler, which means using an unstable internal API. We have gotten quite good at keeping pace with the API evolution, but the consequence is that clippy absolutely needs to be compiled with the version of rustc
it will run on, otherwise you will get strange errors of missing symbols.
Default level: Warn
What it does: This lint checks for comparisons where one side of the relation is either the minimum or maximum value for its type and warns if it involves a case that is always true or always false. Only integer and boolean types are checked.
Why is this bad? An expression like min <= x
may misleadingly imply that is is possible for x
to be less than the minimum. Expressions like max < x
are probably mistakes.
Known problems: None
Example: vec.len() <= 0
, 100 > std::i32::MAX
Default level: Warn
What it does: This lint checks for floating point literals that approximate constants which are defined in std::f32::consts
or std::f64::consts
, respectively, suggesting to use the predefined constant.
Why is this bad? Usually, the definition in the standard library is more precise than what people come up with. If you find that your definition is actually more precise, please file a Rust issue.
Known problems: If you happen to have a value that is within 1/8192 of a known constant, but is not and should not be the same, this lint will report your value anyway. We have not yet noticed any false positives in code we tested clippy with (this includes servo), but YMMV.
Example: let x = 3.14;
Default level: Warn
What it does: This lint checks for incompatible bit masks in comparisons.
The formula for detecting if an expression of the type _ <bit_op> m <cmp_op> c
(where <bit_op>
is one of {&
, |
} and <cmp_op>
is one of {!=
, >=
, >
, !=
, >=
, >
}) can be determined from the following table:
Comparison | Bit-Op | Example | is always | Formula |
---|---|---|---|---|
== or !=
|
& |
x & 2 == 3 |
false |
c & m != c |
< or >=
|
& |
x & 2 < 3 |
true |
m < c |
> or <=
|
& |
x & 1 > 1 |
false |
m <= c |
== or !=
|
` | ` | `x | 1 == 0` |
< or >=
|
` | ` | `x | 1 < 1` |
<= or >
|
` | ` | `x | 1 > 0` |
Why is this bad? If the bits that the comparison cares about are always set to zero or one by the bit mask, the comparison is constant true
or false
(depending on mask, compared value, and operators).
So the code is actively misleading, and the only reason someone would write this intentionally is to win an underhanded Rust contest or create a test-case for this lint.
Known problems: None
Example: x & 1 == 2
(also see table above)
Default level: Warn
What it does: This lint checks for if
conditions that use blocks to contain an expression.
Why is this bad? It isn't really rust style, same as using parentheses to contain expressions.
Known problems: None
Example: if { true } ..
Default level: Warn
What it does: This lint checks for if
conditions that use blocks containing statements, or conditions that use closures with blocks.
Why is this bad? Using blocks in the condition makes it hard to read.
Known problems: None
Example: if { let x = somefunc(); x } ..
or if somefunc(|x| { x == 47 }) ..
Default level: Warn
What it does: This lint checks for expressions of the form x == true
(or vice versa) and suggest using the variable directly.
Why is this bad? Unnecessary code.
Known problems: None.
Example: if x == true { }
could be if x { }
Default level: Warn
What it does: This lint checks for use of Box<Vec<_>>
anywhere in the code.
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.
Known problems: None
Example: struct X { values: Box<Vec<Foo>> }
Default level: Warn
What it does: This lint checks for usage of Box<T>
where an unboxed T
would work fine.
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.
Known problems: None
Example:
fn main() {
let x = Box::new(1);
foo(*x);
println!("{}", *x);
}
Default level: Allow
What it does: This lint checks for on casts between numerical types that may truncate large values. This is expected behavior, so the cast is Allow
by default.
Why is this bad? In some problem domains, it is good practice to avoid truncation. This lint can be activated to help assess where additional checks could be beneficial.
Known problems: None
Example: fn as_u8(x: u64) -> u8 { x as u8 }
Default level: Allow
What it does: This lint checks for casts from an unsigned type to a signed type of the same size. Performing such a cast is a 'no-op' for the compiler, i.e. nothing is changed at the bit level, and the binary representation of the value is reinterpreted. This can cause wrapping if the value is too big for the target signed type. However, the cast works as defined, so this lint is Allow
by default.
Why is this bad? While such a cast is not bad in itself, the results can be surprising when this is not the intended behavior, as demonstrated by the example below.
Known problems: None
Example: u32::MAX as i32
will yield a value of -1
.
Default level: Allow
What it does: This lint checks for casts from any numerical to a float type where the receiving type cannot store all values from the original type without rounding errors. This possible rounding is to be expected, so this lint is Allow
by default.
Basically, this warns on casting any integer with 32 or more bits to f32
or any 64-bit integer to f64
.
Why is this bad? It's not bad at all. But in some applications it can be helpful to know where precision loss can take place. This lint can help find those places in the code.
Known problems: None
Example: let x = u64::MAX; x as f64
Default level: Allow
What it does: This lint checks for casts from a signed to an unsigned numerical type. In this case, negative values wrap around to large positive values, which can be quite surprising in practice. However, as the cast works as defined, this lint is Allow
by default.
Why is this bad? Possibly surprising results. You can activate this lint as a one-time check to see where numerical wrapping can arise.
Known problems: None
Example: let y : i8 = -1; y as u64
will return 18446744073709551615
Default level: Warn
What it does: This lint points out expressions where a character literal is casted to u8
and suggests using a byte literal instead.
Why is this bad? In general, casting values to smaller types is error-prone and should be avoided where possible. In the particular case of converting a character literal to u8, it is easy to avoid by just using a byte literal instead. As an added bonus, b'a'
is even slightly shorter than 'a' as u8
.
Known problems: None
Example: 'x' as u8
Default level: Warn
What it does: This lint Warn
s on using .chars().next()
on a str
to check if it
starts with a given char.
Why is this bad? Readability, this can be written more concisely as _.starts_with(_)
.
Known problems: None.
Example: name.chars().next() == Some('_')
Default level: Warn
What it does: This lint warns on using .clone()
on an &&T
Why is this bad? Cloning an &&T
copies the inner &T
, instead of cloning the underlying
T
Known problems: None.
Example:
fn main() {
let x = vec![1];
let y = &&x;
let z = y.clone();
println!("{:p} {:p}",*y, z); // prints out the same pointer
}
Default level: Warn
What it does: This lint warns on using .clone()
on a Copy
type.
Why is this bad? The only reason Copy
types implement Clone
is for generics, not for
using the clone
method on a concrete type.
Known problems: None.
Example: 42u64.clone()
Default level: Deny
What it does: This lint checks for comparisons to NAN.
Why is this bad? NAN does not compare meaningfully to anything – not even itself – so those comparisons are simply wrong.
Known problems: None
Example: x == NAN
Default level: Warn
What it does: This lint checks for conversions to owned values just for the sake of a comparison.
Why is this bad? The comparison can operate on a reference, so creating an owned value effectively throws it away directly afterwards, which is needlessly consuming code and heap space.
Known problems: None
Example: x.to_owned() == y
Default level: Warn
What it does: This lint checks for nested if
-statements which can be collapsed by
&&
-combining their conditions and for else { if .. }
expressions that can be collapsed to
else if ..
.
Why is this bad? Each if
-statement adds one level of nesting, which makes code look more complex than it really is.
Known problems: None
Example: if x { if y { .. } }
Default level: Warn
What it does: This lint checks for methods with high cyclomatic complexity
Why is this bad? Methods of high cyclomatic complexity tend to be badly readable. Also LLVM will usually optimize small methods better.
Known problems: Sometimes it's hard to find a way to reduce the complexity
Example: No. You'll see it when you get the warning.
Default level: Warn
What it does: This lint checks for #[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(..) { ... }
Default level: Warn
What it does: This lint warns about deriving Hash
but implementing PartialEq
explicitly.
Why is this bad? The implementation of these traits must agree (for example for use with
HashMap
) so it’s probably a bad idea to use a default-generated Hash
implementation with
an explicitely defined PartialEq
. In particular, the following must hold for any type:
k1 == k2 ⇒ hash(k1) == hash(k2)
Known problems: None.
Example:
#[derive(Hash)]
struct Foo;
impl PartialEq for Foo {
..
}
Default level: Warn
What it does: This lint checks for calls to std::mem::drop
with a reference instead of an owned value.
Why is this bad? Calling drop
on a reference will only drop the reference itself, which is a no-op. It will not call the drop
method (from the Drop
trait implementation) on the underlying referenced value, which is likely what was intended.
Known problems: None
Example:
let mut lock_guard = mutex.lock();
std::mem::drop(&lock_guard) // Should have been drop(lock_guard), mutex still locked
operation_that_requires_mutex_to_be_unlocked();
Default level: Warn
What it does: This lint checks for 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) {}
Default level: Warn
What it does: This lint checks for empty loop
expressions.
Why is this bad? Those busy loops burn CPU cycles without doing anything. Think of the environment and either block on something or at least make the thread sleep for some microseconds.
Known problems: None
Example: loop {}
Default level: Allow
What it does: Warns when use
ing all variants of an enum
Why is this bad? It is usually better style to use the prefixed name of an enum variant, rather than importing variants
Known problems: Old-style enums that prefix the variants are still around
Example: use std::cmp::Ordering::*;
Default level: Warn
What it does: Warns on enum variants that are prefixed or suffixed by the same characters
Why is this bad? Enum variant names should specify their variant, not the enum, too.
Known problems: None
Example: enum Cake { BlackForestCake, HummingbirdCake }
Default level: Warn
What it does: This lint checks for equal operands to comparison, logical and bitwise,
difference and division binary operators (==
, >
, etc., &&
, ||
, &
, |
, ^
, -
and
/
).
Why is this bad? This is usually just a typo or a copy and paste error.
Known problems: False negatives: We had some false positives regarding calls (notably racer had one instance of x.pop() && x.pop()
), so we removed matching any function or method calls. We may introduce a whitelist of known pure functions in the future.
Example: x + 1 == x + 1
Default level: Warn
What it does: This lint warns about explicit Clone
implementation for Copy
types.
Why is this bad? To avoid surprising behaviour, these traits should agree and the behaviour
of Copy
cannot be overridden. In almost all situations a Copy
type should have a Clone
implementation that does nothing more than copy the object, which is what
#[derive(Copy, Clone)]
gets you.
Known problems: None.
Example:
#[derive(Copy)]
struct Foo;
impl Clone for Foo {
..
}
Default level: Warn
What it does: This lint checks for
loops over slices with an explicit counter and suggests the use of .enumerate()
.
Why is it bad? Not only is the version using .enumerate()
more readable, the compiler is able to remove bounds checks which can lead to faster code in some instances.
Known problems: None.
Example: for i in 0..v.len() { foo(v[i]); }
or for i in 0..v.len() { bar(i, v[i]); }
Default level: Warn
What it does: This lint checks for loops on x.iter()
where &x
will do, and suggest the latter.
Why is this bad? Readability.
Known problems: False negatives. We currently only warn on some known types.
Example: for x in y.iter() { .. }
(where y is a Vec
or slice)
Default level: Warn
What it does: This lint checks for usage of .extend(s)
on a Vec
to extend the vector by a slice.
Why is this bad? Since Rust 1.6, the extend_from_slice(_)
method is stable and at least for now faster.
Known problems: None.
Example: my_vec.extend(&xs)
Default level: Warn
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()
Default level: Warn
What it does: This lint checks for (in-)equality comparisons on floating-point values (apart from zero), except in functions called *eq*
(which probably implement equality for a type involving floats).
Why is this bad? Floating point calculations are usually imprecise, so asking if two values are exactly equal is asking for trouble. For a good guide on what to do, see the floating point guide.
Known problems: None
Example: y == 1.23f64
Default level: Warn
What it does: This warns when you iterate on a map (HashMap
or BTreeMap
) and ignore
either the keys or values.
Why is this bad? Readability. There are keys
and values
methods that can be used to
express that don't need the values or keys.
Known problems: None
Example:
for (k, _) in &map { .. }
could be replaced by
for k in map.keys() { .. }
Default level: Warn
What it does: This lint checks for for
loops over Option
values.
Why is this bad? Readability. This is more clearly expressed as an if let
.
Known problems: None
Example: for x in option { .. }
. This should be if let Some(x) = option { .. }
.
Default level: Warn
What it does: This lint checks for for
loops over Result
values.
Why is this bad? Readability. This is more clearly expressed as an if let
.
Known problems: None
Example: for x in result { .. }
. This should be if let Ok(x) = result { .. }
.
Default level: Warn
What it does: This lint checks for identity operations, e.g. x + 0
.
Why is this bad? This code can be removed without changing the meaning. So it just obscures what's going on. Delete it mercilessly.
Known problems: None
Example: x / 1 + 0 * 1 - 0 | 0
Default level: Warn
What it does: This lint checks for if/else
with the same body as the then part and the
else part. This lint is Warn
by default.
Why is this bad? This is probably a copy & paste error.
Known problems: Hopefully none.
Example: if .. { 42 } else { 42 }
Default level: Warn
What it does: This lint checks for consecutive ifs
with the same condition. This lint is
Warn
by default.
Why is this bad? This is probably a copy & paste error.
Known problems: Hopefully none.
Example: if a == b { .. } else if a == b { .. }
Default level: Warn
What it does: This lint checks for bit masks in comparisons which can be removed without changing the outcome. The basic structure can be seen in the following table:
Comparison | Bit-Op | Example | equals |
---|---|---|---|
> / <=
|
` |
/ ^` |
`x |
< / >=
|
` |
/ ^` |
x ^ 1 < 4 |
Why is this bad? Not equally evil as bad_bit_mask
, but still a bit misleading, because the bit mask is ineffective.
Known problems: False negatives: This lint will only match instances where we have figured out the math (which is for a power-of-two compared value). This means things like x | 1 >= 7
(which would be better written as x >= 6
) will not be reported (but bit masks like this are fairly uncommon).
Example: x | 1 > 3
(also see table above)
Default level: Warn
What it does: This lint checks for 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.
As a rule of thumb, before slapping #[inline(always)]
on a function, measure if that additional function call really affects your runtime profile sufficiently to make up for the increase in compile time.
Known problems: False positives, big time. This lint is meant to be deactivated by everyone doing serious performance work. This means having done the measurement.
Example:
#[inline(always)]
fn not_quite_hot_code(..) { ... }
Default level: Deny
What it does: This lint checks Regex::new(_)
invocations for correct regex syntax.
Why is this bad? This will lead to a runtime panic.
Known problems: None.
Example: Regex::new("|")
Default level: Warn
What it does: This lints checks for items declared after some statement in a block
Why is this bad? Items live for the entire scope they are declared in. But statements are processed in order. This might cause confusion as it's hard to figure out which item is meant in a statement.
Known problems: None
Example:
fn foo() {
println!("cake");
}
fn main() {
foo(); // prints "foo"
fn foo() {
println!("foo");
}
foo(); // prints "foo"
}
Default level: Warn
What it does: This lint checks for loops on x.next()
.
Why is this bad? next()
returns either Some(value)
if there was a value, or None
otherwise. The insidious thing is that Option<_>
implements IntoIterator
, so that possibly one value will be iterated, leading to some hard to find bugs. No one will want to write such code except to win an Underhanded Rust Contest.
Known problems: None
Example: for x in y.next() { .. }
Default level: Warn
What it does: This lint checks for items that implement .len()
but not .is_empty()
.
Why is this bad? It is good custom to have both methods, because for some data structures, asking about the length will be a costly operation, whereas .is_empty()
can usually answer in constant time. Also it used to lead to false positives on the len_zero
lint – currently that lint will ignore such entities.
Known problems: None
Example:
impl X {
fn len(&self) -> usize { .. }
}
Default level: Warn
What it does: This lint checks for getting the length of something via .len()
just to compare to zero, and suggests using .is_empty()
where applicable.
Why is this bad? Some structures can answer .is_empty()
much faster than calculating their length. So it is good to get into the habit of using .is_empty()
, and having it is cheap. Besides, it makes the intent clearer than a comparison.
Known problems: None
Example: if x.len() == 0 { .. }
Default level: Warn
What it does: This lint checks for let
-bindings, which are subsequently returned.
Why is this bad? It is just extraneous code. Remove it to make your code more rusty.
Known problems: None
Example: { let x = ..; x }
Default level: Warn
What it does: This lint checks for binding a unit value.
Why is this bad? A unit value cannot usefully be used anywhere. So binding one is kind of pointless.
Known problems: None
Example: let x = { 1; };
Default level: Warn
What it does: This lint checks for usage of any LinkedList
, suggesting to use a Vec
or a VecDeque
(formerly called RingBuf
).
Why is this bad? Gankro says:
The TL;DR of
LinkedList
is that it's built on a massive amount of pointers and indirection. It wastes memory, it has terrible cache locality, and is all-around slow.RingBuf
, while "only" amortized for push/pop, should be faster in the general case for almost every possible workload, and isn't even amortized at all if you can predict the capacity you need.
LinkedList
s are only really good if you're doing a lot of merging or splitting of lists. This is because they can just mangle some pointers instead of actually copying the data. Even if you're doing a lot of insertion in the middle of the list,RingBuf
can still be better because of how expensive it is to seek to the middle of aLinkedList
.
Known problems: False positives – the instances where using a LinkedList
makes sense are few and far between, but they can still happen.
Example: let x = LinkedList::new();
Default level: Warn
What it does: This lint checks for mapping clone() over an iterator.
Why is this bad? It makes the code less readable.
Known problems: None
Example: x.map(|e| e.clone());
Default level: Warn
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:
if !m.contains_key(&k) { m.insert(k, v) }
can be rewritten as:
m.entry(k).or_insert(v);
Default level: Warn
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.
Why is this bad? It makes the code less readable.
Known problems: None
Example:
let condition: bool = true;
match condition {
true => foo(),
false => bar(),
}
Default level: Warn
What it does: This lint checks for overlapping match arms.
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"),
_ => (),
}
Default level: Warn
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.
Why is this bad? It just makes the code less readable. That reference destructuring adds nothing to the code.
Known problems: None
Example:
match x {
&A(ref y) => foo(y),
&B => bar(),
_ => frob(&x),
}
Default level: Warn
What it does: This lint checks for match
with identical arm bodies.
Why is this bad? This is probably a copy & paste error.
Known problems: Hopefully none.
Example:
match foo {
Bar => bar(),
Quz => quz(),
Baz => bar(), // <= oups
}
Default level: Warn
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.
Why is this bad? This is in all probability not the intended outcome. At the least it hurts readability of the code.
Known problems: None
Example: min(0, max(100, x))
will always be equal to 0
. Probably the author meant to clamp the value between 0 and 100, but has erroneously swapped min
and max
.
Default level: Warn
What it does: This lint checks for getting the remainder of a division by one.
Why is this bad? The result can only ever be zero. No one will write such code deliberately, unless trying to win an Underhanded Rust Contest. Even for that contest, it's probably a bad idea. Use something more underhanded.
Known problems: None
Example: x % 1
Default level: Allow
What it does: This lint checks for instances of mut mut
references.
Why is this bad? Multiple mut
s don't add anything meaningful to the source.
Known problems: None
Example: let x = &mut &mut y;
Default level: Warn
What it does: This lint checks for usages of Mutex<X>
where an atomic will do.
Why is this bad? Using a Mutex just to make access to a plain bool or reference sequential is shooting flies with cannons. std::atomic::AtomicBool
and std::atomic::AtomicPtr
are leaner and faster.
Known problems: This lint cannot detect if the Mutex is actually used for waiting before a critical section.
Example: let x = Mutex::new(&y);
Default level: Allow
What it does: This lint checks for usages of Mutex<X>
where X
is an integral type.
Why is this bad? Using a Mutex just to make access to a plain integer sequential is shooting flies with cannons. std::atomic::usize
is leaner and faster.
Known problems: This lint cannot detect if the Mutex is actually used for waiting before a critical section.
Example: let x = Mutex::new(0usize);
Default level: Warn
What it does: This lint checks for expressions of the form if c { true } else { false }
(or vice versa) and suggest using the condition directly.
Why is this bad? Redundant code.
Known problems: Maybe false positives: Sometimes, the two branches are painstakingly documented (which we of course do not detect), so they may have some value. Even then, the documentation can be rewritten to match the shorter code.
Example: if x { false } else { true }
Default level: Warn
What it does: This lint checks for lifetime annotations which can be removed by relying on lifetime elision.
Why is this bad? The additional lifetimes make the code look more complicated, while there is nothing out of the ordinary going on. Removing them leads to more readable code.
Known problems: Potential false negatives: we bail out if the function has a where
clause where lifetimes are mentioned.
Example: fn in_and_out<'a>(x: &'a u8, y: u8) -> &'a u8 { x }
Default level: Warn
What it does: This lint checks for looping over the range of 0..len
of some collection just to get the values by index.
Why is this bad? Just iterating the collection itself makes the intent more clear and is probably faster.
Known problems: None
Example:
for i in 0..vec.len() {
println!("{}", vec[i]);
}
Default level: Warn
What it does: This lint checks for return statements at the end of a block.
Why is this bad? Removing the return
and semicolon will make the code more rusty.
Known problems: None
Example: fn foo(x: usize) { return x; }
Default level: Warn
What it does: This lint warns on needlessly including a base struct on update when all fields are changed anyway.
Why is this bad? This will cost resources (because the base has to be somewhere), and make the code less readable.
Known problems: None.
Example: Point { x: 1, y: 0, ..zero_point }
Default level: Warn
What it does: This lint warns about new
not returning Self
.
Why is this bad? As a convention, new
methods are used to make a new instance of a type.
Known problems: None.
Example:
impl Foo {
fn new(..) -> NotAFoo {
}
}
Default level: Warn
What it does: This lint checks for statements which have no effect.
Why is this bad? Similar to dead code, these statements are actually executed. However, as they have no effect, all they do is make the code less readable.
Known problems: None.
Example: 0;
Default level: Allow
What it does: This lint checks for non-ascii characters in string literals.
Why is this bad? Yeah, we know, the 90's called and wanted their charset back. Even so, there still are editors and other programs out there that don't work well with unicode. So if the code is meant to be used internationally, on multiple operating systems, or has other portability requirements, activating this lint could be useful.
Known problems: None
Example: let x = "Hä?"
Default level: Warn
What it does: This lint checks for duplicate open options as well as combinations that make no sense.
Why is this bad? In the best case, the code will be harder to read than necessary. I don't know the worst case.
Known problems: None
Example: OpenOptions::new().read(true).truncate(true)
Default level: Warn
What it does: This lint checks for usage of ok().expect(..)
.
Why is this bad? Because you usually call expect()
on the Result
directly to get a good error message.
Known problems: None.
Example: x.ok().expect("why did I do this again?")
Default level: Warn
What it does: This lint checks for usage of _.map(_).unwrap_or(_)
.
Why is this bad? Readability, this can be written more concisely as _.map_or(_, _)
.
Known problems: None.
Example: x.map(|a| a + 1).unwrap_or(0)
Default level: Warn
What it does: This lint Warn
s on _.map(_).unwrap_or_else(_)
.
Why is this bad? Readability, this can be written more concisely as _.map_or_else(_, _)
.
Known problems: None.
Example: x.map(|a| a + 1).unwrap_or_else(some_function)
Default level: Allow
What it does: This lint checks for .unwrap()
calls on Option
s.
Why is this bad? Usually it is better to handle the None
case, or to at least call .expect(_)
with a more helpful message. Still, for a lot of quick-and-dirty code, unwrap
is a good choice, which is why this lint is Allow
by default.
Known problems: None
Example: x.unwrap()
Default level: Warn
What it does: This lint checks for calls to .or(foo(..))
, .unwrap_or(foo(..))
, etc., and
suggests to use or_else
, unwrap_or_else
, etc., or unwrap_or_default
instead.
Why is this bad? The function will always be called and potentially allocate an object in expressions such as:
foo.unwrap_or(String::new())
this can instead be written:
foo.unwrap_or_else(String::new)
or
foo.unwrap_or_default()
Known problems: If the function as side-effects, not calling it will change the semantic of the program, but you shouldn't rely on that anyway.
Default level: Deny
What it does: Check for out of bounds array indexing with a constant index.
Why is this bad? This will always panic at runtime.
Known problems: Hopefully none.
Example:
let x = [1,2,3,4];
...
x[9];
Default level: Warn
What it does: This lint checks for missing parameters in panic!
.
Known problems: Should you want to use curly brackets in panic!
without any parameter,
this lint will warn.
Example:
panic!("This `panic!` is probably missing a parameter there: {}");
Default level: Warn
What it does: This lint checks for operations where precedence may be unclear and suggests 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
Why is this bad? Because not everyone knows the precedence of those operators by heart, so expressions like these may trip others trying to reason about the code.
Known problems: None
Examples:
-
1 << 2 + 3
equals 32, while(1 << 2) + 3
equals 7 -
-1i32.abs()
equals -1, while(-1i32).abs()
equals 1
Default level: Allow
What it does: This lint warns whenever you print on stdout. The purpose of this lint is to catch debugging remnants.
Why is this bad? People often print on stdout while debugging an application and might forget to remove those prints afterward.
Known problems: Only catches print!
and println!
calls.
Example: println!("Hello world!");
Default level: Warn
What it does: This lint checks for function arguments of type &String
or &Vec
unless the references are mutable.
Why is this bad? Requiring the argument to be of the specific size makes the function less useful for no benefit; slices in the form of &[T]
or &str
usually suffice and can be obtained from other types, too.
Known problems: None
Example: fn foo(&Vec<u32>) { .. }
Default level: Warn
What it does: This lint checks for iterating over ranges with a .step_by(0)
, which never terminates.
Why is this bad? This very much looks like an oversight, since with loop { .. }
there is an obvious better way to endlessly loop.
Known problems: None
Example: for x in (5..5).step_by(0) { .. }
Default level: Warn
What it does: This lint checks for zipping a collection with the range of 0.._.len()
.
Why is this bad? The code is better expressed with .enumerate()
.
Known problems: None
Example: x.iter().zip(0..x.len())
Default level: Warn
What it does: This lint checks for closures which just call another function where the function can be called directly. unsafe
functions or calls where types get adjusted are ignored.
Why is this bad? Needlessly creating a closure just costs heap space and adds code for no benefit.
Known problems: None
Example: xs.map(|x| foo(x))
where foo(_)
is a plain function that takes the exact argument type of x
.
Default level: Warn
What it does: This lint checks for patterns in the form name @ _
.
Why is this bad? It's almost always more readable to just use direct bindings.
Known problems: None
Example:
match v {
Some(x) => (),
y @ _ => (), // easier written as `y`,
}
Default level: Warn
What it does: This lint checks for usage of regex!(_)
which as of now is usually slower than Regex::new(_)
unless called in a loop (which is a bad idea anyway).
Why is this bad? Performance, at least for now. The macro version is likely to catch up long-term, but for now the dynamic version is faster.
Known problems: None
Example: regex!("foo|bar")
Default level: Allow
What it does: This lint checks for .unwrap()
calls on Result
s.
Why is this bad? result.unwrap()
will let the thread panic on Err
values. Normally, you want to implement more sophisticated error handling, and propagate errors upwards with try!
.
Even if you want to panic on errors, not all Error
s implement good messages on display. Therefore it may be beneficial to look at the places where they may get displayed. Activate this lint to do just that.
Known problems: None
Example: x.unwrap()
Default level: Warn
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(_)
.
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.
Known problems: The lint cannot catch loops over dynamically defined ranges. Doing this would require simulating all possible inputs and code paths through the program, which would be complex and error-prone.
Examples: for x in 5..10-5 { .. }
(oops, stray -
)
Default level: Warn
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()
Default level: Allow
What it does: This lint checks for bindings that shadow other bindings already in scope, while reusing the original value.
Why is this bad? Not too much, in fact it's a common pattern in Rust code. Still, some argue that name shadowing like this hurts readability, because a value may be bound to different things depending on position in the code.
Known problems: This lint, as the other shadowing related lints, currently only catches very simple patterns.
Example: let x = x + 1;
Default level: Allow
What it does: This lint checks for bindings that shadow other bindings already in scope, while just changing reference level or mutability.
Why is this bad? Not much, in fact it's a very common pattern in Rust code. Still, some may opt to avoid it in their code base, they can set this lint to Warn
.
Known problems: This lint, as the other shadowing related lints, currently only catches very simple patterns.
Example: let x = &x;
Default level: Allow
What it does: This lint checks for bindings that shadow other bindings already in scope, either without a initialization or with one that does not even use the original value.
Why is this bad? Name shadowing can hurt readability, especially in large code bases, because it is easy to lose track of the active binding at any place in the code. This can be alleviated by either giving more specific names to bindings ore introducing more scopes to contain the bindings.
Known problems: This lint, as the other shadowing related lints, currently only catches very simple patterns.
Example: let x = y; let x = z; // shadows the earlier binding
Default level: Warn
What it does: This lint checks for methods that should live in a trait implementation of a std
trait (see llogiq's blog post for further information) instead of an inherent implementation.
Why is this bad? Implementing the traits improve ergonomics for users of the code, often with very little cost. Also people seeing a mul(..)
method may expect *
to work equally, so you should have good reason to disappoint them.
Known problems: None
Example:
struct X;
impl X {
fn add(&self, other: &X) -> X { .. }
}
Default level: Warn
What it does: This lint checks for string methods that receive a single-character str
as an argument, e.g. _.split("x")
.
Why is this bad? Performing these methods using a char
is faster than using a str
.
Known problems: Does not catch multi-byte unicode characters.
Example: _.split("x")
could be _.split('x')
Default level: Warn
What it does: This lint checks for matches with a single arm where an if let
will usually suffice.
Why is this bad? Just readability – if let
nests less than a match
.
Known problems: None
Example:
match x {
Some(ref foo) -> bar(foo),
_ => ()
}
Default level: Allow
What it does: This lint checks for matches with a two arms where an if let
will usually suffice.
Why is this bad? Just readability – if let
nests less than a match
.
Known problems: Personal style preferences may differ
Example:
match x {
Some(ref foo) -> bar(foo),
_ => bar(other_ref),
}
Default level: Warn
What it does: This lint checks for .to_string()
method calls on values of type &str
.
Why is this bad? This uses the whole formatting machinery just to clone a string. Using .to_owned()
is lighter on resources. You can also consider using a Cow<'a, str>
instead in some cases.
Known problems: None
Example: s.to_string()
where s: &str
Default level: Allow
What it does: The string_add
lint matches all instances of x + _
where x
is of type String
, but only if string_add_assign
does not match.
Why is this bad? It's not bad in and of itself. However, this particular Add
implementation is asymmetric (the other operand need not be String
, but x
does), while addition as mathematically defined is symmetric, also the String::push_str(_)
function is a perfectly good replacement. Therefore some dislike it and wish not to have it in their code.
That said, other people think that String addition, having a long tradition in other languages is actually fine, which is why we decided to make this particular lint allow
by default.
Known problems: None
Example:
let x = "Hello".to_owned();
x + ", World"
Default level: Allow
What it does: This lint matches code of the form x = x + y
(without let
!).
Why is this bad? It's not really bad, but some people think that the .push_str(_)
method is more readable.
Known problems: None.
Example:
let mut x = "Hello".to_owned();
x = x + ", World";
Default level: Warn
What it does: This lint matches the as_bytes
method called on string
literals that contain only ascii characters.
Why is this bad? Byte string literals (e.g. b"foo"
) can be used instead. They are shorter but less discoverable than as_bytes()
.
Example:
let bs = "a byte string".as_bytes();
Default level: Warn
What it does: This lint checks for .to_string()
method calls on values of type String
.
Why is this bad? This is an non-efficient way to clone a String
, .clone()
should be used
instead. String
implements ToString
mostly for generics.
Known problems: None
Example: s.to_string()
where s: String
Default level: Warn
What it does: This lint checks for construction of a structure or tuple just to assign a value in it.
Why is this bad? Readability. If the structure is only created to be updated, why not write the structure you want in the first place?
Known problems: None.
Example: (0, 0).0 = 1
Default level: Warn
What it does: This lint checks for function arguments and let bindings denoted as ref
.
Why is this bad? The ref
declaration makes the function take an owned value, but turns the argument into a reference (which means that the value is destroyed when exiting the function). This adds not much value: either take a reference type, or take an owned value and create references in the body.
For let bindings, let x = &foo;
is preferred over let ref x = foo
. The type of x
is more obvious with the former.
Known problems: If the argument is dereferenced within the function, removing the ref
will lead to errors. This can be fixed by removing the dereferences, e.g. changing *x
to x
within the function.
Example: fn foo(ref x: u8) -> bool { .. }
Default level: Warn
What it does: This lint checks for Regex::new(_)
invocations with trivial regex.
Why is this bad? This can likely be replaced by ==
or str::starts_with
,
str::ends_with
or std::contains
or other str
methods.
Known problems: None.
Example: Regex::new("^foobar")
Default level: Warn
What it does: This lint checks for types used in structs, parameters and let
declarations above a certain complexity threshold.
Why is this bad? Too complex types make the code less readable. Consider using a type
definition to simplify them.
Known problems: None
Example: struct Foo { inner: Rc<Vec<Vec<Box<(u32, u32, u32, u32)>>>> }
Default level: Allow
What it does: This lint checks for string literals that contain unicode in a form that is not equal to its NFC-recomposition.
Why is this bad? If such a string is compared to another, the results may be surprising.
Known problems None
Example: You may not see it, but "à" and "à" aren't the same string. The former when escaped is actually "a\u{300}" while the latter is "\u{e0}".
Default level: Warn
What it does: This lint checks for comparisons to unit.
Why is this bad? Unit is always equal to itself, and thus is just a clumsily written constant. Mostly this happens when someone accidentally adds semicolons at the end of the operands.
Known problems: None
Example: if { foo(); } == { bar(); } { baz(); }
is equal to { foo(); bar(); baz(); }
Default level: Warn
What it does: This lint detects giving a mutable reference to a function that only requires an immutable reference.
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.
Known problems: None
Example my_vec.push(&mut value)
Default level: Warn
What it does: This lint checks for structure field patterns bound to wildcards.
Why is this bad? Using ..
instead is shorter and leaves the focus on the fields that are actually bound.
Known problems: None.
Example: let { a: _, b: ref b, c: _ } = ..
Default level: Warn
What it does: This lint checks for usage of the as_mut_slice(..)
function, which is unstable.
Why is this bad? Using this function doesn't make your code better, but it will preclude it from building with stable Rust.
Known problems: None.
Example: x.as_mut_slice(..)
Default level: Warn
What it does: This lint checks for usage of the as_slice(..)
function, which is unstable.
Why is this bad? Using this function doesn't make your code better, but it will preclude it from building with stable Rust.
Known problems: None.
Example: x.as_slice(..)
Default level: Warn
What it does: This lint checks for using collect()
on an iterator without using the result.
Why is this bad? It is more idiomatic to use a for
loop over the iterator instead.
Known problems: None
Example: vec.iter().map(|x| /* some operation returning () */).collect::<Vec<_>>();
Default level: Warn
What it does: This lint checks for lifetimes in generics that are never used anywhere else.
Why is this bad? The additional lifetimes make the code look more complicated, while there is nothing out of the ordinary going on. Removing them leads to more readable code.
Known problems: None
Example: fn unused_lifetime<'a>(x: u8) { .. }
Default level: Allow
What it does: This lint warns whenever you use Debug
formatting. The purpose of this lint is to catch debugging remnants.
Why is this bad? The purpose of the Debug
trait is to facilitate debugging Rust code. It
should not be used in in user-facing output.
Example: println!("{:?}", foo);
Default level: Warn
What it does: This lint checks for the use of bindings with a single leading underscore
Why is this bad? A single leading underscore is usually used to indicate that a binding will not be used. Using such a binding breaks this expectation.
Known problems: None
Example:
let _x = 0;
let y = _x + 1; // Here we are using `_x`, even though it has a leading underscore.
// We should rename `_x` to `x`
Default level: Warn
What it does: This lints about use of format!("string literal with no argument")
and
format!("{}", foo)
.
Why is this bad? There is no point of doing that. If you want a String
you can use
to_owned
on the string literal or expression. The even worse &format!("foo")
is often
encountered in the wild.
Known problems: None.
Examples: format!("foo")
and format!("{}", foo)
Default level: Warn
What it does: This lint checks for transmutes to the original type of the object.
Why is this bad? Readability. The code tricks people into thinking that the original value was of some other type.
Known problems: None.
Example: core::intrinsics::transmute(t)
where the result type is the same as t
's.
Default level: Warn
What it does: This lint warns about using &vec![..]
when using &[..]
would be possible.
Why is this bad? This is less efficient.
Known problems: None.
Example:
foo(&vec![1, 2])
Default level: Warn
What it does: This lint detects loop + match
combinations that are easier written as a while let
loop.
Why is this bad? The while let
loop is usually shorter and more readable
Known problems: Sometimes the wrong binding is displayed (#383)
Example:
loop {
let x = match y {
Some(x) => x,
None => break,
}
// .. do something with x
}
// is easier written as
while let Some(x) = y {
// .. do something with x
}
Default level: Warn
What it does: This lint checks for while let
expressions on iterators.
Why is this bad? Readability. A simple for
loop is shorter and conveys the intent better.
Known problems: None
Example: while let Some(val) = iter() { .. }
Default level: Allow
What it does: This is the same as wrong_self_convention
, but for public items.
Why is this bad? See wrong_self_convention
.
Known problems: Actually renaming the function may break clients if the function is part of the public interface. In that case, be mindful of the stability guarantees you've given your users.
Example:
impl X {
pub fn as_str(self) -> &str { .. }
}
Default level: Warn
What it does: This lint checks for methods with certain name prefixes and which doesn't match how self is taken. The actual rules are:
Prefix |
self taken |
---|---|
as_ |
&self or &mut self |
from_ |
none |
into_ |
self |
is_ |
&self or none |
to_ |
&self |
Why is this bad? Consistency breeds readability. If you follow the conventions, your users won't be surprised that they e.g. need to supply a mutable reference to a as_..
function.
Known problems: None
Example
impl X {
fn as_str(self) -> &str { .. }
}
Default level: Warn
What it does: This lint checks for 0.0 / 0.0
.
Why is this bad? It's less readable than std::f32::NAN
or std::f64::NAN
Known problems: None
Example 0.0f32 / 0.0
Default level: Deny
What it does: This lint checks for the unicode zero-width space in the code.
Why is this bad? Having an invisible character in the code makes for all sorts of April fools, but otherwise is very much frowned upon.
Known problems: None
Example: You don't see it, but there may be a zero-width space somewhere in this text.