Neptune-Crypto / neptune-core

anonymous peer-to-peer cash
Apache License 2.0
28 stars 8 forks source link

clippy warnings. #45

Closed dan-da closed 1 year ago

dan-da commented 1 year ago

While working on #44 I ran cargo clippy and noticed a lot of warnings. Specifically:

$ cargo clippy 2>&1 | grep ^warning | wc -l
20
$ cargo clippy 2>&1 | grep ^warning
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: useless use of `vec!`
warning: `neptune-core` (lib) generated 15 warnings (run `cargo clippy --fix --lib -p neptune-core` to apply 10 suggestions)
warning: usage of an `Arc` that is not `Send` or `Sync`
warning: usage of an `Arc` that is not `Send` or `Sync`
warning: usage of an `Arc` that is not `Send` or `Sync`
warning: `neptune-core` (bin "neptune-dashboard") generated 3 warnings

I can work on a PR to fix these. Hopefully all are simple.

I was surprised to see the warnings as previously I've worked on projects where CI does not allow PRs to be accepted until all clippy warnings are resolved and also cargo fmt --check is clean.

Is that a policy we could consider implementing here? Keeps things nice and tidy for everyone.

Sword-Smith commented 1 year ago

The warning: useless use of vec! usually means that we are concatenating vectors like this: vec![vec![a, b], vec![c]].concat(). With rustc version 1.72.0 this became a clippy warning, as the outermost Vec<T> can be replaced by an array. So clippy wants you to write [vec![a, b], vec![c]].concat() instead.

This issue arose in several of our repos since the above, deprecated pattern was used a lot in our code.

I would have to do a little more digging into the Arc/Send issue. I don't know the solution to that from the top of my head. But it seems to be related to handling spawned threads' JoinHandle which is not too intricate a part of the code, I think -- so the complexity shouldn't be too bad.

Note that a few more linter warnings show up if you run cargo clippy --all-targets as that command includes tests as well.

dan-da commented 1 year ago

thx. I'll take a look at cleaning these up tmw. Maybe learn a bit more about the codebase in the process.

Sword-Smith commented 1 year ago

thx. I'll take a look at cleaning these up tmw. Maybe learn a bit more about the codebase in the process.

To quickly get an overview of the code, you can start here: https://github.com/Neptune-Crypto/neptune-core/blob/master/developer_docs/documentation_snapshots/20221011.md

The above documentation is not fully up to date (e.g. the mutator set DB only has atomic writes now) but it should give you a good sense of the overall architecture.