Apply some clippy lints #11154

hirschenberger · 2022-04-02T11:46:41Z

Summary

The intention of this PR is not to enable/enforce a set of clippy rules to CI. It is making it easier to do so by fixing a lot of clippy remarks so that maybe @gilescope can integrate more clippy lints step by step without giant changesets.

This PR involved a huge amount of yak-shaving activity (TBH. I underestimated the amount and I would not do it again). I tried to evaluate each change (or kind of change) and hope I found a good compromise. I know that others would have drawn the line somewhere else.

Unfortunately it was not feasable to create an extra commit for all different lints, as it would have increased the amount of work drastically. So I hope you can live with it and review the PR as a whole. Sorry for the reviewers. I'll sure change your suggestions and remarks.

I tried to keep an eye on API-compatibility. The only thing I changed on function signatures is removing lifetime-annotations that can be elided.

I created a list of lints that fall into three categories:

Nearly always applied
Applied sometimes depending on the code-context
Never applied

You can lookup these lints and what they do here: Clippy Lints

Nearly always applied

clone_on_copy: very frequent, maybe not really necessary performance-wise but reduces a lot of line-noise
needless_lifetimes: the compiler got smarter over the years
mem_replace_with_default
unwrap_or_else_default
redundant_pattern_matching
redundant_static_lifetimes
collapsible_else_if
single_match
manual_saturating_arithmetic
needless_borrow: a lot of them, changed them for type clarity
redundant_closure: also many of them, I hope it reduces codesize a bit and speeds up compilaton a bit
map_collect_result_unit
try_err
iter_cloned_collect
unnecessary_lazy_evaluations
mem_replace_option_with_none
match_ref_pats
into_iter_on_ref
field_reassign_with_default

Applied sometimes depending on the code-context

