rust: setup clippy lints and fix existing lint issues #4455

ptrus · 2022-01-28T20:39:09Z

codecov · 2022-01-31T14:50:53Z

Codecov Report

Merging #4455 (cb18563) into master (795eb65) will increase coverage by 0.17%.
The diff coverage is 70.40%.

@@            Coverage Diff             @@
##           master    #4455      +/-   ##
==========================================
+ Coverage   68.66%   68.83%   +0.17%     
==========================================
  Files         415      415              
  Lines       46571    46799     +228     
==========================================
+ Hits        31976    32214     +238     
+ Misses      10635    10625      -10     
  Partials     3960     3960

Impacted Files	Coverage Δ
go/consensus/tendermint/abci/mux.go	`59.15% <0.00%> (+3.28%)`	⬆️
go/consensus/tendermint/api/app.go	`0.00% <0.00%> (ø)`
go/consensus/tendermint/apps/beacon/beacon.go	`73.77% <0.00%> (ø)`
...consensus/tendermint/apps/keymanager/keymanager.go	`62.63% <0.00%> (ø)`
go/consensus/tendermint/apps/registry/genesis.go	`47.32% <0.00%> (ø)`
...o/consensus/tendermint/apps/scheduler/scheduler.go	`71.14% <0.00%> (ø)`
...nt/apps/supplementarysanity/supplementarysanity.go	`77.61% <0.00%> (ø)`
go/roothash/api/api.go	`80.00% <ø> (ø)`
go/runtime/registry/host.go	`71.88% <0.00%> (-4.81%)`	⬇️
go/staking/api/api.go	`62.33% <ø> (ø)`
... and 46 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44b27c2...cb18563. Read the comment docs.

pro-wh

23/74

hold on let me empty out the linter

pro-wh · 2022-01-31T21:38:33Z

runtime/src/common/key_format.rs

-        if data.len() < 1 + Self::size() {
-            panic!("key format: malformed input");
-        }
+        assert!(data.len() > Self::size(), "key format: malformed input");


I'd somehow find this more understandable with data.len() >= 1 + Self::size()

Clippy disagrees: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one
but we can disable that rule if we prefer not to enforce it.

we could make it more formal

const VERSION_PREFIX_SIZE: usize = the_size_of_a_u8; data.len() >= VERSION_PREFIX_SIZE + Self:size()

or let's go with clippy's (probably less thoughtful) recommendation and add a comment about "must be strictly greater than because there must additionally be a version prefix thingy"

addendum: "and the prefix is exactly one byte, so it suffices only to use > instead of >=. marvel at our resourcefulness"

Ah I see, I thought you generally preferred the 1+ version. Yeah I can make this case clearer.

I updated the code with the VERSION_PREFIX_SIZE approach.

runtime/src/common/key_format.rs

pro-wh

50/74

runtime/src/consensus/tendermint/verifier.rs

runtime/src/storage/mkvs/cache/cache.rs

pro-wh · 2022-02-01T23:12:21Z

runtime/src/storage/mkvs/cache/mod.rs

+}
+
+/// Callback type used for updating cache items after a commit.
+pub type CacheUpdater<C> = Box<dyn Fn(&mut C)>;


ℹ️ -> () removed

runtime/src/storage/mkvs/sync/mod.rs

runtime/src/common/sgx/avr.rs

pro-wh

74/74

emptying out the linter again

pro-wh · 2022-02-02T19:37:08Z

runtime/src/storage/mkvs/tree/mod.rs

+    }
+
+    /// Commit the options set so far into a newly constructed tree instance.
+    pub fn new_tree(self, read_syncer: Box<dyn ReadSync>) -> Tree {


ℹ️ this replaces new. and other minor doesn't-affect-interface changes below

couple of if { panic } changed to assert

field: field shorthand

vec ref to slice

pro-wh · 2022-02-02T19:47:02Z

runtime/src/storage/mkvs/tree/remove.rs

+                                if let NodeKind::Internal = classify_noderef!(nd_child) {
+                                    if let NodeBox::Internal(ref mut inode) = *nd_child.borrow_mut()
+                                    {
+                                        inode.label = noderef_as!(node_ref, Internal).label.merge(


lol this macro work is scary

runtime/src/common/crypto/signature.rs

runtime/src/common/key_format.rs

runtime/src/common/sgx/avr.rs

runtime/src/consensus/registry.rs

runtime/src/storage/mkvs/tree/iterator.rs

runtime/src/storage/mkvs/tree/tree.rs

ptrus added the s:ready-ci Status: ready for CI label Jan 28, 2022

ptrus force-pushed the ptrus/feature/rust-lints branch 8 times, most recently from d8c7b46 to 8fa4652 Compare January 31, 2022 13:26

ptrus marked this pull request as ready for review January 31, 2022 14:52

ptrus requested review from kostko, peterjgilbert, pro-wh and Yawning as code owners January 31, 2022 14:52

pro-wh reviewed Jan 31, 2022

View reviewed changes

ptrus force-pushed the ptrus/feature/rust-lints branch 2 times, most recently from d847286 to 6094e68 Compare February 1, 2022 15:51

pro-wh reviewed Feb 1, 2022

View reviewed changes

Yawning reviewed Feb 2, 2022

View reviewed changes

runtime/src/common/sgx/avr.rs Show resolved Hide resolved

pro-wh approved these changes Feb 2, 2022

View reviewed changes

ptrus force-pushed the ptrus/feature/rust-lints branch 2 times, most recently from 3d73468 to 5280ba6 Compare February 3, 2022 15:22

kostko reviewed Feb 4, 2022

View reviewed changes

ptrus force-pushed the ptrus/feature/rust-lints branch 3 times, most recently from ee6bd85 to 154cce9 Compare February 4, 2022 16:21

ptrus mentioned this pull request Feb 4, 2022

rust: fix staking_params_are_empty #4460

Closed

rust: setup clippy lints and fix existing lint issues

cb18563

ptrus force-pushed the ptrus/feature/rust-lints branch from 154cce9 to cb18563 Compare February 4, 2022 17:25

kostko approved these changes Feb 4, 2022

View reviewed changes

ptrus merged commit 80f8c9a into master Feb 4, 2022

ptrus deleted the ptrus/feature/rust-lints branch February 4, 2022 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rust: setup clippy lints and fix existing lint issues #4455

rust: setup clippy lints and fix existing lint issues #4455

ptrus commented Jan 28, 2022

codecov bot commented Jan 31, 2022 •

edited

Loading

pro-wh left a comment

pro-wh Jan 31, 2022

ptrus Feb 1, 2022

pro-wh Feb 1, 2022

pro-wh Feb 1, 2022 •

edited

Loading

ptrus Feb 2, 2022

ptrus Feb 3, 2022

pro-wh left a comment

pro-wh Feb 1, 2022

pro-wh left a comment

pro-wh Feb 2, 2022

pro-wh Feb 2, 2022

rust: setup clippy lints and fix existing lint issues #4455

rust: setup clippy lints and fix existing lint issues #4455

Conversation

ptrus commented Jan 28, 2022

codecov bot commented Jan 31, 2022 • edited Loading

Codecov Report

pro-wh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pro-wh Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pro-wh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pro-wh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 31, 2022 •

edited

Loading

pro-wh Feb 1, 2022 •

edited

Loading