Merge #627: Fix a bunch of clippy lints

7eccfdb clippy: whitelist iter_kv lint since it is broken (Andrew Poelstra)
6de288f cargo fmt (Andrew Poelstra)
f9c4e53 clippy: fix PartialOrd impl on an Ord type (Andrew Poelstra)
1f8dfe2 clippy: clean up iterator (Andrew Poelstra)
5ab7afe clippy: whitelist inapplicable lints (Andrew Poelstra)
2237906 clippy: use try_fold in place of fold in several places (Andrew Poelstra)
4f7782f clippy: minor things (Andrew Poelstra)
a61f71a clippy: eliminate a bunch of redundant `into_iter` calls (Andrew Poelstra)

Pull request description:

  Supercedes #615.

ACKs for top commit:
  tcharding:
    ACK 7eccfdb
  sanket1729:
    ACK 7eccfdb

Tree-SHA512: ea4f14f754b07c9ae127684e6ca1d9f121361a7be617d45a19af0269af735d2f420f59abdf15b943052b34cc9afba31cddc87d75a86694f8eefa0d672d4b031b

Author: apoelstra

clippy.toml

@@ -1 +1,3 @@
 msrv = "1.48.0"
+# plan API returns Self as an error type for an large-ish enum
+large-error-threshold = 256

src/descriptor/checksum.rs

@@ -114,11 +114,11 @@ impl Engine {
         let mut chars = [0 as char; CHECKSUM_LENGTH];
         let mut checksum_remaining = CHECKSUM_LENGTH;
 
-        for j in 0..CHECKSUM_LENGTH {
+        for checksum_ch in &mut chars {
             checksum_remaining -= 1;
             let unpacked = self.inner.residue().unpack(checksum_remaining);
             let fe = Fe32::try_from(unpacked).expect("5 bits fits in an fe32");
-            chars[j] = fe.to_char();
+            *checksum_ch = fe.to_char();
         }
         chars
     }

src/descriptor/key.rs

@@ -589,7 +589,7 @@ impl DescriptorPublicKey {
                 };
                 xpub.derivation_paths
                     .paths()
-                    .into_iter()
+                    .iter()
                     .map(|p| origin_path.extend(p))
                     .collect()
             }