len_zero: I only applied it in the empty case: if map.is_empty() { not in the not empty case: if !map.is_empty() { for readability. In my opinion if map.len() > 0 { is easier to grasp.
needless_return: Sometimes an explicit return makes the code more clear
collapsible_if
option_map_unit_fn
vec_init_then_push
comparison_chain: Subsituting an if-elseif-else chain with a match makes the code less readable when no real matching but if condittions are involved.
or_fun_call Sometimes the fun is really trivial and maybe const like Zero::zero() and should be inlined nevertheless
let_and_return There was some false alarms where clippy could not resolve a macro between let and return.
match_like_matches_macro
nonminimal_bool

Never applied

result_unit_err
wrong_self_convention API
len_without_is_empty API
new_without_default API
unit_arg
eval_order_dependence false alarm due to macros
should_implement_trait API
from_over_into API
borrowed_box API
unused_unit API
enum_variant_names API
module_inception API

polkadot address: 1ZApdbwrNnT1Hey4xeTAH6SZPDgbA9ZVftNuu9jTRbkc2Bg

bkchr

I did not yet review all changes.

client/cli/src/commands/generate.rs

client/cli/src/commands/inspect_key.rs

client/cli/src/commands/vanity.rs

client/db/src/bench.rs

client/service/src/client/block_rules.rs

client/peerset/src/peersstate.rs

client/network/src/transactions.rs

client/network/src/protocol.rs

Co-authored-by: Bastian Köcher <[email protected]>

# Conflicts: # client/service/src/client/wasm_override.rs # primitives/state-machine/src/trie_backend_essence.rs # utils/fork-tree/src/lib.rs # utils/frame/benchmarking-cli/src/pallet/command.rs # utils/frame/remote-externalities/src/lib.rs

hirschenberger · 2022-04-14T13:00:44Z

I'll try to keep this PR in sync with master, but I suspect it to conflict frequently due to it's sheer size.

bin/node/bench/src/txpool.rs

gilescope · 2022-04-28T14:13:46Z

@bkchr any concerns or is all good?

bkchr · 2022-04-28T15:51:09Z

@bkchr any concerns or is all good?

I hope to finish the review tomorrow.

bkchr

Sorry for the delay @hirschenberger

Looks good and ty for your work :)

utils/frame/benchmarking-cli/src/shared/stats.rs

utils/frame/benchmarking-cli/src/pallet/command.rs

bkchr · 2022-04-30T09:54:29Z

/tip small

bkchr · 2022-04-30T09:56:05Z

/tip small

substrate-tip-bot · 2022-04-30T09:56:07Z

Please fix the following problems before calling the tip bot again:

Contributor did not properly post their Polkadot or Kusama address.

Make sure the pull request description has: "{network} address: {address}".

Co-authored-by: Bastian Köcher <[email protected]>

# Conflicts: # client/network/src/protocol/notifications/behaviour.rs # client/network/src/service.rs

shawntabrizi · 2022-04-30T13:11:46Z

@hirschenberger if you would like a treasury tip, please include your address information above

hirschenberger · 2022-04-30T18:13:37Z

@hirschenberger if you would like a treasury tip, please include your address information above

Ok, thanks

bkchr · 2022-04-30T21:28:09Z

/tip small

bkchr · 2022-04-30T21:28:20Z

bot merge

hirschenberger · 2022-05-01T08:30:12Z

/tip small

@bkchr The tip-bot seems to be sleeping...

shawntabrizi · 2022-05-01T09:37:05Z

/tip small

substrate-tip-bot · 2022-05-01T09:37:09Z

A small tip was successfully submitted for hirschenberger (1ZApdbwrNnT1Hey4xeTAH6SZPDgbA9ZVftNuu9jTRbkc2Bg on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/treasury/tips

* Apply some clippy hints * Revert clippy ci changes * Update client/cli/src/commands/generate.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/cli/src/commands/inspect_key.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/db/src/bench.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/db/src/bench.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/service/src/client/block_rules.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/service/src/client/block_rules.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/network/src/transactions.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/network/src/protocol.rs Co-authored-by: Bastian Köcher <[email protected]> * Revert due to missing `or_default` function. * Fix compilation and simplify code * Undo change that corrupts benchmark. * fix clippy * Update client/service/test/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/state-db/src/noncanonical.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/state-db/src/noncanonical.rs remove leftovers! * Update client/tracing/src/logging/directives.rs Co-authored-by: Bastian Köcher <[email protected]> * Update utils/fork-tree/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * added needed ref * Update frame/referenda/src/benchmarking.rs * Simplify byte-vec creation * let's just not overlap the ranges * Correction * cargo fmt * Update utils/frame/benchmarking-cli/src/shared/stats.rs Co-authored-by: Bastian Köcher <[email protected]> * Update utils/frame/benchmarking-cli/src/pallet/command.rs Co-authored-by: Bastian Köcher <[email protected]> * Update utils/frame/benchmarking-cli/src/pallet/command.rs Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Giles Cope <[email protected]>

shawntabrizi · 2022-10-09T19:59:49Z

This PR changed sort to sort_unstable which is something we want to actively avoid.

See: #6754

This is how we had a consensus problem on Kusama.

cc @gilescope can you make sure that such linting in the future does not do this?

* Apply some clippy hints * Revert clippy ci changes * Update client/cli/src/commands/generate.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/cli/src/commands/inspect_key.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/db/src/bench.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/db/src/bench.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/service/src/client/block_rules.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/service/src/client/block_rules.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/network/src/transactions.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/network/src/protocol.rs Co-authored-by: Bastian Köcher <[email protected]> * Revert due to missing `or_default` function. * Fix compilation and simplify code * Undo change that corrupts benchmark. * fix clippy * Update client/service/test/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/state-db/src/noncanonical.rs Co-authored-by: Bastian Köcher <[email protected]> * Update client/state-db/src/noncanonical.rs remove leftovers! * Update client/tracing/src/logging/directives.rs Co-authored-by: Bastian Köcher <[email protected]> * Update utils/fork-tree/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * added needed ref * Update frame/referenda/src/benchmarking.rs * Simplify byte-vec creation * let's just not overlap the ranges * Correction * cargo fmt * Update utils/frame/benchmarking-cli/src/shared/stats.rs Co-authored-by: Bastian Köcher <[email protected]> * Update utils/frame/benchmarking-cli/src/pallet/command.rs Co-authored-by: Bastian Köcher <[email protected]> * Update utils/frame/benchmarking-cli/src/pallet/command.rs Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Giles Cope <[email protected]>

hirschenberger added 2 commits April 2, 2022 10:04

Apply some clippy hints

eb6c135

Merge remote-tracking branch 'origin/master' into issue_11054

e55308a

hirschenberger requested review from shawntabrizi, kianenigma, athei, acatangiu, sorpaas and andresilva as code owners April 2, 2022 11:46

hirschenberger changed the title ~~Issue 11054~~ Apply some clippy lints Apr 2, 2022

Revert clippy ci changes

0399809

bkchr reviewed Apr 2, 2022

View reviewed changes

hirschenberger and others added 12 commits April 3, 2022 00:21

Update client/cli/src/commands/generate.rs

beca8c8

Co-authored-by: Bastian Köcher <[email protected]>

Update client/cli/src/commands/inspect_key.rs

2e28e4e

Co-authored-by: Bastian Köcher <[email protected]>

Update client/db/src/bench.rs

a14e027

Co-authored-by: Bastian Köcher <[email protected]>

Update client/db/src/bench.rs

1a0b11d

Co-authored-by: Bastian Köcher <[email protected]>

Update client/service/src/client/block_rules.rs

e072cf3

Co-authored-by: Bastian Köcher <[email protected]>

Update client/service/src/client/block_rules.rs

3f22750

Co-authored-by: Bastian Köcher <[email protected]>

Update client/network/src/transactions.rs

8892ce0

Co-authored-by: Bastian Köcher <[email protected]>

Update client/network/src/protocol.rs

0ded163

Co-authored-by: Bastian Köcher <[email protected]>

Revert due to missing or_default function.

3314718

Fix compilation and simplify code

492f365

Merge remote-tracking branch 'origin/master' into issue_11054

818b944

Merge branch 'master' into issue_11054

442faa2

gilescope reviewed Apr 14, 2022

View reviewed changes

bin/node/bench/src/txpool.rs Outdated Show resolved Hide resolved

gilescope added 5 commits April 20, 2022 11:58

Correction

24bc48c

Merge remote-tracking branch 'upstream/master' into issue_11054

635fb92

Merge remote-tracking branch 'upstream/master' into issue_11054

ac98fb8

Merge remote-tracking branch 'upstream/master' into issue_11054

91abc4c

cargo fmt

d530cce

bkchr approved these changes Apr 30, 2022

View reviewed changes

utils/frame/benchmarking-cli/src/shared/stats.rs Outdated Show resolved Hide resolved

utils/frame/benchmarking-cli/src/pallet/command.rs Outdated Show resolved Hide resolved

utils/frame/benchmarking-cli/src/pallet/command.rs Outdated Show resolved Hide resolved

hirschenberger and others added 4 commits April 30, 2022 11:59

Update utils/frame/benchmarking-cli/src/shared/stats.rs

1297c1f

Co-authored-by: Bastian Köcher <[email protected]>

Update utils/frame/benchmarking-cli/src/pallet/command.rs

2a007df

Co-authored-by: Bastian Köcher <[email protected]>

Update utils/frame/benchmarking-cli/src/pallet/command.rs

c3d82dd

Co-authored-by: Bastian Köcher <[email protected]>

Merge remote-tracking branch 'origin/master' into issue_11054

59a81ff

# Conflicts: # client/network/src/protocol/notifications/behaviour.rs # client/network/src/service.rs

paritytech-processbot bot merged commit 0ba251c into paritytech:master Apr 30, 2022

This was referenced Aug 15, 2022

declone and close the door #12035

Merged

a few lints more #12049

Closed

shawntabrizi mentioned this pull request Oct 9, 2022

Avoid Unstable Sort #12455

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply some clippy lints #11154

Apply some clippy lints #11154

hirschenberger commented Apr 2, 2022 •

edited

Loading

bkchr left a comment

hirschenberger commented Apr 14, 2022

gilescope commented Apr 28, 2022

bkchr commented Apr 28, 2022

bkchr left a comment •

edited

Loading

bkchr commented Apr 30, 2022

bkchr commented Apr 30, 2022

substrate-tip-bot bot commented Apr 30, 2022

shawntabrizi commented Apr 30, 2022

hirschenberger commented Apr 30, 2022

bkchr commented Apr 30, 2022

bkchr commented Apr 30, 2022

hirschenberger commented May 1, 2022

shawntabrizi commented May 1, 2022

substrate-tip-bot bot commented May 1, 2022

shawntabrizi commented Oct 9, 2022 •

edited

Loading

Apply some clippy lints #11154

Apply some clippy lints #11154

Conversation

hirschenberger commented Apr 2, 2022 • edited Loading

Summary

Nearly always applied

Applied sometimes depending on the code-context

Never applied

bkchr left a comment

Choose a reason for hiding this comment

hirschenberger commented Apr 14, 2022

gilescope commented Apr 28, 2022

bkchr commented Apr 28, 2022

bkchr left a comment • edited Loading

Choose a reason for hiding this comment

bkchr commented Apr 30, 2022

bkchr commented Apr 30, 2022

substrate-tip-bot bot commented Apr 30, 2022

shawntabrizi commented Apr 30, 2022

hirschenberger commented Apr 30, 2022

bkchr commented Apr 30, 2022

bkchr commented Apr 30, 2022

hirschenberger commented May 1, 2022

shawntabrizi commented May 1, 2022

substrate-tip-bot bot commented May 1, 2022

shawntabrizi commented Oct 9, 2022 • edited Loading

hirschenberger commented Apr 2, 2022 •

edited

Loading

bkchr left a comment •

edited

Loading

shawntabrizi commented Oct 9, 2022 •

edited

Loading