Merge #282: Fix pedantic clippy lints

5b262ee Fix pedantic clippy lints in fieldvec (Abeeujah)
510b0c4 Refactor primitives::encode Option handling (Abeeujah)
5f2a228 Refactor witness_version to use functional combinators (Abeeujah)
dc217db Fix trailing whitespace in ErrorIterator error message (Abeeujah)
f2ace2c Fix pedantic lints in primitives::checksum module (Abeeujah)

Pull request description:

  This PR refactors a couple of data flow control and constructs regarding the `Option<T>` type, to be as idiomatic and ergonomic as the MSRV permits.
  
  Commits are by Modules, but can be squashed if needed.


ACKs for top commit:
  apoelstra:
    ACK 5b262ee; successfully ran local tests


Tree-SHA512: 842a6e50236f7290f5b1e2b96cedd2136a1be6344fb3ff7bb627f5f6e79bbe75baf686c310bb82b4c55d9660d0297cd8f651dc46909bc21098f8a856895c1988

Author: apoelstra

src/primitives/checksum.rs

@@ -8,7 +8,7 @@ use core::marker::PhantomData;
 use core::{fmt, mem, ops};
 
 use super::Polynomial;
-use crate::primitives::hrp::Hrp;
+use crate::primitives::hrp::{Hrp, LowercaseByteIter};
 use crate::{Fe1024, Fe32};
 
 /// Trait defining a particular checksum.
