| Age | Commit message (Collapse) | Author |
|---|
|
Instead of:
if text.starts_with(prefix) {
&text[prefix.len()..]
} else {
&text
}
use this:
if let Some(rest) = text.strip_prefix(prefix) {
rest
} else {
&text
}
The strip_prefix is easier to understand, and also removes the
dependency between the if condition and the computation to get the
slice after the prefix. The dependency is reasonably clear, but the
compiler does not understand it so it's plausible that a future change
might change the condition but not the slice. The approach using
strip_prefix avoids that.
This was found by the clippy lint manual_strip:
https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip
|
|
A chain of if statements is not guaranteed to test every possible
case, the way match does. Thus it seems better to use match here, when
there are three possible results from a comparison.
Found by clippy lint comparison_chain:
https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
|
|
We should arguably use a different approach, but I don't understand
the code enough to suggest how.
Found by clippy lint transmute_ptr_to_ptr:
https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr
|
|
We have two loops where the last statement is a return or a break,
without there being a continue. This means the loops will never loop.
This is a symptom of the code being hard to understand. We should
probably rewrite the logic without a loop, but I did not feel
confident doing that yet.
Found by clippy lint never_loop:
https://rust-lang.github.io/rust-clippy/master/index.html#never_loop
|
|
A complex type without a name tends to be harder to understand. We
have a few of them. Using "type" to give them a name would hopefully
help, but I don't understand the code well enough to come up with
meaningful names, and meaningless names won't help. Also, arguably, a
struct or enum would often be the better approach, but would change
the API. Thus, I'm only configuring clippy to allow complex types. We
should later revisit this and improve the code, when we're making
other API changes anyway.
Found by clippy lint type_complexity:
https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
|
|
By default, clippy allows up to seven arguments to a function.
Anything more and it will complain.
https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
Seven is already a very large number. However, changing the number of
arguments in public functions would change the API of the library, so
I've upped the limit to ten, which is enough to placate clippy.
Arguably, when we are changing the API anyway, we should consider ways
to remove the number of arguments to functions, possibly by using the
builder pattern.
|
|
Generally speaking, single-character names are not great for the
person reading the code later. They don't usually act as a cognitive
aid to understand the code. However, this in code implementing
cryptographic operations that implements mathematical formulas that
canonically use single-letter names it's clearer to use the same name
in the code. Thus, I only tell clippy those names are OK in these
cases.
Found by clippy lint many_single_char_names:
https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
|
|
clippy warns if one of the variants of an enum is significantly larger
than the others, as that can cause surprising memory use: an enum uses
as much space as its largest variant. We have some such enums, but
changing them to have variants of similar size would change the API so
I've chosen to allow variants up to 512 bytes.
Found by clippy lint large_enum_variant:
https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
|
|
An if condition is an expression and can be as complex as the
programmer wants. However, the more complex a condition is, the harder
it tends to be to understand. I marked functions with such if
conditions so that clippy won't complain about the code. I probably
should have simplified the code, perhaps by extracting the condition
to its own function, but it would have been much harder to do, so I
didn't.
Found by clippy lint blocks_in_if_conditions:
https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
|
|
Arguably this code should be rewritten to avoid needing "unsafe", but
this is the minimal change for reassuring clippy.
Found by clippy lint ptr_offset_with_cast:
https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
|
|
Instead of giving Ok() the return value of a function that returns no
value, i.e., the unit value (), give it explicitly. This is clearer
and easier to follow for the reader.
Found by clippy lint unit_arg:
https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
|
|
It seems clearer to write this with "if let".
Found by clippy lint option_map_unit_fn:
https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
|
|
In this code:
self.user_attributes.retain(|_| (keep[i], i += 1).0);
it can be unclear to the reader that the increment of i actually
happens before keep is indexed. Especially so for people who've been
inflicted by C and its many surprising reasons for undefined behavior.
It seems better to write this using an iterator.
Found by clippy lint eval_order_dependence:
https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
|
|
Instead of this:
foo.as_ref().map(|x| x.as_slice())
do this:
foo.as_deref()
It's shorter and more to the point, and should thus be easier to
understand.
Found by clippy lint option_as_ref_deref:
https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
|
|
Certain method names are typically provided by traits, and it can be
confusing to readers when a method uses that name without the type
implementing the trait. Mark the cases we have to tell clippy these
cases are OK. Implementing the corresponding traits would have changed
API so I opted not to do that.
Found by clippy lint new_without_default:
https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
|
|
It is customary in Rust to implement the Default trait for types that
can have a new method that takes no arguments. However, that's not
always wanted.
I've marked all the structures that have a new without arguments but
don't implement Default, so that if we get more of them, clippy will
warn.
Found by clippy lint new_without_default:
https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
|
|
It is Rust custom that the new method for a type returns an instance
of that type. However, sometimes that's not wanted. Tell clippy that
these cases are OK. I opted to not do this globally, because that
would prevent clippy from catching future cases.
Found by clippy warning new_ret_no_self:
https://rust-lang.github.io/rust-clippy/master/index.html#new_ret_no_self
|
|
It is customary for Rust data container structures to have both
is_empty and len, methods, because is_empty can both more to the point
and also faster to implement.
Sometimes is_empty makes no sense, as it doesn't for
openpgp::types::Curve, which isn't a container structure. Arguably it
would be better to rename Curve::len to something like Curve::bits,
but as that would change the API, I've opted to tell clippy that not
having is_empty in this case is OK.
Found by clippy lint len_without_is_empty:
https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
|
|
Found by clippy lint let_and_return:
https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
|
|
The methods were shadowing the implementation of the same function via
the Display trait. One implementation is enough.
Found by the clippy trait inherent_to_string_shadow_display:
https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display
|
|
Instead of this:
if foo {
if bar {
...
}
}
do this:
if foo && bar {
...
}
Nesting statements implies a more complicated code structure than it
really is. Thus it's arguably simpler to write a combined condition by
joining the two conditions with a logical and operation.
Found by clippy lint collapsible_if:
https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
|
|
Instead of using a variable named foo, which usually a placeholder,
use a more descriptive name.
|
|
If a closure just passes the argument it gets to a function, the
closure is usually redundant and you can just use the function instead
of the closure. Thus, instead of this:
iter().map(|x| foo(x))
you can write this:
iter().map(foo)
This is shorter and simpler.
Sometimes it doesn't work and the closure is necessary. Such locations
can be marked with `#[allow(clippy::redundant_closure)]`.
Found by clippy lint redundant_closure:
https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
|
|
A stable sort is one where values that compare equal are in the same
order relative to each other in the output as they were in the input.
Thus, if input has values a0, b, and a1, with a0==a1, a0<b, a1<b, then
the output of a stable sort is guaranteed to be be [a0, a1, b]. For an
unstable sort, the output may also be [a1, a0, b]. This can matter if,
for example, the values are structs and only one field is used for
comparison.
On data where an unstable sort (.sort_unstable) works, it's bad to use
a stable sort (.sort), because it uses more CPU without affecting the
end result.
If the types of values in a vector are primitive types, as they are in
the cases changed by this commit, an unstable sort will do just fine.
Found by clippy lint stable_sort_primitive:
https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
|
|
If the function or closure passed into .filter_map doesn't do any
filtering or mapping. the method call can be replaced with a simple
.map or .filter. The clippy lint suggests which. In this change, we
can always replace .filter_map with .filter. We also need to change
the closure to return a boolean value, since that's what .filter
expects.
Found by clippy lint unnecessary_filter_map:
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
|
|
Instead of this:
something.iter().cloned().collect()
do this:
something.to_vec()
It's shorter, simpler, and more to the point, and thus easier to
understand.
Found by clippy lint iter_cloned_collect:
https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect
|
|
Test code embedded in document comments do not need to have a main
function, the Rust test infrastructure provides one. Keeping the test
code shorter is more idiomatic and shorter, both of which make it
easier to understand.
Found by clippy lint needless_doctest_main:
https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
|
|
Just call the .iter() method instead of .into_iter() when either is
OK. The shorter form is shorter and should thus be easier to read.
Found by clippy lint into_iter_on_ref:
https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_on_ref
|
|
Instead of this:
if let Some(()) = key.secret_mut().decrypt_in_place(algo, &p).ok() { ... }
use this:
if key.secret_mut().decrypt_in_place(algo, &p).is_ok() { ... }
It's more to the point and easier to understand.
|
|
Instead of
iter().filter(|x| pred(x)).next()
use this:
iter().find(|x| pred(x))
This is more to the point, and thus easier to understand.
Found by clippy lint filter_next:
https://rust-lang.github.io/rust-clippy/master/index.html#filter_next
|
|
Instead of this:
name.chars().next() == Some('!')
use this:
name.starts_with('!')
It's more to the point, and does not require the reader to reason
about iterators and possible values returneed by .next().
Found by clippy lint chars_next_cmp:
https://rust-lang.github.io/rust-clippy/master/index.html#chars_next_cmp
|
|
Instead of:
foo().and_then(|x| Ok(x.transmogrify())
use this:
foo().map(|x| x.transmogrify)
because it's shorter and simpler to understand.
Suggested by clippy lint bind_instead_of_map:
https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
|
|
In a match arm, instead of binding the matched value to the name "_",
just don't bind. Its shorter and easier to understand
Found by clippy lint redundant_patter:
https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
|
|
Rather than using magic numbers, use names to express the various
values.
|
|
When creating a struct with a field foo, using a variable also named
foo, it's not necessary to name the field explicitly. Thus, instead
of:
Self { foo: foo }
use this:
Self { foo }
The shorter form is more idiomatic and thus less confusing to
experienced Rust programmers.
This was found by the clippy lint redundant_field_names:
https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
Sponsored-by: author
|
|
This code can probably be rewritten to not cause the warning, but as
it's crypto code, I fear making a mistake, so I'm just disabling the
warning here.
Found by clippy lint same_item_push:
https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
|
|
The dedicated method on iterators is shorter and more to the point,
and should thus be easier to understand.
Found by clippy lint map_clone:
https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
|
|
It's arguably simpler to write a format string that doesn't take
arguments than one with an argument that looks like a format string.
Found by clippy lint write_literal:
https://rust-lang.github.io/rust-clippy/master/index.html#write_literal
|
|
Reduce this:
foo(&mut bar):
into
foo(&bar);
when the function does not expect a mutable reference, just a
read-only reference. The extra mut serves no function, but can confuse
the reader.
Found by clippy lint unnecessary_mut_passed:
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
|
|
Reduce
&mut foo[...].copy_from_slice(bar);
to just
foo[...].copy_from_slice(bar);
since the &mut does nothing except obfuscate the code.
Found by clippy lint unnecessary_operation:
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
|
|
The code used to do foo.into(), but the result was of the same type as
foo already was. Thus the extra call is redundant and likely to
confuse a reader.
Found by clippy lint useless_conversion:
https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
|
|
The if let is more idiomatic and thus should be easier for Rust
programmers to understand. It also avoids to implicit dependency
between the if condition and the unwrap.
Found by clippy lint unnecessary_unrap:
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
|
|
This was found by clippy lint op_ref:
https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
|
|
This works in Rust now:
foo?;
as a replacement for:
if foo.is_none() {
return None;
}
It's similar to ? for error handling and can only be used in functions
that return an Option.
The instance of this in a macro caused the problem to be reported a
lot of time, once per time the macro was used, but luckily it can
could be fixed in only one place. Magic!
Found by clippy lint question_mark:
https://rust-lang.github.io/rust-clippy/master/index.html#question_mark
|
|
Suggested by Neal Walfield.
Found by clippy lint useless_format:
https://rust-lang.github.io/rust-clippy/master/index.html#useless_format
|
|
Instead of
Err(...)?;
which is correct but not idiomatic, write this:
return Err(...);
This was found by clippy lintt try_err:
https://rust-lang.github.io/rust-clippy/master/index.html#try_err
Sponsored-by: author
|
|
Found by clippy lint nonminimal_bool:
https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
|
|
Replace
let bar = std::mem::replace(&foo, Default::Default());
with
let bar = std::mem::take(&foo);
The new version seems a little clearer. Found by clippy:
https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default
Sponsored-by: author
|
|
Instead of "return foo" as the last statement executed in a function,
just use "foo", which is more idiomatic style.
This was found by the clippy lint needless_return:
https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
|
|
This was found by clippy lint len_zero:
https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy
|
