Merge #295: ci: fix clippy lints and a couple other small things

48634a8 clippy: enable lossless-cast lint (Andrew Poelstra)
1b43322 clippy: address all current lints (Andrew Poelstra)
1a3cc54 ci: improve compiler versioning (Andrew Poelstra)

Pull request description:

  * Fix clippy lints
  * Enable a few more lints related to casting
  * Fix CI to use the `nightly-version` version of clippy so that we won't break as new lints come out


ACKs for top commit:
  canndrew:
    ACK 48634a8


Tree-SHA512: b1df3d9afa13eb239acd5ed791c8128196f857a7907127852d783dee65db3ccd86773cabc5eaff3e2a56932888976ebf244a128ebb80845729560febae6c4662

Author: apoelstra

.github/workflows/main.yml

@@ -7,12 +7,22 @@ jobs:
     runs-on: ubuntu-24.04
     outputs:
       nightly_version: ${{ steps.read_toolchain.outputs.nightly_version }}
+      msrv_version: ${{ steps.read_msrv.outputs.msrv_version }}
     steps:
       - name: "Checkout repo"
         uses: actions/checkout@v4
       - name: "Read nightly version"
         id: read_toolchain
-        run: echo "nightly_version=$(cat nightly-version)" >> $GITHUB_OUTPUT
+        run: |
+          set -euo pipefail
+          version=$(cat nightly-version)
+          echo "nightly_version=$version" >> $GITHUB_OUTPUT
+      - name: Read MSRV from clippy.toml
+        id: read_msrv
+        run: |
+          set -euo pipefail
+          msrv=$(grep '^msrv *= *"' clippy.toml | sed -E 's/.*"([^"]+)".*/\1/')
+          echo "msrv_version=$msrv" >> "$GITHUB_OUTPUT"
 
   fmt:
     name: Rustfmt
@@ -66,7 +76,7 @@ jobs:
           - stable
           - beta
           - ${{ needs.Prepare.outputs.nightly_version }}
-          - 1.78.0
+          - ${{ needs.Prepare.outputs.msrv_version }}
     steps:
       - name: Checkout Crate
         uses: actions/checkout@v4
@@ -100,6 +110,7 @@ jobs:
 
   clippy:
     name: Clippy
+    needs: Prepare
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
@@ -110,6 +121,7 @@ jobs:
       - name: Checkout Toolchain
         uses: dtolnay/rust-toolchain@stable
         with:
+          toolchain: ${{ needs.Prepare.outputs.nightly_version }}
           components: clippy
       - name: Running cargo clippy
         run: cargo clippy --workspace --all-targets -- --deny warnings

Cargo.toml

@@ -47,3 +47,21 @@ exclude = ["jets-bench"]
 
 [lints.rust]
 unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)'] }
+
+[lints.clippy]
+# Exclude lints we don't think are valuable.
+needless_question_mark = "allow" # https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
+manual_range_contains = "allow" # More readable than clippy's format.
+uninlined_format_args = "allow" # This is a subjective style choice.
+float_cmp = "allow" # Bitcoin floats are typically limited to 8 decimal places and we want them exact.
+match_bool = "allow" # Adds extra indentation and LOC.
+match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other.
+must_use_candidate = "allow" # Useful for audit but many false positives.
+similar_names = "allow" # Too many (subjectively) false positives.
+# Cast-related lints
+cast_lossless = "warn"
+cast_possible_truncation = "allow" # All casts should include a code comment (except test code).
+cast_possible_wrap = "allow" # Same as above re code comment.
+cast_precision_loss = "warn"
+cast_ptr_alignment = "warn"
+cast_sign_loss = "allow" # All casts should include a code comment (except in test code).

simplicity-sys/Cargo.toml

@@ -20,3 +20,14 @@ test-utils = []
 
 [lints.rust]
 unexpected_cfgs = { level = "deny", check-cfg = ['cfg(fuzzing)'] }
+
+[lints.clippy]
+# Exclude lints we don't think are valuable.
+needless_question_mark = "allow" # https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
+manual_range_contains = "allow" # More readable than clippy's format.
+uninlined_format_args = "allow" # This is a subjective style choice.
+float_cmp = "allow" # Bitcoin floats are typically limited to 8 decimal places and we want them exact.
+match_bool = "allow" # Adds extra indentation and LOC.
+match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other.
+must_use_candidate = "allow" # Useful for audit but many false positives.
+similar_names = "allow" # Too many (subjectively) false positives.

src/node/mod.rs