@@ -179,7 +179,7 @@ impl<'a, ExtField> PrintImpl<'a, ExtField> {
     /// Panics if any of the input values fail various sanity checks.
     pub fn new(name: &'a str, generator: &'a [Fe32], target: &'a [Fe32]) -> Self {
         // Sanity checks.
-        assert_ne!(name.len(), 0, "type name cannot be the empty string",);
+        assert_ne!(name.len(), 0, "type name cannot be the empty string");
         assert_ne!(
             generator.len(),
             0,
@@ -396,9 +396,7 @@ impl PackedFe32 for PackedNull {
 
     #[inline]
     fn pack<I: Iterator<Item = u8>>(mut iter: I) -> Self {
-        if iter.next().is_some() {
-            panic!("Cannot pack anything into a PackedNull");
-        }
+        assert!(iter.next().is_none(), "Cannot pack anything into a PackedNull");
         Self
     }
 }
@@ -469,13 +467,12 @@ impl Iterator for HrpFe32Iter<'_> {
     #[inline]
     fn next(&mut self) -> Option<Fe32> {
         if let Some(ref mut high_iter) = &mut self.high_iter {
-            match high_iter.next() {
-                Some(high) => return Some(Fe32(high >> 5)),
-                None => {
-                    self.high_iter = None;
-                    return Some(Fe32::Q);
-                }
-            }
+            return if let Some(high) = high_iter.next() {
+                Some(Fe32(high >> 5))
+            } else {
+                self.high_iter = None;
+                Some(Fe32::Q)
+            };
         }
         if let Some(ref mut low_iter) = &mut self.low_iter {
             match low_iter.next() {
@@ -488,17 +485,11 @@ impl Iterator for HrpFe32Iter<'_> {
 
     #[inline]
     fn size_hint(&self) -> (usize, Option<usize>) {
-        let high = match &self.high_iter {
-            Some(high_iter) => {
-                let (min, max) = high_iter.size_hint();
-                (min + 1, max.map(|max| max + 1)) // +1 for the extra Q
-            }
-            None => (0, Some(0)),
-        };
-        let low = match &self.low_iter {
-            Some(low_iter) => low_iter.size_hint(),
-            None => (0, Some(0)),
-        };
+        let high = self.high_iter.as_ref().map_or((0, Some(0)), |high_iter| {
+            let (min, max) = high_iter.size_hint();
+            (min + 1, max.map(|max| max + 1)) // +1 for the extra Q
+        });
+        let low = self.low_iter.as_ref().map_or((0, Some(0)), LowercaseByteIter::size_hint);
 
         let min = high.0 + low.0;
         let max = high.1.zip(low.1).map(|(high, low)| high + low);

src/primitives/correction.rs

@@ -299,7 +299,7 @@ impl<Ck: Checksum> Iterator for ErrorIterator<'_, Ck> {
         let ret = -num / den;
         match ret.try_into() {
             Ok(ret) => Some((neg_i, ret)),
-            Err(_) => unreachable!("error guaranteed to  lie in base field"),
+            Err(_) => unreachable!("error guaranteed to lie in base field"),
         }
     }
 }

src/primitives/decode.rs

@@ -215,17 +215,10 @@ impl<'s> UncheckedHrpstring<'s> {
     /// ```
     #[inline]
     pub fn witness_version(&self) -> Option<Fe32> {
-        let data_part = self.data_part_ascii();
-        if data_part.is_empty() {
-            return None;
-        }
-
-        // unwrap ok because we know we gave valid bech32 characters.
-        let witness_version = Fe32::from_char(data_part[0].into()).unwrap();
-        if witness_version.to_u8() > 16 {
-            return None;
-        }
-        Some(witness_version)
+        self.data_part_ascii
+            .first()
+            .map(|&ch| Fe32::from_char(ch.into()).unwrap()) // unwrap ok because we know we gave valid bech32 characters.
+            .filter(|version| version.to_u8() <= 16)
     }
 
     /// Validates that data has a valid checksum for the `Ck` algorithm and returns a [`CheckedHrpstring`].
@@ -427,17 +420,10 @@ impl<'s> CheckedHrpstring<'s> {
     /// ```
     #[inline]
     pub fn witness_version(&self) -> Option<Fe32> {
-        let data_part = self.data_part_ascii_no_checksum();
-        if data_part.is_empty() {
-            return None;
-        }
-
-        // unwrap ok because we know we gave valid bech32 characters.
-        let witness_version = Fe32::from_char(data_part[0].into()).unwrap();
-        if witness_version.to_u8() > 16 {
-            return None;
-        }
-        Some(witness_version)
+        self.data_part_ascii_no_checksum()
+            .first()
+            .map(|&ch| Fe32::from_char(ch.into()).unwrap()) // unwrap ok because we know we gave valid bech32 characters.
+            .filter(|version| version.to_u8() <= 16)
     }
 
     /// Returns an iterator that yields the data part of the parsed bech32 encoded string as [`Fe32`]s.

src/primitives/encode.rs

@@ -209,40 +209,36 @@ where
     #[inline]
     fn next(&mut self) -> Option<char> {
         if let Some(ref mut hrp_iter) = self.hrp_iter {
-            match hrp_iter.next() {
-                Some(c) => return Some(c),
-                None => {
-                    self.hrp_iter = None;
-                    return Some('1');
-                }
-            }
+            return if let Some(c) = hrp_iter.next() {
+                Some(c)
+            } else {
+                self.hrp_iter = None;
+                Some('1')
+            };
         }
 
-        self.checksummed.next().map(|fe| fe.to_char())
+        self.checksummed.next().map(Fe32::to_char)
     }
 
     #[inline]
     fn size_hint(&self) -> (usize, Option<usize>) {
-        match &self.hrp_iter {
+        self.hrp_iter.as_ref().map_or_else(
             // We have yielded the hrp and separator already.
-            None => self.checksummed.size_hint(),
+            || self.checksummed.size_hint(),
             // Yet to finish yielding the hrp (and the separator).
-            Some(hrp_iter) => {
+            |hrp_iter| {
                 let (hrp_min, hrp_max) = hrp_iter.size_hint();
                 let (chk_min, chk_max) = self.checksummed.size_hint();
 
                 let min = hrp_min + 1 + chk_min; // +1 for the separator.
 
                 // To provide a max boundary we need to have gotten a value from the hrp iter as well as the
                 // checksummed iter, otherwise we have to return None since we cannot know the maximum.
-                let max = match (hrp_max, chk_max) {
-                    (Some(hrp_max), Some(chk_max)) => Some(hrp_max + 1 + chk_max),
-                    (_, _) => None,
-                };
+                let max = hrp_max.zip(chk_max).map(|(hrp, chk)| hrp + 1 + chk);
 
                 (min, max)
-            }
-        }
+            },
+        )
     }
 }
 
@@ -330,10 +326,7 @@ where
 
     #[inline]
     fn size_hint(&self) -> (usize, Option<usize>) {
-        let hrp = match &self.hrp_iter {
-            Some(hrp_iter) => hrp_iter.size_hint(),
-            None => (0, Some(0)),
-        };
+        let hrp = self.hrp_iter.as_ref().map_or((0, Some(0)), HrpFe32Iter::size_hint);
 
         let data = self.checksummed.size_hint();
 

src/primitives/fieldvec.rs

@@ -352,7 +352,7 @@ impl<F: Clone + Default> iter::FromIterator<F> for FieldVec<F> {
         // This goofy map construction is needed because we cannot use the
         // `[F::default(); N]` syntax without adding a `Copy` bound to `F`.
         // After Rust 1.63 we will be able to use array::from_fn.
-        let mut inner_a = [(); NO_ALLOC_MAX_LENGTH].map(|_| F::default());
+        let mut inner_a = [(); NO_ALLOC_MAX_LENGTH].map(|()| F::default());
         let mut len = 0;
         for elem in iter.by_ref().take(NO_ALLOC_MAX_LENGTH) {
             inner_a[len] = elem;
@@ -389,7 +389,7 @@ impl<F: Clone + Default> iter::FromIterator<F> for FieldVec<F> {
 
 impl<F: fmt::Display> fmt::Display for FieldVec<F> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        for fe in self.iter() {
+        for fe in self {
             fe.fmt(f)?;
         }
         Ok(())