Simplify matches (oxidecomputer#1490)

Drive-by cleanup to simplify match statements: - Use `..` instead of `foobar: _` to ignore fields - Consolidate `match` statement with identical branches (I don't think we're ever deliberately destructuring objects to check their shape, but let me know if that's an incorrect assumption!)
jmpesp · Oct 4, 2024 · da58373 · da58373
1 parent aad43cf
commit da58373
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 98 deletions.
diff --git a/crucible-client-types/src/lib.rs b/crucible-client-types/src/lib.rs
@@ -41,19 +41,12 @@ pub enum VolumeConstructionRequest {
 impl VolumeConstructionRequest {
     pub fn targets(&self) -> Vec<SocketAddr> {
         match self {
-            VolumeConstructionRequest::Volume {
-                id: _,
-                block_size: _,
-                sub_volumes,
-                read_only_parent: _,
-            } => sub_volumes.iter().flat_map(|s| s.targets()).collect(),
-            VolumeConstructionRequest::Region {
-                block_size: _,
-                blocks_per_extent: _,
-                extent_count: _,
-                opts,
-                gen: _,
-            } => opts.target.clone(),
+            VolumeConstructionRequest::Volume { sub_volumes, .. } => {
+                sub_volumes.iter().flat_map(|s| s.targets()).collect()
+            }
+            VolumeConstructionRequest::Region { opts, .. } => {
+                opts.target.clone()
+            }
             _ => vec![],
         }
     }

diff --git a/downstairs/src/lib.rs b/downstairs/src/lib.rs
@@ -1316,9 +1316,9 @@ impl ActiveConnection {
             Message::ExtentFlush {
                 repair_id,
                 extent_id,
-                client_id: _,
                 flush_number,
                 gen_number,
+                ..
             } => {
                 let msg = {
                     debug!(
@@ -3680,16 +3680,7 @@ mod test {
                 assert!(work.completed.is_complete(dep));
             }
 
-            matches!(
-                job,
-                IOop::Flush {
-                    dependencies: _,
-                    flush_number: _,
-                    gen_number: _,
-                    snapshot_details: _,
-                    extent_limit: _,
-                }
-            )
+            matches!(job, IOop::Flush { .. })
         };
 
         if is_flush {

diff --git a/protocol/src/lib.rs b/protocol/src/lib.rs
@@ -592,53 +592,55 @@ impl Message {
     /// Return true if this message contains an Error result
     pub fn err(&self) -> Option<&CrucibleError> {
         match self {
-            Message::HereIAm { .. } => None,
-            Message::YesItsMe { .. } => None,
-            Message::VersionMismatch { .. } => None,
-            Message::ReadOnlyMismatch { .. } => None,
-            Message::EncryptedMismatch { .. } => None,
-            Message::PromoteToActive { .. } => None,
-            Message::YouAreNowActive { .. } => None,
-            Message::YouAreNoLongerActive { .. } => None,
-            Message::UuidMismatch { .. } => None,
-            Message::Ruok { .. } => None,
-            Message::Imok { .. } => None,
-            Message::ExtentClose { .. } => None,
-            Message::ExtentReopen { .. } => None,
-            Message::ExtentFlush { .. } => None,
-            Message::ExtentRepair { .. } => None,
-            Message::RepairAckId { .. } => None,
-            Message::RegionInfoPlease { .. } => None,
-            Message::RegionInfo { .. } => None,
-            Message::ExtentVersionsPlease { .. } => None,
-            Message::ExtentVersions { .. } => None,
-            Message::LastFlush { .. } => None,
-            Message::LastFlushAck { .. } => None,
-            Message::Write { .. } => None,
-            Message::ExtentLiveClose { .. } => None,
-            Message::ExtentLiveFlushClose { .. } => None,
-            Message::ExtentLiveRepair { .. } => None,
-            Message::ExtentLiveReopen { .. } => None,
-            Message::ExtentLiveNoOp { .. } => None,
-            Message::Flush { .. } => None,
-            Message::ReadRequest { .. } => None,
-            Message::WriteUnwritten { .. } => None,
-            Message::Unknown(..) => None,
-
-            Message::ExtentError { error, .. } => Some(error),
-            Message::ErrorReport { error, .. } => Some(error),
+            Message::HereIAm { .. }
+            | Message::YesItsMe { .. }
+            | Message::VersionMismatch { .. }
+            | Message::ReadOnlyMismatch { .. }
+            | Message::EncryptedMismatch { .. }
+            | Message::PromoteToActive { .. }
+            | Message::YouAreNowActive { .. }
+            | Message::YouAreNoLongerActive { .. }
+            | Message::UuidMismatch { .. }
+            | Message::Ruok { .. }
+            | Message::Imok { .. }
+            | Message::ExtentClose { .. }
+            | Message::ExtentReopen { .. }
+            | Message::ExtentFlush { .. }
+            | Message::ExtentRepair { .. }
+            | Message::RepairAckId { .. }
+            | Message::RegionInfoPlease { .. }
+            | Message::RegionInfo { .. }
+            | Message::ExtentVersionsPlease { .. }
+            | Message::ExtentVersions { .. }
+            | Message::LastFlush { .. }
+            | Message::LastFlushAck { .. }
+            | Message::Write { .. }
+            | Message::ExtentLiveClose { .. }
+            | Message::ExtentLiveFlushClose { .. }
+            | Message::ExtentLiveRepair { .. }
+            | Message::ExtentLiveReopen { .. }
+            | Message::ExtentLiveNoOp { .. }
+            | Message::Flush { .. }
+            | Message::ReadRequest { .. }
+            | Message::WriteUnwritten { .. }
+            | Message::Unknown(..) => None,
+
+            Message::ExtentError { error, .. }
+            | Message::ErrorReport { error, .. } => Some(error),
 
             Message::ExtentLiveCloseAck { result, .. } => result.as_ref().err(),
-            Message::ExtentLiveRepairAckId { result, .. } => {
+
+            Message::ExtentLiveRepairAckId { result, .. }
+            | Message::ExtentLiveAckId { result, .. }
+            | Message::WriteAck { result, .. }
+            | Message::FlushAck { result, .. }
+            | Message::WriteUnwrittenAck { result, .. } => {
                 result.as_ref().err()
             }
-            Message::ExtentLiveAckId { result, .. } => result.as_ref().err(),
-            Message::WriteAck { result, .. } => result.as_ref().err(),
-            Message::FlushAck { result, .. } => result.as_ref().err(),
+
             Message::ReadResponse { header, .. } => {
                 header.blocks.as_ref().err()
             }
-            Message::WriteUnwrittenAck { result, .. } => result.as_ref().err(),
         }
     }
 }
@@ -658,7 +660,7 @@ impl std::fmt::Display for Message {
                         start,
                         ..
                     },
-                data: _,
+                ..
             } => f
                 .debug_struct("Message::Write")
                 .field("upstairs_id", &upstairs_id)
@@ -678,7 +680,7 @@ impl std::fmt::Display for Message {
                         start,
                         ..
                     },
-                data: _,
+                ..
             } => f
                 .debug_struct("Message::WriteUnwritten")
                 .field("upstairs_id", &upstairs_id)
@@ -696,7 +698,7 @@ impl std::fmt::Display for Message {
                         job_id,
                         blocks,
                     },
-                data: _,
+                ..
             } => f
                 .debug_struct("Message::ReadResponse")
                 .field("upstairs_id", &upstairs_id)

diff --git a/upstairs/src/downstairs.rs b/upstairs/src/downstairs.rs
@@ -1077,10 +1077,7 @@ impl Downstairs {
                 repair.state = LiveRepairState::Noop { noop_id, reopen_id };
                 self.create_and_enqueue_noop_io(gw, vec![repair_id], noop_id);
             }
-            LiveRepairState::Noop {
-                noop_id: _,
-                reopen_id,
-            } => {
+            LiveRepairState::Noop { reopen_id, .. } => {
                 info!(
                     self.log,
                     "RE:{} Wait for result from reopen command {}",
@@ -7975,7 +7972,7 @@ pub(crate) mod test {
 
             println!("repair op: {:?}", repair_op);
             match repair_op {
-                IOop::ExtentLiveNoOp { dependencies: _ } => {}
+                IOop::ExtentLiveNoOp { .. } => {}
                 x => {
                     panic!("Incorrect work type returned: {:?}", x);
                 }
@@ -8009,11 +8006,10 @@ pub(crate) mod test {
 
         match repair_op {
             IOop::ExtentLiveRepair {
-                dependencies: _,
                 extent,
                 source_downstairs,
-                source_repair_address: _,
                 repair_downstairs,
+                ..
             } => {
                 assert_eq!(extent, ExtentId(0));
                 assert_eq!(source_downstairs, source);

diff --git a/upstairs/src/dummy_downstairs_tests.rs b/upstairs/src/dummy_downstairs_tests.rs
@@ -131,13 +131,7 @@ impl DownstairsHandle {
     pub async fn negotiate_start(&mut self) {
         let packet = self.recv().await.unwrap();
         if let Message::HereIAm {
-            version,
-            upstairs_id: _,
-            session_id: _,
-            gen: _,
-            read_only,
-            encrypted: _,
-            alternate_versions: _,
+            version, read_only, ..
         } = &packet
         {
             info!(

diff --git a/upstairs/src/lib.rs b/upstairs/src/lib.rs
@@ -1248,32 +1248,22 @@ impl IOop {
                 let job_type = "WriteU".to_string();
                 (job_type, blocks.len(), dependencies.clone())
             }
-            IOop::Flush {
-                dependencies,
-                flush_number: _flush_number,
-                gen_number: _gen_number,
-                snapshot_details: _,
-                extent_limit: _,
-            } => {
+            IOop::Flush { dependencies, .. } => {
                 let job_type = "Flush".to_string();
                 (job_type, 0, dependencies.clone())
             }
             IOop::ExtentFlushClose {
                 dependencies,
                 extent,
-                flush_number: _,
-                gen_number: _,
-                repair_downstairs: _,
+                ..
             } => {
                 let job_type = "FClose".to_string();
                 (job_type, extent.0 as usize, dependencies.clone())
             }
             IOop::ExtentLiveRepair {
                 dependencies,
                 extent,
-                source_downstairs: _,
-                source_repair_address: _,
-                repair_downstairs: _,
+                ..
             } => {
                 let job_type = "Repair".to_string();
                 (job_type, extent.0 as usize, dependencies.clone())

diff --git a/upstairs/src/volume.rs b/upstairs/src/volume.rs
@@ -1335,10 +1335,7 @@ impl Volume {
         // Beyond this point zero or one target changes where found
 
         match &compare_result {
-            CompareResult::Volume {
-                sub_compares,
-                read_only_parent_compare: _,
-            } => {
+            CompareResult::Volume { sub_compares, .. } => {
                 // Walk the cases. As some deltas are okay for a read only
                 // parent that are not for a sub_volume, we need to look at all
                 // the possible permutations Nexus supports.