feat: enable clippy::correctness as part of CI (#8299)

This enables clippy `clippy::correctness` checks as part of CI as discussed [here](#8291 (comment)).

It is added as part of `sanity check` step for consistency, since that is where `cargo check` is executed.

This fixes the following issues in order to pass the added checks:
* noop `.clone()`
* this loop never actually loops 
* `0` is the minimum value for this type, the case where the two sides are not equal never occurs, consider using `self.burst == 0` instead

This PR is a part of #8145

.buildkite/pipeline.yml

@@ -90,6 +90,8 @@ steps:
 
       ./scripts/formatting --check
 
+      ./scripts/run_clippy.sh
+
       rm target/rpc_errors_schema.json
       cargo check -p near-jsonrpc --features dump_errors_schema
       if ! git --no-pager diff --no-index chain/jsonrpc/res/rpc_errors_schema.json target/rpc_errors_schema.json; then

chain/chain/src/store_validator/validate.rs

@@ -879,46 +879,46 @@ pub(crate) fn state_part_header_exists(
 pub(crate) fn block_height_cmp_tail_final(
     sv: &mut StoreValidator,
 ) -> Result<(), StoreValidatorError> {
-    if sv.inner.block_heights_less_tail.len() >= 2 {
-        let len = sv.inner.block_heights_less_tail.len();
+    let len = sv.inner.block_heights_less_tail.len();
+    if len >= 2 {
         let blocks = &sv.inner.block_heights_less_tail;
         err!("Found {:?} Blocks with height lower than Tail, {:?}", len, blocks)
     }
     Ok(())
 }
 
 pub(crate) fn tx_refcount_final(sv: &mut StoreValidator) -> Result<(), StoreValidatorError> {
-    let len = sv.inner.tx_refcount.len();
-    if len > 0 {
-        for tx_refcount in sv.inner.tx_refcount.iter() {
-            err!("Found {:?} Txs that are not counted, i.e. {:?}", len, tx_refcount);
-        }
+    if let Some(tx_refcount) = sv.inner.tx_refcount.iter().next() {
+        err!(
+            "Found {:?} Txs that are not counted, e.g. {:?}",
+            sv.inner.tx_refcount.len(),
+            tx_refcount
+        );
     }
     Ok(())
 }
 
 pub(crate) fn receipt_refcount_final(sv: &mut StoreValidator) -> Result<(), StoreValidatorError> {
-    let len = sv.inner.receipt_refcount.len();
-    if len > 0 {
-        for receipt_refcount in sv.inner.receipt_refcount.iter() {
-            err!("Found {:?} receipts that are not counted, i.e. {:?}", len, receipt_refcount);
-        }
+    if let Some(receipt_refcount) = sv.inner.receipt_refcount.iter().next() {
+        err!(
+            "Found {:?} receipts that are not counted, e.g. {:?}",
+            sv.inner.receipt_refcount.len(),
+            receipt_refcount
+        );
     }
     Ok(())
 }
 
 pub(crate) fn block_refcount_final(sv: &mut StoreValidator) -> Result<(), StoreValidatorError> {
-    if sv.inner.block_refcount.len() > 1 {
-        let len = sv.inner.block_refcount.len();
-        for block_refcount in sv.inner.block_refcount.iter() {
-            err!("Found {:?} Blocks that are not counted, i.e. {:?}", len, block_refcount);
-        }
+    if let Some(block_refcount) = sv.inner.block_refcount.iter().next() {
+        err!(
+            "Found {:?} Blocks that are not counted, e.g. {:?}",
+            sv.inner.block_refcount.len(),
+            block_refcount
+        );
     }
-    if sv.inner.genesis_blocks.len() > 1 {
-        let len = sv.inner.genesis_blocks.len();
-        for tail_block in sv.inner.genesis_blocks.iter() {
-            err!("Found {:?} Genesis Blocks, i.e. {:?}", len, tail_block);
-        }
+    if let Some(tail_block) = sv.inner.genesis_blocks.first() {
+        err!("Found {:?} Genesis Blocks, e.g. {:?}", sv.inner.genesis_blocks.len(), tail_block);
     }
     Ok(())
 }

chain/chunks/src/lib.rs

@@ -612,6 +612,9 @@ impl ShardsManager {
                 continue;
             }
 
+            // This is false positive, similar to what was reported here:
+            // https://github.com/rust-lang/rust-clippy/issues/5940
+            #[allow(clippy::if_same_then_else)]
             let fetch_from = if request_from_archival {
                 shard_representative_target.clone()
             } else if we_own_part {

chain/network/src/concurrency/rate.rs

@@ -19,7 +19,7 @@ impl Limit {
         if self.qps <= 0. {
             anyhow::bail!("qps has to be >0");
         }
-        if self.burst <= 0 {
+        if self.burst == 0 {
             anyhow::bail!("burst has to be >0");
         }
         Ok(())

core/store/src/trie/mod.rs

@@ -950,7 +950,7 @@ impl Trie {
         key: &[u8],
         mode: KeyLookupMode,
     ) -> Result<Option<ValueRef>, StorageError> {
-        let key_nibbles = NibbleSlice::new(key.clone());
+        let key_nibbles = NibbleSlice::new(key);
         let result = self.lookup(key_nibbles);
 
         // For now, to test correctness, flat storage does double the work and

scripts/run_clippy.sh

@@ -1,3 +1,6 @@
 #!/usr/bin/env bash
-cargo clippy --all  -- -A clippy::type-complexity -A clippy::needless-pass-by-value -A clippy::while-let-loop -A clippy::too-many-arguments -A clippy::unit_arg -A clippy::if_same_then_else -A clippy::collapsible_if -A clippy::useless-let-if-seq -A clippy::map-entry -D warnings -A clippy::implicit-hasher -A clippy::ptr-arg -A renamed-and-removed-lints -A clippy::needless-range-loop -A clippy::unused_io_amount -A clippy::wrong-self-convention
+# clippy adoption is in progress, see https://github.com/near/nearcore/issues/8145
+cargo clippy -- \
+    -A clippy::all \
+    -D clippy::correctness
 

tools/state-viewer/src/apply_chunk.rs

@@ -85,7 +85,7 @@ pub(crate) fn apply_chunk(
     let shard_id = chunk.shard_id();
     let prev_state_root = chunk.prev_state_root();
 
-    let transactions = chunk.transactions().clone();
+    let transactions = chunk.transactions();
     let prev_block =
         chain_store.get_block(&prev_block_hash).context("Failed getting chunk's prev block")?;
     let prev_height_included = prev_block.chunks()[shard_id as usize].height_included();
@@ -126,7 +126,7 @@ pub(crate) fn apply_chunk(
                 &hash("nonsense block hash for testing purposes".as_ref()),
             ),
             &receipts,
-            &transactions,
+            transactions,
             chunk_header.validator_proposals(),
             gas_price,
             chunk_header.gas_limit(),
@@ -380,7 +380,7 @@ fn apply_receipt_in_chunk(
         println!("Applying chunk at height {} in shard {}. Equivalent command (which will run faster than apply_receipt):\nview_state apply_chunk --chunk_hash {}\n",
                  height, shard_id, chunk_hash.0);
         let (apply_result, gas_limit) =
-            apply_chunk(runtime.clone(), chain_store, chunk_hash.clone(), None, None)?;
+            apply_chunk(runtime, chain_store, chunk_hash.clone(), None, None)?;
         let chunk_extra = crate::commands::resulting_chunk_extra(&apply_result, gas_limit);
         println!("resulting chunk extra:\n{:?}", chunk_extra);
         results.push(apply_result);