Merge rust-bitcoin/rust-secp256k1#526: Run the formatter in CI

75f3886812 Add cargo fmt to pre-commit githook (Tobin C. Harding)
0516ddeb8d Add formatting check to CI (Tobin C. Harding)
c7807dff9c Run the formatter (Tobin C. Harding)

Pull request description:

  We recently introduced `rustfmt` to the codebase but I forgot to turn it on in CI.

  - Patch 1: Preparatory formatting fixes, introduced since we merged the [formatting PR](https://github.com/rust-bitcoin/rust-secp256k1/pull/499)
  - Patch 2: Enable formatting in CI
  - Patch 3: Add formatting to the pre-commit hook

ACKs for top commit:
  apoelstra:
    ACK 75f3886812

Tree-SHA512: 5ac4ab4015a9728ef890e0c4fe90afcb5e45ab7665da5a8ee289dc877c1ea5c6236e54b68b7122841597864b04606c8bfae7dec86c4b6be74d32437299057b5f

.github/workflows/rust.yml

@@ -18,6 +18,10 @@ jobs:
           toolchain: nightly
           override: true
           components: rust-src
+      - name: Check formatting
+        env:
+          DO_FMT: true
+        run: ./contrib/test.sh
       - name: Running address sanitizer
         env:
           DO_ASAN: true

contrib/test.sh

@@ -88,6 +88,15 @@ if [ "$DO_ASAN" = true ]; then
     cargo run --release --features=alloc --manifest-path=./no_std_test/Cargo.toml | grep -q "Verified alloc Successfully"
 fi
 
+# Run formatter if told to.
+if [ "$DO_FMT" = true ]; then
+    if [ "$NIGHTLY" = false ]; then
+        echo "DO_FMT requires a nightly toolchain (consider using RUSTUP_TOOLCHAIN)"
+        exit 1
+    fi
+    rustup component add rustfmt
+    cargo fmt --check || exit 1
+fi
 
 # Bench if told to, only works with non-stable toolchain (nightly, beta).
 if [ "$DO_BENCH" = true ]

githooks/pre-commit

@@ -48,3 +48,6 @@ git diff-index --check --cached $against -- || exit 1
 
 # Check that code lints cleanly.
 cargo clippy --features=rand-std,recovery,lowmemory,global-context --all-targets -- -D warnings || exit 1
+
+# Check that there are no formatting issues.
+cargo +nightly fmt --check || exit 1

src/key.rs

@@ -1515,7 +1515,7 @@ mod test {
     use core::str::FromStr;
 
     #[cfg(feature = "rand")]
-    use rand::{self, RngCore, rngs::mock::StepRng};
+    use rand::{self, rngs::mock::StepRng, RngCore};
     use serde_test::{Configure, Token};
     #[cfg(target_arch = "wasm32")]
     use wasm_bindgen_test::wasm_bindgen_test as test;

src/lib.rs

@@ -530,19 +530,16 @@ pub(crate) fn random_32_bytes<R: rand::Rng + ?Sized>(rng: &mut R) -> [u8; 32] {
 
 #[cfg(test)]
 mod tests {
-    #[allow(unused_imports)]    // When building with no default features.
-    use super::*;
-
     use std::str::FromStr;
 
     #[cfg(target_arch = "wasm32")]
     use wasm_bindgen_test::wasm_bindgen_test as test;
 
+    #[allow(unused_imports)] // When building with no default features.
+    use super::*;
+    use crate::{constants, ecdsa, from_hex, Error, Message};
     #[cfg(feature = "alloc")]
     use crate::{ffi, PublicKey, Secp256k1, SecretKey};
-    use crate::{
-        constants, ecdsa, from_hex, Error, Message,
-    };
 
     macro_rules! hex {
         ($hex:expr) => {{
@@ -889,9 +886,10 @@ mod tests {
     #[test]
     #[cfg(feature = "rand-std")]
     fn test_hex() {
-        use super::to_hex;
         use rand::RngCore;
 
+        use super::to_hex;
+
         let mut rng = rand::thread_rng();
         const AMOUNT: usize = 1024;
         for i in 0..AMOUNT {

src/macros.rs

@@ -17,22 +17,17 @@
 #[macro_export]
 macro_rules! impl_array_newtype {
     ($thing:ident, $ty:ty, $len:expr) => {
-
         // We cannot derive these traits because Rust 1.41.1 requires `std::array::LengthAtMost32`.
 
         impl PartialEq for $thing {
             #[inline]
-            fn eq(&self, other: &$thing) -> bool {
+            fn eq(&self, other: &$thing) -> bool { &self[..] == &other[..] }
-                &self[..] == &other[..]
-            }
         }
 
         impl Eq for $thing {}
 
         impl core::hash::Hash for $thing {
-            fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
+            fn hash<H: core::hash::Hasher>(&self, state: &mut H) { (&self[..]).hash(state) }
-                (&self[..]).hash(state)
-            }
         }
 
         impl PartialOrd for $thing {
@@ -44,9 +39,7 @@ macro_rules! impl_array_newtype {
 
         impl Ord for $thing {
             #[inline]
-            fn cmp(&self, other: &$thing) -> core::cmp::Ordering {
+            fn cmp(&self, other: &$thing) -> core::cmp::Ordering { self[..].cmp(&other[..]) }
-                self[..].cmp(&other[..])
-            }
         }
 
         impl AsRef<[$ty; $len]> for $thing {
@@ -81,7 +74,7 @@ macro_rules! impl_array_newtype {
                 dat.as_mut_ptr()
             }
         }
-    }
+    };
 }
 
 macro_rules! impl_pretty_debug {
@@ -139,5 +132,5 @@ macro_rules! impl_fast_comparisons {
                 self.0.eq_fast_unstable(&other.0)
             }
         }
-    }
+    };
 }

src/schnorr.rs

@@ -11,7 +11,9 @@ use crate::ffi::{self, CPtr};
 use crate::key::{KeyPair, XOnlyPublicKey};
 #[cfg(feature = "global-context")]
 use crate::SECP256K1;
-use crate::{constants, from_hex, impl_array_newtype, Error, Message, Secp256k1, Signing, Verification};
+use crate::{
+    constants, from_hex, impl_array_newtype, Error, Message, Secp256k1, Signing, Verification,
+};
 
 /// Represents a Schnorr signature.
 #[derive(Copy, Clone)]