refactor: simplify match statements using let-else syntax #840
Conversation
Replaced verbose match statements that extract a value and early return on the alternative branch with the more concise let-else syntax. Changes include: - moq-relay: Simplified Option and Result matching in web.rs, auth.rs, cluster.rs - moq-lite: Simplified error handling patterns in model (track.rs, frame.rs, group.rs) and ietf modules (subscriber.rs, publisher.rs) - hang: Simplified nested matches in annexb.rs and group.rs This improves code readability while maintaining the same behavior. All tests pass.
WalkthroughThis pull request refactors control flow patterns across multiple Rust modules by systematically replacing pattern matching blocks with let-else constructs for handling Option and Result types. Examples include converting match statements to let Some and let-Ok patterns with else branches for early returns, simplifying prefix checking, and streamlining error handling paths. The refactoring affects files in the moq-lite, moq-relay, and hang crates. All changes maintain existing semantics and behavior while altering code style and readability. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rs/moq-relay/src/cluster.rs (1)
204-208: The.unwrap()on line 206 assumes the cancelled node exists inactive.While the current code flow appears safe (nodes are only added after the self-check on lines 199-202), using
.unwrap()here could panic if the invariant is ever violated. Consider usingif let Some(handle) = active.remove(...)for defensive coding.♻️ Suggested defensive handling
let Some(origin) = origin else { tracing::info!(%node, "origin cancelled"); - active.remove(node.as_str()).unwrap().abort(); + if let Some(handle) = active.remove(node.as_str()) { + handle.abort(); + } continue; };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
rs/hang/src/import/annexb.rsrs/hang/src/model/group.rsrs/moq-lite/src/ietf/publisher.rsrs/moq-lite/src/ietf/subscriber.rsrs/moq-lite/src/model/frame.rsrs/moq-lite/src/model/group.rsrs/moq-lite/src/model/track.rsrs/moq-relay/src/auth.rsrs/moq-relay/src/cluster.rsrs/moq-relay/src/web.rs
🧰 Additional context used
📓 Path-based instructions (2)
rs/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
rs/**/*.rs: Usecargofor Rust-specific operations
Rust tests should be integrated within source files rather than in separate test files
Files:
rs/moq-lite/src/ietf/publisher.rsrs/moq-lite/src/ietf/subscriber.rsrs/moq-lite/src/model/track.rsrs/moq-relay/src/auth.rsrs/moq-lite/src/model/group.rsrs/hang/src/model/group.rsrs/moq-relay/src/cluster.rsrs/hang/src/import/annexb.rsrs/moq-lite/src/model/frame.rsrs/moq-relay/src/web.rs
rs/hang/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Media-specific encoding/decoding should be implemented in the hang layer on top of moq-lite
Files:
rs/hang/src/model/group.rsrs/hang/src/import/annexb.rs
🧠 Learnings (1)
📚 Learning: 2025-12-20T13:22:14.684Z
Learnt from: Frando
Repo: moq-dev/moq PR: 794
File: justfile:150-156
Timestamp: 2025-12-20T13:22:14.684Z
Learning: In the moq project, for iroh connections, the endpoint ID (public key) in the justfile is derived from the secret key configured in dev/relay.toml. They form an Ed25519 keypair, not identical values.
Applied to files:
rs/moq-relay/src/auth.rs
🧬 Code graph analysis (2)
rs/moq-relay/src/auth.rs (2)
rs/moq-token/src/key.rs (1)
from_file(216-222)rs/moq-lite/src/model/origin.rs (3)
root(281-306)root(432-434)root(532-534)
rs/moq-relay/src/cluster.rs (1)
rs/moq-lite/src/model/origin.rs (3)
root(281-306)root(432-434)root(532-534)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check
🔇 Additional comments (15)
rs/moq-lite/src/ietf/subscriber.rs (2)
79-81: LGTM!The let-else pattern is idiomatic for this early-return scenario. The semantics are preserved—returning
Err(Error::InvalidRole)whenoriginisNone.
116-118: LGTM!Consistent application of the let-else pattern as in
start_announce.rs/moq-lite/src/model/frame.rs (2)
166-171: LGTM!The let-else pattern correctly handles the
RecvErrorcase (sender dropped) by returningError::Cancel, then separately checks for application-level errors instate.closed. Semantics preserved.
184-189: LGTM!Consistent pattern with
read_chunks—clean separation between channel-level errors and application-level closed state.rs/moq-lite/src/model/track.rs (1)
168-176: LGTM!The let-else refactor correctly handles the channel closure case. The subsequent match on
state.closedappropriately handles clean termination vs. error propagation.rs/hang/src/import/annexb.rs (1)
21-26: LGTM!Clean application of let-else for the nested
Result<Option<_>>pattern. The?propagates errors while let-else handles theNonecase with an early return.rs/moq-lite/src/model/group.rs (1)
182-185: LGTM!The let-else pattern cleanly handles the
Nonecase forself.active. The variable shadowing on line 185 (framebecomesBytesfromread_all()) is intentional and idiomatic here.rs/moq-lite/src/ietf/publisher.rs (1)
82-89: LGTM!The let-else refactoring correctly handles the
Nonecase by sending aSubscribeErrorwith a 404 status and returning early. This is semantically equivalent to the previous match statement while being more concise and readable.rs/hang/src/model/group.rs (1)
63-66: LGTM!The let-else pattern correctly handles the end-of-stream case by returning
Ok(None)when no more frames are available. The error propagation via?happens before the pattern match, preserving the original error handling semantics.rs/moq-relay/src/auth.rs (2)
71-71: LGTM!The method chain
as_deref().map(...).transpose()?is idiomatic Rust for conditionally applying a fallible operation to anOption. This correctly handles:None→None,Some(path)with successful load →Some(key), andSome(path)with failed load → propagates error.
114-116: LGTM!The let-else pattern correctly handles the path prefix validation, returning
AuthError::IncorrectRootwhen the URL path doesn't match the token's root claim.rs/moq-relay/src/cluster.rs (1)
119-130: LGTM!The let-else with
.filter()elegantly combines the root existence check and self-node comparison. If no root is configured or if we are the root node ourselves, the else branch correctly runs as root and waits for leaf connections.rs/moq-relay/src/web.rs (3)
235-237: LGTM!The let-else pattern cleanly handles the unauthorized case when no subscriber origin is available for the given token.
268-270: LGTM!Consistent with the pattern used in
serve_announcedfor authorization checking.
283-288: LGTM!The two-stage let-else pattern cleanly separates error semantics:
Errfromnext_group()indicates a transport/protocol failure (500), whileOk(None)indicates no group is available yet (404). This is clearer than a nested match and correctly maps to HTTP status codes.
Replaced verbose match statements that extract a value and early return
on the alternative branch with the more concise let-else syntax.
Changes include:
cluster.rs
frame.rs, group.rs) and ietf modules (subscriber.rs, publisher.rs)
This improves code readability while maintaining the same behavior.
All tests pass.