-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
unused_io_amount
zero_width_space
Those lints are Warn by default:
absurd_extreme_comparisons
almost_swapped
approx_constant
assign_op_pattern
bad_bit_mask
blacklisted_name
block_in_if_condition_expr
block_in_if_condition_stmt
bool_comparison
box_vec
boxed_local
builtin_type_shadow
char_lit_as_u8
chars_next_cmp
clone_double_ref
clone_on_copy
cmp_null
cmp_owned
collapsible_if
crosspointer_transmute
cyclomatic_complexity
deprecated_semver
deref_addrof
derive_hash_xor_eq
diverging_sub_expression
doc_markdown
double_neg
double_parens
drop_ref
duplicate_underscore_argument
empty_loop
enum_clike_unportable_variant
enum_variant_names
eq_op
eval_order_dependence
expl_impl_clone_on_copy
explicit_counter_loop
explicit_into_iter_loop
explicit_iter_loop
filter_next
float_cmp
for_kv_map
for_loop_over_option
for_loop_over_result
forget_ref
get_unwrap
identity_op
if_let_redundant_pattern_matching
if_let_some_result
if_same_then_else
ifs_same_cond
ineffective_bit_mask
inline_always
iter_next_loop
iter_nth
iter_skip_next
large_enum_variant
len_without_is_empty
len_zero
let_and_return
let_unit_value
linkedlist
logic_bug
manual_swap
many_single_char_names
map_clone
map_entry
match_bool
match_overlapping_arm
match_ref_pats
match_same_arms
min_max
misrefactored_assign_op
mixed_case_hex_literals
module_inception
modulo_one
mutex_atomic
needless_bool
needless_borrow
needless_lifetimes
needless_range_loop
needless_return
needless_update
neg_multiply
new_ret_no_self
new_without_default
new_without_default_derive
no_effect
nonsensical_open_options
not_unsafe_ptr_arg_deref
ok_expect
or_fun_call
overflow_check_conditional
panic_params
partialeq_ne_impl
precedence
print_with_newline
ptr_arg
range_step_by_zero
range_zip_with_len
redundant_closure
redundant_closure_call
redundant_pattern
regex_macro
reverse_range_loop
search_is_some
serde_api_misuse
short_circuit_statement
should_implement_trait
single_char_pattern
single_match
string_extend_chars
string_lit_as_bytes
suspicious_assignment_formatting
suspicious_else_formatting
temporary_assignment
temporary_cstring_as_ptr
too_many_arguments
toplevel_ref_arg
transmute_ptr_to_ref
trivial_regex
type_complexity
unit_cmp
unnecessary_mut_passed
unnecessary_operation
unneeded_field_pattern
unsafe_removed_from_name
unused_collect
unused_label
unused_lifetimes
useless_attribute
useless_format
useless_let_if_seq
useless_transmute
useless_vec
while_let_loop
while_let_on_iterator
wrong_self_convention
wrong_transmute
zero_divided_by_zero
zero_prefixed_literal
Those lints are Allow by default:
assign_ops
cast_possible_truncation
cast_possible_wrap
cast_precision_loss
cast_sign_loss
enum_glob_use
filter_map
float_arithmetic
if_not_else
indexing_slicing
integer_arithmetic
invalid_upcast_comparisons
items_after_statements
mem_forget
missing_docs_in_private_items
mut_mut
mutex_integer
non_ascii_literal
nonminimal_bool
option_map_unwrap_or
option_map_unwrap_or_else
option_unwrap_used
print_stdout
pub_enum_variant_names
result_unwrap_used
shadow_reuse
shadow_same
shadow_unrelated
similar_names
single_match_else
string_add
string_add_assign
stutter
unicode_not_nfc
unseparated_literal_suffix
use_debug
used_underscore_binding
wrong_pub_self_convention
Those lints are deprecated:
extend_from_slice
str_to_string
string_to_string
unstable_as_mut_slice
unstable_as_slice
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: 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: Checks for foo = bar; bar = foo
sequences.
Why is this bad? This looks like a failed attempt to swap.
Known problems: None.
Example:
a = b;
b = a;
Default level: Warn
What it does: 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: Checks for a = a op b
or a = b commutative_op a
patterns.
Why is this bad? These can be written as the shorter a op= b
.
Known problems: While forbidden by the spec, OpAssign
traits may have
implementations that differ from the regular Op
impl.
Example:
let mut a = 5;
...
a = a + b;
Default level: Allow
What it does: Checks for compound assignment operations (+=
and similar).
Why is this bad? Projects with many developers from languages without those operations may find them unreadable and not worth their weight.
Known problems: Types implementing OpAssign
don't necessarily implement Op
.
Example:
a += 1;
Default level: Warn
What it does: 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:
if (x & 1 == 2) { … }
Default level: Warn
What it does: Checks for usage of blacklisted names for variables, such
as foo
.
Why is this bad? These names are usually placeholder names and should be avoided.
Known problems: None.
Example:
let foo = 3.14;
Configuration: This lint has the following configuration variables:
-
blacklisted-names: Vec<String>
: The list of blacklisted names to lint about (defaults to["foo", "bar", "baz", "quux"]
).
Default level: Warn
What it does: 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: 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: 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: 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: 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);
}
Configuration: This lint has the following configuration variables:
-
too-large-for-stack: u64
: The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap (defaults to200
).
Default level: Warn
What it does: Warns if a generic shadows a built-in type.
Why is this bad? This gives surprising type errors.
Known problems: None.
Example:
impl<u32> Foo<u32> {
fn impl_func(&self) -> u32 {
42
}
}
Default level: Allow
What it does: 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: 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: 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: 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 u128 // will return 18446744073709551615
Default level: Warn
What it does: Checks for expressions where a character literal is cast
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: Checks for usage of .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: Checks for usage of .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: Checks for usage of .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: 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 equality comparisons with ptr::null
Why is this bad? It's easier and more readable to use the inherent .is_null()
method instead
Known problems: None.
Example:
if x == ptr::null { .. }
Default level: Warn
What it does: 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: 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 {
…
}
}
// or
if x {
…
} else {
if y {
…
}
}
Should be written:
if x && y {
…
}
// or
if x {
…
} else if y {
…
}
Default level: Warn
What it does: Checks for transmutes between a type T
and *T
.
Why is this bad? It's easy to mistakenly transmute between a type and a pointer to that type.
Known problems: None.
Example:
core::intrinsics::transmute(t)` // where the result type is the same as `*t` or `&t`'s
Default level: Warn
What it does: 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.
Configuration: This lint has the following configuration variables:
-
cyclomatic-complexity-threshold: u64
: The maximum cyclomatic complexity a function can have (defaults to25
).
Default level: Warn
What it does: 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 a 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: Checks for usage of *&
and *&mut
in expressions.
Why is this bad? Immediately dereferencing a reference is no-op and makes the code less clear.
Known problems: Multiple dereference/addrof pairs are not handled so
the suggested fix for x = **&&y
is x = *&y
, which is still incorrect.
Example:
let a = f(*&mut b);
let c = *&d;
Default level: Warn
What it does: Checks for 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 explicitly 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: Checks for diverging calls that are not match arms or statements.
Why is this bad? It is often confusing to read. In addition, the sub-expression evaluation order for Rust is not well documented.
Known problems: Someone might want to use some_bool || panic!()
as a shorthand.
Example:
let a = b() || panic!() || c();
// `c()` is dead, `panic!()` is only called if `b()` returns `false`
let x = (a, b, c, panic!());
// can simply be replaced by `panic!()`
Default level: Warn
What it does: Checks for the presence of _
, ::
or camel-case words
outside ticks in documentation.
Why is this bad? Rustdoc supports markdown formatting, _
, ::
and
camel-case probably indicates some code which should be included between
ticks. _
can also be used for empasis in markdown, this lint tries to
consider that.
Known problems: Lots of bad docs won’t be fixed, what the lint checks for is limited, and there are still false positives.
Examples:
/// Do something with the foo_bar parameter. See also that::other::module::foo.
// ^ `foo_bar` and `that::other::module::foo` should be ticked.
fn doit(foo_bar) { .. }
Configuration: This lint has the following configuration variables:
-
doc-valid-idents: Vec<String>
: The list of words this lint should not consider as identifiers needing ticks (defaults to[ "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", "GPLv2", "GPLv3", "GitHub", "IPv4", "IPv6", "JavaScript", "NaN", "OAuth", "OpenGL", "TrueType", "iOS", "macOS", ]
).
Default level: Warn
What it does: Detects expressions of the form --x
.
Why is this bad? It can mislead C/C++ programmers to think x
was
decremented.
Known problems: None.
Example:
--x;
Default level: Warn
What it does: Checks for unnecessary double parentheses.
Why is this bad? This makes code harder to read and might indicate a mistake.
Known problems: None.
Example:
((0))
foo((0))
((1, 2))
Default level: Warn
What it does: 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: 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: 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: Warn
What it does: Checks for C-like enumerations that are
repr(isize/usize)
and have values that don't fit into an i32
.
Why is this bad? This will truncate the variant value on 32 bit architectures, but works fine on 64 bit.
Known problems: None.
Example:
#[repr(usize)]
enum NonPortable {
X = 0x1_0000_0000,
Y = 0
}
Default level: Allow
What it does: Checks for use Enum::*
.
Why is this bad? It is usually better style to use the prefixed name of an enumeration variant, rather than importing variants.
Known problems: Old-style enumerations that prefix the variants are still around.
Example:
use std::cmp::Ordering::*;
Default level: Warn
What it does: Detects enumeration variants that are prefixed or suffixed by the same characters.
Why is this bad? Enumeration variant names should specify their variant, not repeat the enumeration name.
Known problems: None.
Example:
enum Cake {
BlackForestCake,
HummingbirdCake,
}
Configuration: This lint has the following configuration variables:
-
enum-variant-name-threshold: u64
: The minimum number of enum variants for the lints about variant names to trigger (defaults to3
).
Default level: Warn
What it does: 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: Checks for a read and a write to the same variable where whether the read occurs before or after the write depends on the evaluation order of sub-expressions.
Why is this bad? It is often confusing to read. In addition, the sub-expression evaluation order for Rust is not well documented.
Known problems: Code which intentionally depends on the evaluation order, or which is correct for any evaluation order.
Example:
let mut x = 0;
let a = {x = 1; 1} + x;
// Unclear whether a is 1 or 2.
Default level: Warn
What it does: Checks for explicit Clone
implementations 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: 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]);
for i in 0..v.len() { bar(i, v[i]); }
Default level: Warn
What it does: Checks for loops on y.into_iter()
where y
will do, and
suggests the latter.
Why is this bad? Readability.
Known problems: None
Example:
// with `y` a `Vec` or slice:
for x in y.into_iter() { .. }
Default level: Warn
What it does: Checks for loops on x.iter()
where &x
will do, and
suggests the latter.
Why is this bad? Readability.
Known problems: False negatives. We currently only warn on some known types.
Example:
// with `y` a `Vec` or slice:
for x in y.iter() { .. }
Default level: Deprecated
What it does: Nothing. This lint has been deprecated.
Deprecation reason: This used to check for Vec::extend
, which was slower than
Vec::extend_from_slice
. Thanks to specialization, this is no longer true.
Default level: Allow
What it does: Checks for usage of _.filter(_).map(_)
,
_.filter(_).flat_map(_)
, _.filter_map(_).flat_map(_)
and similar.
Why is this bad? Readability, this can be written more concisely as a single method call.
Known problems: Often requires a condition + Option/Iterator creation inside the closure.
Example:
iter.filter(|x| x == 0).map(|x| x * 2)
Default level: Warn
What it does: Checks for usage of _.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: Allow
What it does: Checks for float arithmetic.
Why is this bad? For some embedded systems or kernel development, it can be useful to rule out floating-point numbers.
Known problems: None.
Example:
a + 1.0
Default level: Warn
What it does: 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
y != x // where both are floats
Default level: Warn
What it does: Checks for iterating a map (HashMap
or BTreeMap
) and
ignoring 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: 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: 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: Checks for calls to std::mem::forget
with a reference
instead of an owned value.
Why is this bad? Calling forget
on a reference will only forget the
reference itself, which is a no-op. It will not forget the underlying referenced
value, which is likely what was intended.
Known problems: None.
Example:
let x = Box::new(1);
std::mem::forget(&x) // Should have been forget(x), x will still be dropped
Default level: Warn
What it does: Checks for use of .get().unwrap()
(or
.get_mut().unwrap
) on a standard library type which implements Index
Why is this bad? Using the Index trait ([]
) is more clear and more
concise.
Known problems: None.
Example:
let some_vec = vec![0, 1, 2, 3];
let last = some_vec.get(3).unwrap();
*some_vec.get_mut(0).unwrap() = 1;
The correct use would be:
let some_vec = vec![0, 1, 2, 3];
let last = some_vec[3];
some_vec[0] = 1;
Default level: Warn
What it does: 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:* Lint for redundant pattern matching over Result
or Option
Why is this bad? It's more concise and clear to just use the proper utility function
Known problems: None.
Example:
if let Ok(_) = Ok::<i32, i32>(42) {}
if let Err(_) = Err::<i32, i32>(42) {}
if let None = None::<()> {}
if let Some(_) = Some(42) {}
The more idiomatic use would be:
if Ok::<i32, i32>(42).is_ok() {}
if Err::<i32, i32>(42).is_err() {}
if None::<()>.is_none() {}
if Some(42).is_some() {}
Default level: Warn
What it does:* Checks for unnecessary ok()
in if let.
Why is this bad? Calling ok()
in if let is unnecessary, instead match on Ok(pat)
Known problems: None.
Example:
for result in iter {
if let Some(bench) = try!(result).parse().ok() {
vec.push(bench)
}
}
Could be written:
for result in iter {
if let Ok(bench) = try!(result).parse() {
vec.push(bench)
}
}
Default level: Allow
What it does: Checks for usage of !
or !=
in an if condition with an
else branch.
Why is this bad? Negations reduce the readability of statements.
Known problems: None.
Example:
if !v.is_empty() {
a()
} else {
b()
}
Could be written:
if v.is_empty() {
b()
} else {
a()
}
Default level: Warn
What it does: Checks for if/else
with the same body as the then part
and the else part.
Why is this bad? This is probably a copy & paste error.
Known problems: Hopefully none.
Example:
let foo = if … {
42
} else {
42
};
Default level: Warn
What it does: Checks for consecutive if
s with the same condition.
Why is this bad? This is probably a copy & paste error.
Known problems: Hopefully none.
Example:
if a == b {
…
} else if a == b {
…
}
Note that this lint ignores all conditions with a function call as it could have side effects:
if foo() {
…
} else if foo() { // not linted
…
}
Default level: Allow
What it does: Checks for usage of indexing or slicing.
Why is this bad? Usually, this can be safely allowed. However, in some domains such as kernel development, a panic can cause the whole operating system to crash.
Known problems: Hopefully none.
Example:
...
x[2];
&x[0..2];
Default level: Warn
What it does: 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:
if (x | 1 > 3) { … }
Default level: Warn
What it does: 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: Allow
What it does: Checks for plain integer arithmetic.
Why is this bad? This is only checked against overflow in debug builds. In some applications one wants explicitly checked, wrapping or saturating arithmetic.
Known problems: None.
Example:
a + 1
Default level: Deny
What it does: Checks regex creation (with Regex::new
,
RegexBuilder::new
or RegexSet::new
) for correct regex syntax.
Why is this bad? This will lead to a runtime panic.
Known problems: None.
Example:
Regex::new("|")
Default level: Allow
What it does: Checks for comparisons where the relation is always either true or false, but where one side has been upcast so that the comparison is necessary. Only integer types are checked.
Why is this bad? An expression like let x : u8 = ...; (x as u32) > 300
will mistakenly imply that it is possible for x
to be outside the range of
u8
.
Known problems: https://github.com/Manishearth/rust-clippy/issues/886
Example:
let x : u8 = ...; (x as u32) > 300
Default level: Allow
What it does: 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: 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: Checks for use of .iter().nth()
(and the related
.iter_mut().nth()
) on standard library types with O(1) element access.
Why is this bad? .get()
and .get_mut()
are more efficient and more
readable.
Known problems: None.
Example:
let some_vec = vec![0, 1, 2, 3];
let bad_vec = some_vec.iter().nth(3);
let bad_slice = &some_vec[..].iter().nth(3);
The correct use would be:
let some_vec = vec![0, 1, 2, 3];
let bad_vec = some_vec.get(3);
let bad_slice = &some_vec[..].get(3);
Default level: Warn
What it does: Checks for use of .skip(x).next()
on iterators.
Why is this bad? .nth(x)
is cleaner
Known problems: None.
Example:
let some_vec = vec![0, 1, 2, 3];
let bad_vec = some_vec.iter().skip(3).next();
let bad_slice = &some_vec[..].iter().skip(3).next();
The correct use would be:
let some_vec = vec![0, 1, 2, 3];
let bad_vec = some_vec.iter().nth(3);
let bad_slice = &some_vec[..].iter().nth(3);
Default level: Warn
What it does: Checks for large variants on enum
s.
Why is this bad? Enum size is bounded by the largest variant. Having a large variant can penalize the memory layout of that enum.
Known problems: None.
Example:
enum Test {
A(i32),
B([i32; 8000]),
}
Configuration: This lint has the following configuration variables:
-
enum-variant-size-threshold: u64
: The maximum size of a emum's variant to avoid box suggestion (defaults to200
).
Default level: Warn
What it does: 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 {
pub fn len(&self) -> usize { .. }
}
Default level: Warn
What it does: 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: 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: 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: 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: Checks for boolean expressions that contain terminals that can be eliminated.
Why is this bad? This is most likely a logic bug.
Known problems: Ignores short circuiting behavior.
Example:
if a && b || a { ... }
The b
is unnecessary, the expression is equivalent to if a
.
Default level: Warn
What it does: Checks for manual swapping.
Why is this bad? The std::mem::swap
function exposes the intent better
without deinitializing or copying either variable.
Known problems: None.
Example:
let t = b;
b = a;
a = t;
Default level: Warn
What it does: Checks for too many variables whose name consists of a single character.
Why is this bad? It's hard to memorize what a variable means without a descriptive name.
Known problems: None?
Example:
let (a, b, c, d, e, f, g) = (...);
Configuration: This lint has the following configuration variables:
-
single-char-binding-names-threshold: u64
: The maximum number of single char bindings a scope may have (defaults to5
).
Default level: Warn
What it does: Checks for mapping clone()
over an iterator.
Why is this bad? It makes the code less readable than using the
.cloned()
adapter.
Known problems: None.
Example:
x.map(|e| e.clone());
Default level: Warn
What it does: 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: 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: 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: 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: Checks for match
with identical arm bodies.
Why is this bad? This is probably a copy & paste error. If arm bodies
are the same on purpose, you can factor them
using |
.
Known problems: False positive possible with order dependent match
(see issue #860).
Example:
match foo {
Bar => bar(),
Quz => quz(),
Baz => bar(), // <= oops
}
This should probably be
match foo {
Bar => bar(),
Quz => quz(),
Baz => baz(), // <= fixed
}
or if the original code was not a typo:
match foo {
Bar | Baz => bar(), // <= shows the intent better
Quz => quz(),
}
Default level: Allow
What it does: Checks for usage of std::mem::forget(t)
where t
is Drop
.
Why is this bad? std::mem::forget(t)
prevents t
from running its
destructor, possibly causing leaks.
Known problems: None.
Example:
mem::forget(Rc::new(55)))
Default level: Warn
What it does: 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))
It 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: Checks for a op= a op b
or a op= b op a
patterns.
Why is this bad? Most likely these are bugs where one meant to write a op= b
.
Known problems: Someone might actually mean a op= a op b
, but that
should rather be written as a = (2 * a) op b
where applicable.
Example:
let mut a = 5;
...
a += a + b;
Default level: Allow
What it does: Warns if there is missing doc for any documentable item (public or private).
Why is this bad? Doc is good. rustc has a MISSING_DOCS
allowed-by-default lint for
public members, but has no way to enforce documentation of private items. This lint fixes that.
Known problems: None.
Default level: Warn
What it does: Warns on hexadecimal literals with mixed-case letter digits.
Why is this bad? It looks confusing.
Known problems: None.
Example:
let y = 0x1a9BAcD;
Default level: Warn
What it does: Checks for modules that have the same name as their parent module
Why is this bad? A typical beginner mistake is to have mod foo;
and again mod foo { .. }
in foo.rs
.
The expectation is that items inside the inner mod foo { .. }
are then
available
through foo::x
, but they are only available through foo::foo::x
.
If this is done on purpose, it would be better to choose a more
representative module name.
Known problems: None.
Example:
// lib.rs
mod foo;
// foo.rs
mod foo {
...
}
Default level: Warn
What it does: 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: Checks for instances of mut mut
references.
Why is this bad? Multiple mut
s don't add anything meaningful to the
source. This is either a copy'n'paste error, or it shows a fundamental
misunderstanding of references.
Known problems: None.
Example:
let x = &mut &mut y;
Default level: Warn
What it does: 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: 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: 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: Checks for address of operations (&
) that are going to
be dereferenced immediately by the compiler.
Why is this bad? Suggests that the receiver of the expression borrows the expression.
Known problems: None.
Example:
let x: &i32 = &&&&&&5;
Default level: Warn
What it does: 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: 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: 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: Checks for 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: Checks for multiplication by -1 as a form of negation.
Why is this bad? It's more readable to just negate.
Known problems: This only catches integers (for now).
Example:
x * -1
Default level: Warn
What it does: Checks for 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: Checks for types with a fn new() -> Self
method and no
implementation of
Default
.
Why is this bad? The user might expect to be able to use
Default
as the
type can be constructed without arguments.
Known problems: Hopefully none.
Example:
struct Foo(Bar);
impl Foo {
fn new() -> Self {
Foo(Bar::new())
}
}
Instead, use:
struct Foo(Bar);
impl Default for Foo {
fn default() -> Self {
Foo(Bar::new())
}
}
You can also have new()
call Default::default()
.
Default level: Warn
What it does: Checks for types with a fn new() -> Self
method
and no implementation of
Default
,
where the Default
can be derived by #[derive(Default)]
.
Why is this bad? The user might expect to be able to use
Default
as the
type can be constructed without arguments.
Known problems: Hopefully none.
Example:
struct Foo;
impl Foo {
fn new() -> Self {
Foo
}
}
Just prepend #[derive(Default)]
before the struct
definition.
Default level: Warn
What it does: 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: 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: Allow
What it does: Checks for boolean expressions that can be written more concisely.
Why is this bad? Readability of boolean expressions suffers from unnecessary duplication.
Known problems: Ignores short circuiting behavior of ||
and
&&
. Ignores |
, &
and ^
.
Example:
if a && true // should be: if a
if !(a == b) // should be: if a != b
Default level: Warn
What it does: 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: Checks for public functions that dereferences raw pointer arguments but are not marked unsafe.
Why is this bad? The function should probably be marked unsafe
, since
for an arbitrary raw pointer, there is no way of telling for sure if it is
valid.
Known problems:
- It does not check functions recursively so if the pointer is passed to a
private non-
unsafe
function which does the dereferencing, the lint won't trigger. - It only checks for arguments whose type are raw pointers, not raw pointers
got from an argument in some other way (
fn foo(bar: &[*const u8])
orsome_argument.get_raw_ptr()
).
Example:
pub fn foo(x: *const u8) { println!("{}", unsafe { *x }); }
Default level: Warn
What it does: Checks for usage of ok().expect(..)
.
Why is this bad? Because you usually call expect()
on the Result
directly to get a better error message.
Known problems: None.
Example:
x.ok().expect("why did I do this again?")
Default level: Allow
What it does: 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: Allow
What it does: Checks for usage of _.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: 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: 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 acting as the default.
Known problems: If the function has side-effects, not calling it will change the semantic of the program, but you shouldn't rely on that anyway.
Example:
foo.unwrap_or(String::new())
this can instead be written:
foo.unwrap_or_else(String::new)
or
foo.unwrap_or_default()
Default level: Deny
What it does: Checks 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];
&x[2..9];
Default level: Warn
Default level: Warn
What it does: Checks for missing parameters in panic!
.
Why is this bad? Contrary to the format!
family of macros, there are
two forms of panic!
: if there are no parameters given, the first argument
is not a format string and used literally. So while format!("{}")
will
fail to compile, panic!("{}")
will not.
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: Checks for manual re-implementations of PartialEq::ne
.
Why is this bad? PartialEq::ne
is required to always return the
negated result of PartialEq::eq
, which is exactly what the default
implementation does. Therefore, there should never be any need to
re-implement it.
Known problems: None.
Example:
struct Foo;
impl PartialEq for Foo {
fn eq(&self, other: &Foo) -> bool { ... }
fn ne(&self, other: &Foo) -> bool { !(self == other) }
}
Default level: Warn
What it does: 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? 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.
Example:
-
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: Checks for printing 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 warns when you using print!()
with a format string that
ends in a newline.
Why is this bad? You should use println!()
instead, which appends the newline.
Known problems: None.
Example:
print!("Hello {}!\n", name);
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: Allow
What it does: Detects enumeration variants that are prefixed or suffixed by the same characters.
Why is this bad? Enumeration variant names should specify their variant, not repeat the enumeration name.
Known problems: None.
Example:
enum Cake {
BlackForestCake,
HummingbirdCake,
}
Default level: Warn
What it does: 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: 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: 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 adds code for no benefit and gives the optimizer more work.
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: Detects closures called in the same expression where they are defined.
Why is this bad? It is unnecessarily adding to the expression's complexity.
Known problems: None.
Example:
(|| 42)()
Default level: Warn
What it does: 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: 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: 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: 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.
Example:
for x in 5..10-5 { .. } // oops, stray `-`
Default level: Warn
What it does: Checks for 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: Warn
What it does: Checks for mis-uses of the serde API.
Why is this bad? Serde is very finnicky about how its API should be used, but the type system can't be used to enforce it (yet).
Known problems: None.
Example: Implementing Visitor::visit_string
but not Visitor::visit_str
.
Default level: Allow
What it does: 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: 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: 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: Checks for the use of short circuit boolean conditions as a statement.
Why is this bad? Using a short circuit boolean condition as a statement may hide the fact that the second part is executed or not depending on the outcome of the first part.
Known problems: None.
Example:
f() && g(); // We should write `if f() { g(); }`.
Default level: Warn
What it does: 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: Allow
What it does: Checks for names that are very similar and thus confusing.
Why is this bad? It's hard to distinguish between names that differ only by a single character.
Known problems: None?
Example:
let checked_exp = something;
let checked_expr = something_else;
Default level: Warn
What it does: 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: 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: 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: Deprecated
What it does: Nothing. This lint has been deprecated.
Deprecation reason: This used to check for .to_string()
method calls on values
of type &str
. This is not unidiomatic and with specialization coming, to_string
could be
specialized to be as efficient as to_owned
.
Default level: Allow
What it does: Checks for 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: Checks for string appends 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
Default level: Warn
What it does: Checks for 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()
.
Known Problems: None.
Example:
let bs = "a byte string".as_bytes();
Default level: Deprecated
What it does: Nothing. This lint has been deprecated.
Deprecation reason: This used to check for .to_string()
method calls on values
of type String
. This is not unidiomatic and with specialization coming, to_string
could be
specialized to be as efficient as clone
.
Default level: Allow
What it does: Detects type names that are prefixed or suffixed by the containing module's name.
Why is this bad? It requires the user to type the module name twice.
Known problems: None.
Example:
mod cake {
struct BlackForestCake;
}
Default level: Warn
What it does: Checks for use of the non-existent =*
, =!
and =-
operators.
Why is this bad? This is either a typo of *=
, !=
or -=
or confusing.
Known problems: None.
Example:
a =- 42; // confusing, should it be `a -= 42` or `a = -42`?
Default level: Warn
What it does: Checks for formatting of else if
. It lints if the else
and if
are not on the same line or the else
seems to be missing.
Why is this bad? This is probably some refactoring remnant, even if the code is correct, it might look confusing.
Known problems: None.
Example:
if foo {
} if bar { // looks like an `else` is missing here
}
if foo {
} else
if bar { // this is the `else` block of the previous `if`, but should it be?
}
Default level: Warn
What it does: 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: Checks for getting the inner pointer of a temporary CString
.
Why is this bad? The inner pointer of a CString
is only valid as long
as the CString
is alive.
Known problems: None.
Example:
let c_str = CString::new("foo").unwrap().as_ptr();
unsafe {
call_some_ffi_func(c_str);
}
Here c_str
point to a freed address. The correct use would be:
let c_str = CString::new("foo").unwrap();
unsafe {
call_some_ffi_func(c_str.as_ptr());
}
Default level: Warn
What it does: Checks for functions with too many parameters.
Why is this bad? Functions with lots of parameters are considered bad style and reduce readability (“what does the 5th parameter mean?”). Consider grouping some parameters into a new type.
Known problems: None.
Example:
fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) { .. }
Configuration: This lint has the following configuration variables:
-
too-many-arguments-threshold: u64
: The maximum number of argument a function or method can have (defaults to7
).
Default level: Warn
What it does: 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: Checks for transmutes from a pointer to a reference.
Why is this bad? This can always be rewritten with &
and *
.
Known problems: None.
Example:
let _: &T = std::mem::transmute(p); // where p: *const T
// can be written:
let _: &T = &*p;
Default level: Warn
What it does: Checks for trivial regex creation (with Regex::new
,
RegexBuilder::new
or RegexSet::new
).
Why is this bad? Matching the regex 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: 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)>>>> }
Configuration: This lint has the following configuration variables:
-
type-complexity-threshold: u64
: The maximum complexity a type can have (defaults to250
).
Default level: Allow
What it does: 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: 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: 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: Checks for expression statements that can be reduced to a sub-expression.
Why is this bad? Expressions by themselves often have no side-effects. Having such expressions reduces readability.
Known problems: None.
Example:
compute_array()[0];
Default level: Warn
What it does: 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: Checks for imports that remove "unsafe" from an item's name.
Why is this bad? Renaming makes it less clear which traits and structures are unsafe.
Known problems: None.
Example:
use std::cell::{UnsafeCell as TotallySafeCell};
extern crate crossbeam;
use crossbeam::{spawn_unsafe as spawn};
Default level: Allow
What it does: Warns if literal suffixes are not separated by an underscore.
Why is this bad? It is much less readable.
Known problems: None.
Example:
let y = 123832i32;
Default level: Deprecated
What it does: Nothing. This lint has been deprecated.
Deprecation reason: This used to check for Vec::as_mut_slice
, which was unstable with good
stable alternatives. Vec::as_mut_slice
has now been stabilized.
Default level: Deprecated
What it does: Nothing. This lint has been deprecated.
Deprecation reason: This used to check for Vec::as_slice
, which was unstable with good
stable alternatives. Vec::as_slice
has now been stabilized.
Default level: Warn
What it does: 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: Deny
What it does: Checks for unused written/read amount.
Why is this bad? io::Write::write
and io::Read::read
are not guaranteed to
process the entire buffer. They return how many bytes were processed, which might be smaller
than a given buffer's length. If you don't need to deal with partial-write/read, use
write_all
/read_exact
instead.
Known problems: Detects only common patterns.
Example:
use std::io;
fn foo<W: io::Write>(w: &mut W) -> io::Result<()> {
// must be `w.write_all(b"foo")?;`
w.write(b"foo")?;
Ok(())
}
Default level: Warn
What it does: Checks for unused labels.
Why is this bad? Maybe the label should be used in which case there is an error in the code or it should be removed.
Known problems: Hopefully none.
Example:
fn unused_label() {
'label: for i in 1..2 {
if i > 4 { continue }
}
Default level: Warn
What it does: 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: Checks for use of 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: Allow
What it does: 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: The lint does not work properly with desugaring and macro, it has been allowed in the mean time.
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: Checks for extern crate
and use
items annotated with lint attributes
Why is this bad? Lint attributes have no effect on crate imports. Most likely a !
was
forgotten
Known problems: Technically one might allow unused_import
on a use
item,
but it's easier to remove the unused item.
Example:
#[deny(dead_code)]
extern crate foo;
#[allow(unused_import)]
use foo::bar;
Default level: Warn
What it does: Checks for the use of format!("string literal with no argument")
and format!("{}", foo)
where foo
is a string.
Why is this bad? There is no point of doing that. format!("too")
can
be replaced by "foo".to_owned()
if you really need a String
. The even
worse &format!("foo")
is often encountered in the wild. format!("{}", foo)
can be replaced by foo.clone()
if foo: String
or foo.to_owned()
if foo: &str
.
Known problems: None.
Examples:
format!("foo")
format!("{}", foo)
Default level: Warn
What it does: Checks for variable declarations immediately followed by a conditional affectation.
Why is this bad? This is not idiomatic Rust.
Known problems: None.
Example:
let foo;
if bar() {
foo = 42;
} else {
foo = 0;
}
let mut baz = None;
if bar() {
baz = Some(42);
}
should be written
let foo = if bar() {
42
} else {
0
};
let baz = if bar() {
Some(42)
} else {
None
};
Default level: Warn
What it does: Checks for transmutes to the original type of the object and transmutes that could be a cast.
Why is this bad? Readability. The code tricks people into thinking that something complex is going on.
Known problems: None.
Example:
core::intrinsics::transmute(t) // where the result type is the same as `t`'s
Default level: Warn
What it does: Checks for usage of &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: 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: 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: 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: Checks for transmutes that can't ever be correct on any architecture.
Why is this bad? It's basically guaranteed to be undefined behaviour.
Known problems: When accessing C, users might want to store pointer
sized objects in extradata
arguments to save an allocation.
Example:
let ptr: *const T = core::intrinsics::transmute('x')`
Default level: Warn
What it does: 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: Warn
What it does: Warns if an integral constant literal starts with 0
.
Why is this bad? In some languages (including the infamous C language and most of its familly), this marks an octal constant. In Rust however, this is a decimal constant. This could be confusing for both the writer and a reader of the constant.
Known problems: None.
Example:
In Rust:
fn main() {
let a = 0123;
println!("{}", a);
}
prints 123
, while in C:
#include <stdio.h>
int main() {
int a = 0123;
printf("%d\n", a);
}
prints 83
(as 83 == 0o123
while 123 == 0o173
).
Default level: Deny
What it does: 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.