Skip to content

chore: use is_empty() instead of .len() == 0 / .len() > 0 - #829

Merged
manan19 merged 2 commits into
mainfrom
chore/use-is-empty
Apr 27, 2026
Merged

chore: use is_empty() instead of .len() == 0 / .len() > 0#829
manan19 merged 2 commits into
mainfrom
chore/use-is-empty

Conversation

@manan19

@manan19 manan19 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Mechanical Clippy-style refactor: .len() == 0.is_empty() and .len() > 0!x.is_empty() across 21 files (49 occurrences). verification.rs is already handled by #827, so this PR skips it.

Also includes a few related simplifications in link_store.rs that the refactor surfaced:

  • Drop duplicate is_empty() || .len() == 0 checks in link_add_key / link_remove_key — the OR-clauses were logically redundant.
  • Remove unreachable is_empty() check on Option<&[u8; 24]> — a fixed-size 24-byte array reference can never be empty (caught in PR chore: drop unused malloc_conf, idiomatic cleanups, and first tests for src/jobs/ #796 review).
  • Simplify data_bytes.is_some() && data_bytes.as_ref().unwrap().len() > 0 in store/account/message.rs to data_bytes.as_ref().is_some_and(|b| !b.is_empty()).

Adds 6 regression tests in link_store_test.rs covering the error branches in link_add_key / link_remove_key ("targetId provided without type", "link type invalid") which had no prior direct test coverage.

Supersedes #592. Split out from #796 per review feedback.

Test plan

  • cargo test passes locally
  • cargo fmt --check passes
  • 6 new link_store_test regression tests pass

🤖 Generated with Claude Code

Mechanical Clippy-style refactor across 21 files (49 occurrences).
Replaces `.len() == 0` with `.is_empty()` and `.len() > 0` with
`!x.is_empty()`. Verification.rs already covered by #827.

Also includes a few related simplifications in link_store.rs:
- Drops duplicate `is_empty() || .len() == 0` checks in link_add_key
  and link_remove_key (the OR-clauses were logically redundant)
- Removes unreachable `is_empty()` check on `Option<&[u8; 24]>` —
  a fixed-size 24-byte array can never be empty
- Simplifies `data_bytes.is_some() && data_bytes.as_ref().unwrap().len() > 0`
  in store/account/message.rs to `data_bytes.as_ref().is_some_and(|b| !b.is_empty())`

Adds 6 regression tests in link_store_test.rs covering the error
branches in link_add_key / link_remove_key (`"targetId provided
without type"`, `"link type invalid"`) which had no prior direct
coverage.

Supersedes #592.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 24, 2026 23:31
@vercel

vercel Bot commented Apr 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
snapchain-docs Ready Ready Preview, Comment Apr 25, 2026 0:23am

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR performs an idiomatic Rust refactor replacing .len() == 0 / .len() > 0 checks with .is_empty() / !is_empty() across the codebase, and includes a small set of link_store simplifications plus regression tests for newly-covered error branches.

Changes:

  • Replace empty/non-empty checks with is_empty() / !is_empty() across storage, networking, consensus, and validation modules.
  • Simplify redundant/previously-unreachable validations in src/storage/store/account/link_store.rs.
  • Add regression tests in link_store_test.rs covering link_add_key / link_remove_key error branches.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/storage/trie/trie_node.rs Uses is_empty() for key/prefix emptiness checks.
src/storage/store/shard.rs Uses !is_empty() when computing next_page_token.
src/storage/store/engine_tests.rs Updates test assertions to use is_empty() idioms.
src/storage/store/block.rs Uses !is_empty() when computing next_page_token.
src/storage/store/account/username_proof_store.rs Uses is_empty() for username proof name validation and merge-conflict checks.
src/storage/store/account/reaction_store.rs Uses !is_empty() for pagination token logic.
src/storage/store/account/onchain_event_store.rs Uses !is_empty() for pagination token logic in event paging.
src/storage/store/account/message.rs Uses !is_empty() for page token logic and simplifies data_bytes presence/non-empty checks via is_some_and.
src/storage/store/account/link_store_test.rs Adds regression tests for make_add_key / make_remove_key error branches.
src/storage/store/account/link_store.rs Simplifies redundant validations and updates empty checks; removes unreachable len()==0 check on fixed-size ts-hash reference.
src/storage/store/account/event.rs Uses !is_empty() for event page token logic.
src/storage/store/account/cast_store.rs Uses !is_empty() for pagination token logic in cast paging.
src/storage/store/account/block_event_store.rs Uses !is_empty() for pagination token logic in block-event paging.
src/perf/perftest.rs Uses !is_empty() in perf statistics computations.
src/network/http_server.rs Uses is_empty() in base64 optional deserialization.
src/network/gossip.rs Uses !is_empty() for config override checks.
src/mempool/block_receiver.rs Uses is_empty() for early returns / continues on empty event lists.
src/core/validations/message.rs Uses is_empty() for required-bytes, signatures, hashes, and fname checks.
src/core/types.rs Uses !is_empty() for proposer selection precondition assertions.
src/consensus/malachite/read_sync.rs Uses is_empty() for peer set emptiness check.
src/consensus/consensus.rs Uses !is_empty() for validator set/address configuration assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/storage/store/account/link_store_test.rs Outdated
Comment thread src/storage/store/account/block_event_store.rs Outdated
Comment thread src/storage/store/account/link_store.rs Outdated
Two issues caught by CI and Copilot review:

1. block_event_store.rs no longer needs `use tracing::error;` — the
   import was carried over from an older revision when the file's only
   call site used the macro. Rust 1.95 + -Dwarnings turns this unused
   import into a hard CI failure.

2. link_store.rs's link_remove_key error message was "targetID …"
   while link_add_key (and other stores like reaction_store) use
   "targetId …". Standardize on the lowercase 'd' variant; update the
   matching test assertion.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/consensus/consensus.rs (50.0%): Missing lines 108
  • src/consensus/malachite/read_sync.rs (100%)
  • src/core/types.rs (100%)
  • src/core/validations/message.rs (100%)
  • src/mempool/block_receiver.rs (100%)
  • src/network/gossip.rs (100%)
  • src/network/http_server.rs (0.0%): Missing lines 74
  • src/perf/perftest.rs (0.0%): Missing lines 227-230
  • src/storage/store/account/block_event_store.rs (100%)
  • src/storage/store/account/cast_store.rs (100%)
  • src/storage/store/account/event.rs (100%)
  • src/storage/store/account/link_store.rs (100%)
  • src/storage/store/account/link_store_test.rs (97.4%): Missing lines 2158,2173
  • src/storage/store/account/message.rs (100%)
  • src/storage/store/account/onchain_event_store.rs (100%)
  • src/storage/store/account/reaction_store.rs (100%)
  • src/storage/store/account/username_proof_store.rs (100%)
  • src/storage/store/block.rs (100%)
  • src/storage/store/shard.rs (100%)
  • src/storage/trie/trie_node.rs (100%)

Summary

  • Total: 118 lines
  • Missing: 8 lines
  • Coverage: 93%

src/consensus/consensus.rs

  104             return sets.to_vec();
  105         }
  106 
  107         if let Some(addresses) = &self.validator_addresses {
! 108             assert!(!addresses.is_empty());
  109             return vec![ValidatorSetConfig {
  110                 effective_at: 0,
  111                 validator_public_keys: addresses.clone(),
  112                 shard_ids: vec![shard_id],

src/network/http_server.rs

  70     }
  71 
  72     pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result<Option<Vec<u8>>, D::Error> {
  73         let base64 = String::deserialize(d)?.replace(" ", "+");
! 74         if base64.is_empty() {
  75             Ok(None)
  76         } else {
  77             let decoded = BASE64_STANDARD
  78                 .decode(base64.as_bytes())

src/perf/perftest.rs

  223                     // }
  224                 }
  225             }
  226             time = stats_calculation_timer.tick() => {
! 227                 let avg_time_to_confirmation = if !time_to_confirmation.is_empty() {time_to_confirmation.iter().sum::<u64>() as f64 / time_to_confirmation.len() as f64} else {0f64};
! 228                 let max_time_to_confirmation = if !time_to_confirmation.is_empty() {time_to_confirmation.clone().into_iter().max().unwrap()} else {0};
! 229                 let avg_block_time = if !block_times.is_empty() {block_times.iter().sum::<u64>() as f64 / block_times.len() as f64} else {0f64};
! 230                 let max_block_time = if !block_times.is_empty() {block_times.clone().into_iter().max().unwrap()} else {0};
  231                 let time_elapsed = time.duration_since(start).as_secs();
  232                 let confirmed_msgs_per_sec = num_messages_confirmed as f64 / cfg.stats_calculation_interval.as_secs_f64();
  233                 let submitted_msgs_per_sec = num_messages_submitted as f64 / cfg.stats_calculation_interval.as_secs_f64();
  234                 println!("{:#?} (secs): Blocks produced: {:#?}, Msgs submitted/sec: {:#?}, Msgs confirmed/sec: {:#?}, Avg time to confirmation (secs): {:#?}, Max time to confirmation (secs): {:#?}, Avg block time (secs): {:#?}, Max block time (secs): {:#?}", time_elapsed, block_count, submitted_msgs_per_sec, confirmed_msgs_per_sec, avg_time_to_confirmation, max_time_to_confirmation, avg_block_time, max_block_time);

src/storage/store/account/link_store_test.rs

  2154             if let Some(message::message_data::Body::LinkBody(body)) = data.body.as_mut() {
  2155                 body.r#type = link_type.to_string();
  2156                 body.target = target;
  2157             }
! 2158         }
  2159         msg
  2160     }
  2161 
  2162     fn make_link_remove_with(

  2169             if let Some(message::message_data::Body::LinkBody(body)) = data.body.as_mut() {
  2170                 body.r#type = link_type.to_string();
  2171                 body.target = target;
  2172             }
! 2173         }
  2174         msg
  2175     }
  2176 
  2177     #[test]

@manan19
manan19 merged commit 15c7562 into main Apr 27, 2026
14 checks passed
@manan19
manan19 deleted the chore/use-is-empty branch April 27, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants