zcash / librustzcash

Rust-language assets for Zcash
Other
337 stars 252 forks source link

[Meta] Clippy and rustfmt contradict each other in specific rule #750

Open pacu opened 1 year ago

pacu commented 1 year ago

If I run cargo format and clippy I'd get contradicting errors. Fixing one them for one tool would break the other tool. Specifically around the clippy::suspicious-else-formatting rule.

librustzcash % cargo fmt --all -- --check

Diff in /Users/pacu/Repos/ECC/librustzcash/zcash_client_backend/src/welding_rig.rs at line 203:
     tree: &mut CommitmentTree<Node>,
     existing_witnesses: &mut [&mut IncrementalWitness<Node>],
     mut batch_runner: Option<&mut TaggedBatchRunner<P, K::Scope, T>>,
-) -> Vec<WalletTx<K::Nf>> 
-{
+) -> Vec<WalletTx<K::Nf>> {
     let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
     let block_height = block.height();
     let block_hash = block.hash();
Diff in /Users/pacu/Repos/ECC/librustzcash/zcash_client_sqlite/src/lib.rs at line 523:
         &mut self,
         block: &PrunedBlock,
         updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
-    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> 
-    {
+    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
         // database updates for each block are transactional
         self.transactionally(|up| {
             // Insert the block into the database.

These two things are changes that clippy made me do.

error: this looks like an `else {..}` but the `else` is missing
   --> zcash_client_backend/src/welding_rig.rs:206:9
    |
206 | ) -> Vec<WalletTx<K::Nf>> {
    |         ^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::suspicious-else-formatting` implied by `-D warnings`
    = note: to remove this lint, add the missing `else` or add a new line before the next block
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting

error: could not compile `zcash_client_backend` due to previous error
warning: build failed, waiting for other jobs to finish...
error: build failed
pacu@zecretlab librustzcash % cargo clippy --all-features --all-targets -- -D warnings 
    Checking zcash_client_backend v0.6.1 (/Users/pacu/Repos/ECC/librustzcash/zcash_client_backend)
    Checking zcash_client_sqlite v0.4.2 (/Users/pacu/Repos/ECC/librustzcash/zcash_client_sqlite)
error: this looks like an `else {..}` but the `else` is missing
   --> zcash_client_sqlite/src/lib.rs:526:16
    |
526 |     ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::suspicious-else-formatting` implied by `-D warnings`
    = note: to remove this lint, add the missing `else` or add a new line before the next block
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting
str4d commented 1 year ago

This issue is caused by https://github.com/tokio-rs/tracing/issues/2410 and isn't something we can fix right now. In the interim, prefer rustfmt correctness (once the upstream issue is resolved, the clippy warning will go away).

str4d commented 1 year ago

I've fixed this upstream in https://github.com/tokio-rs/tracing/pull/2437 and confirmed that our clippy lint is fixed by it. Once that PR is accepted and in a release, a cargo update will be sufficient to close this issue.