@@ -703,7 +703,7 @@ impl<N: Marker> Node<N> {
     ///
     /// The linear string has no sharing and may be **exponentially larger**
     /// than the originally shared expression!
-    pub fn display_expr(&self) -> DisplayExpr<N> {
+    pub fn display_expr(&self) -> DisplayExpr<'_, N> {
         DisplayExpr::from(self)
     }
 }

src/policy/satisfy.rs

@@ -312,7 +312,7 @@ mod tests {
 
     fn get_satisfier(
         env: &ElementsEnv<Arc<elements::Transaction>>,
-    ) -> PolicySatisfier<XOnlyPublicKey> {
+    ) -> PolicySatisfier<'_, XOnlyPublicKey> {
         let mut preimages = HashMap::new();
 
         for i in 0..3 {
@@ -555,14 +555,14 @@ mod tests {
             let witness = to_witness(&program);
             assert_eq!(2, witness.len());
 
-            assert_eq!(Value::u1(bit as u8), *witness[0]);
+            assert_eq!(Value::u1(u8::from(bit)), *witness[0]);
             let preimage_bytes = witness[1]
                 .iter_padded()
                 .try_collect_bytes()
                 .expect("to bytes");
             let witness_preimage =
                 Preimage32::try_from(preimage_bytes.as_slice()).expect("to array");
-            assert_eq!(preimages[bit as usize], witness_preimage);
+            assert_eq!(preimages[usize::from(bit)], witness_preimage);
 
             execute_successful(program, &env);
         };
@@ -663,7 +663,7 @@ mod tests {
                         ],
                     );
 
-                    match bit0 as u8 + bit1 as u8 + bit2 as u8 {
+                    match u8::from(bit0) + u8::from(bit1) + u8::from(bit2) {
                         3 => assert_branches(&policy, &[bit0, bit1, false]),
                         2 => assert_branches(&policy, &[bit0, bit1, bit2]),
                         _ => assert!(policy.satisfy(&satisfier, &env).is_err()),

src/types/context.rs

@@ -231,7 +231,7 @@ impl Context {
     }
 
     /// Locks the underlying slab mutex.
-    fn lock(&self) -> LockedContext {
+    fn lock(&self) -> LockedContext<'_> {
         LockedContext {
             context: Arc::as_ptr(&self.slab),
             slab: self.slab.lock().unwrap(),

src/value.rs

@@ -435,7 +435,7 @@ impl Value {
     }
 
     /// A reference to this value, which can be recursed over.
-    pub fn as_ref(&self) -> ValueRef {
+    pub fn as_ref(&self) -> ValueRef<'_> {
         ValueRef {
             inner: &self.inner,
             bit_offset: self.bit_offset,
@@ -444,17 +444,17 @@ impl Value {
     }
 
     /// Access the inner value of a left sum value.
-    pub fn as_left(&self) -> Option<ValueRef> {
+    pub fn as_left(&self) -> Option<ValueRef<'_>> {
         self.as_ref().as_left()
     }
 
     /// Access the inner value of a right sum value.
-    pub fn as_right(&self) -> Option<ValueRef> {
+    pub fn as_right(&self) -> Option<ValueRef<'_>> {
         self.as_ref().as_right()
     }
 
     /// Access the inner values of a product value.
-    pub fn as_product(&self) -> Option<(ValueRef, ValueRef)> {
+    pub fn as_product(&self) -> Option<(ValueRef<'_>, ValueRef<'_>)> {
         self.as_ref().as_product()
     }
 
@@ -590,7 +590,7 @@ impl Value {
     /// The returned bytes match the padded bit-encoding of the value. You
     /// may wish to call [`Self::iter_padded`] instead to obtain the bits,
     /// but this method is more efficient in some contexts.
-    pub fn raw_byte_iter(&self) -> RawByteIter {
+    pub fn raw_byte_iter(&self) -> RawByteIter<'_> {
         RawByteIter {
             value: self,
             yielded_bytes: 0,
@@ -600,14 +600,14 @@ impl Value {
     /// Return an iterator over the compact bit encoding of the value.
     ///
     /// This encoding is used for writing witness data and for computing IHRs.
-    pub fn iter_compact(&self) -> CompactBitsIter {
+    pub fn iter_compact(&self) -> CompactBitsIter<'_> {
         CompactBitsIter::new(self.as_ref())
     }
 
     /// Return an iterator over the padded bit encoding of the value.
     ///
     /// This encoding is used to represent the value in the Bit Machine.
-    pub fn iter_padded(&self) -> PreOrderIter {
+    pub fn iter_padded(&self) -> PreOrderIter<'_> {
         PreOrderIter {
             inner: BitIter::new(self.raw_byte_iter()).take(self.ty.bit_width()),
         }