GitLab

At the time of writing, running cargo clippy on master results in 85 warnings and 5 errors - many of which indicate sub-optimal code or even potential bugs. Allowing clippy warnings past CI causes three major problems:

1. It makes running clippy basically useless for developers who want to see if they've introduced any clippy warnings in their own code since it's impossible to filter out all of the existing warnings.
2. Following on from 1), Fixing broken windows comes into play - the more clippy warnings that exist in the codebase, the more likely it is that new ones will be introduced in further commits.
3. Some of the warnings represent genuine potential bugs

I've done a quick cleanup in a commit here which shows the kind of minor changes needed to fix 50% of the existing clippy warnings (this branch has 43 warnings 5 errors): 6762d354

Most of the remaining warnings probably need a bit more of an in-depth investigation since the fixes for some potentially have side effects such as replacing unwrap_or with unwrap_or_else.

In my opinion the best route to zero clippy warnings would be the following steps:

• MR the above commit to deal with the simple fixes
• Create a tracking issue for the remaining clippy warnings/errors
• Add #![allow(clippy::<lint_name>)] // Pending review in #587 to suppress the remaining clippy warnings
• Now that there are zero clippy warnings/errors, modify check-compile.gitlab-ci.yml to make clippy warnings fail the build by removing -- --warn clippy::all from the cargo clippy line
• Fix the remaining suppressed warnings over time by either refactoring or making a decision to permanently leave the suppression where appropriate
• Add a page to the Book explaining Clippy policy

Apologies if this proves to be a contentious suggestion from someone who's barely been a contributor for 24 hours - I believe it would have a positive impact on code quality with little downside though :)

Explanations of clippy lints can be found here for reference: https://rust-lang.github.io/rust-clippy/master/index.html

Remaining suppressed warnings with TODO commment

Clippy Lint	Instances	MR
type_complexity	39
useless_conversion	35	!1198 (merged)
or_fun_call	30
new_without_default	27
option_map_unit_fn	26	!1071 (merged)
redundant_closure	20	!1065 (merged)
collapsible_if	20	!1064 (merged)
too_many_arguments	19
unused_unit	18
many_single_char_names	12	!1066 (merged)
float_cmp	11
clone_on_copy	10	!1092 (merged)
single_match	10
identity_op	9
useless_format	8
needless_range_loop	8
if_same_then_else	7
assign_op_pattern	7
blocks_in_if_conditions	7
into_iter_on_ref	7
large_enum_variant	7
let_and_return	7
approx_constant	5
manual_saturating_arithmetic	5
excessive_precision	5
needless_update	5
redundant_clone	4
len_without_is_empty	4
match_single_binding	4
expect_fun_call	3
op_ref	3
unit_arg	3
needless_return	3
nonminimal_bool	3
len_zero	2
neg_multiply	2
drop_copy	2
erasing_op	2
redundant_pattern_matching	2
cmp_owned	2
extra_unused_lifetimes	2
chars_next_cmp	2
unnecessary_operation	2
zero_ptr	2
needless_lifetimes	2
while_let_on_iterator	2
unused_io_amount	1
logic_bug	1
match_ref_pats	1
for_loops_over_fallibles	1
unnecessary_mut_passed	1
map_clone	1
eval_order_dependence	1
modulo_one	1
toplevel_ref_arg	1
map_entry	1
option_as_ref_deref	1
redundant_static_lifetimes	1
int_plus_one	1
single_char_pattern	1
filter_next	1
single_component_path_imports	1
useless_let_if_seq	1
match_bool	1
panic_params	1
print_literal	1
blacklisted_name	1
same_item_push	1
unnested_or_patterns	?
bind_instead_of_map	?