@@ -852,8 +852,7 @@ fn parse_xkey_deriv<K: InnerXKey>(
         // step all the vectors of indexes contain a single element. If it did though, one of the
         // vectors contains more than one element.
         // Now transform this list of vectors of steps into distinct derivation paths.
-        .fold(Ok(Vec::new()), |paths, index_list| {
-            let mut paths = paths?;
+        .try_fold(Vec::new(), |mut paths, index_list| {
             let mut index_list = index_list?.into_iter();
             let first_index = index_list
                 .next()
@@ -940,12 +939,9 @@ impl<K: InnerXKey> DescriptorXKey<K> {
         let (fingerprint, path) = keysource;
 
         let (compare_fingerprint, compare_path) = match self.origin {
-            Some((fingerprint, ref path)) => (
-                fingerprint,
-                path.into_iter()
-                    .chain(self.derivation_path.into_iter())
-                    .collect(),
-            ),
+            Some((fingerprint, ref path)) => {
+                (fingerprint, path.into_iter().chain(&self.derivation_path).collect())
+            }
             None => (
                 self.xkey.xkey_fingerprint(secp),
                 self.derivation_path.into_iter().collect::<Vec<_>>(),

src/descriptor/tr.rs

@@ -90,13 +90,7 @@ impl<Pk: MiniscriptKey> PartialEq for Tr<Pk> {
 impl<Pk: MiniscriptKey> Eq for Tr<Pk> {}
 
 impl<Pk: MiniscriptKey> PartialOrd for Tr<Pk> {
-    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
-        match self.internal_key.partial_cmp(&other.internal_key) {
-            Some(cmp::Ordering::Equal) => {}
-            ord => return ord,
-        }
-        self.tree.partial_cmp(&other.tree)
-    }
+    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> { Some(self.cmp(other)) }
 }
 
 impl<Pk: MiniscriptKey> Ord for Tr<Pk> {
@@ -737,7 +731,7 @@ where
             wit.push(Placeholder::TapScript(leaf_script.0));
             wit.push(Placeholder::TapControlBlock(control_block));
 
-            let wit_size = witness_size(&wit);
+            let wit_size = witness_size(wit);
             if min_wit_len.is_some() && Some(wit_size) > min_wit_len {
                 continue;
             } else {

src/iter/mod.rs

@@ -82,7 +82,7 @@ impl<'a, Pk: MiniscriptKey> TreeLike for &'a policy::Concrete<Pk> {
     }
 }
 
-impl<'a, Pk: MiniscriptKey> TreeLike for Arc<policy::Concrete<Pk>> {
+impl<Pk: MiniscriptKey> TreeLike for Arc<policy::Concrete<Pk>> {
     fn as_node(&self) -> Tree<Self> {
         use policy::Concrete::*;
         match self.as_ref() {

src/lib.rs

@@ -85,6 +85,8 @@
 #![deny(dead_code)]
 #![deny(unused_imports)]
 #![deny(missing_docs)]
+// Clippy lints that we have disabled
+#![allow(clippy::iter_kv_map)] // https://github.com/rust-lang/rust-clippy/issues/11752
 
 #[cfg(target_pointer_width = "16")]
 compile_error!(
@@ -116,7 +118,6 @@ mod macros;
 mod pub_macros;
 
 use internals::hex::exts::DisplayHex;
-pub use pub_macros::*;
 
 pub mod descriptor;
 pub mod expression;
@@ -812,6 +813,7 @@ mod tests {
     }
 }
 
+#[allow(unused_imports)] // this is an internal prelude module; not all imports are used with every feature combination
 mod prelude {
     // Mutex implementation from LDK
     // https://github.com/lightningdevkit/rust-lightning/blob/9bdce47f0e0516e37c89c09f1975dfc06b5870b1/lightning-invoice/src/sync.rs

src/miniscript/types/extra_props.rs

@@ -792,11 +792,11 @@ impl Property for ExtData {
                 .iter()
                 .rev()
                 .enumerate()
-                .fold(Some(0), |acc, (i, &(x, y))| {
+                .try_fold(0, |acc, (i, &(x, y))| {
                     if i <= k {
-                        opt_add(acc, x)
+                        x.map(|x| acc + x)
                     } else {
-                        opt_add(acc, y)
+                        y.map(|y| acc + y)
                     }
                 });
 
@@ -805,11 +805,11 @@ impl Property for ExtData {
             .iter()
             .rev()
             .enumerate()
-            .fold(Some(0), |acc, (i, &(x, y))| {
+            .try_fold(0, |acc, (i, &(x, y))| {
                 if i <= k {
-                    opt_max(acc, x)
+                    x.map(|x| acc + x)
                 } else {
-                    opt_max(acc, y)
+                    y.map(|y| acc + y)
                 }
             });
 
@@ -819,26 +819,25 @@ impl Property for ExtData {
             max_sat_size_vec
                 .iter()
                 .enumerate()
-                .fold(Some((0, 0)), |acc, (i, &(x, y))| {
+                .try_fold((0, 0), |acc, (i, &(x, y))| {
                     if i <= k {
-                        opt_tuple_add(acc, x)
+                        x.map(|(x0, x1)| (acc.0 + x0, acc.1 + x1))
                     } else {
-                        opt_tuple_add(acc, y)
+                        y.map(|(y0, y1)| (acc.0 + y0, acc.1 + y1))
                     }
                 });
 
         ops_count_sat_vec.sort_by(sat_minus_dissat);
-        let op_count_sat =
-            ops_count_sat_vec
-                .iter()
-                .enumerate()
-                .fold(Some(0), |acc, (i, &(x, y))| {
-                    if i <= k {
-                        opt_add(acc, x)
-                    } else {
-                        opt_add(acc, Some(y))
-                    }
-                });
+        let op_count_sat = ops_count_sat_vec
+            .iter()
+            .enumerate()
+            .try_fold(0, |acc, (i, &(x, y))| {
+                if i <= k {
+                    x.map(|x| acc + x)
+                } else {
+                    Some(acc + y)
+                }
+            });
 
         Ok(ExtData {
             pk_cost: pk_cost + n - 1, //all pk cost + (n-1)*ADD
@@ -1049,11 +1048,6 @@ fn opt_max<T: Ord>(a: Option<T>, b: Option<T>) -> Option<T> {
 /// Returns Some(x+y) is both x and y are Some. Otherwise, returns `None`.
 fn opt_add(a: Option<usize>, b: Option<usize>) -> Option<usize> { a.and_then(|x| b.map(|y| x + y)) }
 
-/// Returns Some((x0+y0, x1+y1)) is both x and y are Some. Otherwise, returns `None`.
-fn opt_tuple_add(a: Option<(usize, usize)>, b: Option<(usize, usize)>) -> Option<(usize, usize)> {
-    a.and_then(|x| b.map(|(w, s)| (w + x.0, s + x.1)))
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/plan.rs

@@ -255,13 +255,13 @@ impl Plan {
             // scriptSig len (1) + OP_0 (1) + OP_PUSHBYTES_32 (1) + <script hash> (32)
             (_, DescriptorType::ShWsh) | (_, DescriptorType::ShWshSortedMulti) => 1 + 1 + 1 + 32,
             // Native Segwit v0 (scriptSig len (1))
-            __ => 1,
+            _ => 1,
         }
     }
 
     /// The size in bytes of the witness that satisfies this plan
     pub fn witness_size(&self) -> usize {
-        if let Some(_) = self.descriptor.desc_type().segwit_version() {
+        if self.descriptor.desc_type().segwit_version().is_some() {
             witness_size(self.template.as_ref())
         } else {
             0 // should be 1 if there's at least one segwit input in the tx, but that's out of
@@ -383,7 +383,7 @@ impl Plan {
                     .entry(pk)
                     .and_modify(|(leaf_hashes, _)| {
                         if let Some(lh) = leaf_hash {
-                            if leaf_hashes.iter().find(|&&i| i == lh).is_none() {
+                            if leaf_hashes.iter().all(|&i| i != lh) {
                                 leaf_hashes.push(lh);
                             }
                         }
@@ -397,17 +397,14 @@ impl Plan {
             }
         } else {
             for item in &self.template {
-                match item {
-                    Placeholder::EcdsaSigPk(pk) => {
-                        let public_key = pk.to_public_key().inner;
-                        let master_fingerprint = pk.master_fingerprint();
-                        for derivation_path in pk.full_derivation_paths() {
-                            input
-                                .bip32_derivation
-                                .insert(public_key, (master_fingerprint, derivation_path));
-                        }
+                if let Placeholder::EcdsaSigPk(pk) = item {
+                    let public_key = pk.to_public_key().inner;
+                    let master_fingerprint = pk.master_fingerprint();
+                    for derivation_path in pk.full_derivation_paths() {
+                        input
+                            .bip32_derivation
+                            .insert(public_key, (master_fingerprint, derivation_path));
                     }
-                    _ => {}
                 }
             }
 
@@ -669,7 +666,7 @@ impl IntoAssets for DescriptorPublicKey {
 }
 
 impl IntoAssets for Vec<DescriptorPublicKey> {
-    fn into_assets(self) -> Assets { Assets::from_iter(self.into_iter()) }
+    fn into_assets(self) -> Assets { Assets::from_iter(self) }
 }
 
 impl IntoAssets for sha256::Hash {
@@ -705,6 +702,7 @@ impl Assets {
     pub fn new() -> Self { Self::default() }
 
     /// Add some assets
+    #[allow(clippy::should_implement_trait)] // looks like the `ops::Add` trait
     pub fn add<A: IntoAssets>(mut self, asset: A) -> Self {
         self.append(asset.into_assets());
         self
@@ -723,14 +721,11 @@ impl Assets {
     }
 
     fn append(&mut self, b: Self) {
-        self.keys.extend(b.keys.into_iter());
-        self.sha256_preimages.extend(b.sha256_preimages.into_iter());
-        self.hash256_preimages
-            .extend(b.hash256_preimages.into_iter());
-        self.ripemd160_preimages
-            .extend(b.ripemd160_preimages.into_iter());
-        self.hash160_preimages
-            .extend(b.hash160_preimages.into_iter());
+        self.keys.extend(b.keys);
+        self.sha256_preimages.extend(b.sha256_preimages);
+        self.hash256_preimages.extend(b.hash256_preimages);
+        self.ripemd160_preimages.extend(b.ripemd160_preimages);
+        self.hash160_preimages.extend(b.hash160_preimages);
 
         self.relative_timelock = b.relative_timelock.or(self.relative_timelock);
         self.absolute_timelock = b.absolute_timelock.or(self.absolute_timelock);

src/psbt/finalizer.rs

@@ -428,7 +428,7 @@ pub(super) fn finalize_input<C: secp256k1::Verification>(
     // Now mutate the psbt input. Note that we cannot error after this point.
     // If the input is mutated, it means that the finalization succeeded.
     {
-        let original = mem::replace(&mut psbt.inputs[index], Default::default());
+        let original = mem::take(&mut psbt.inputs[index]);
         let input = &mut psbt.inputs[index];
         input.non_witness_utxo = original.non_witness_utxo;
         input.witness_utxo = original.witness_utxo;