Merge rust-bitcoin#5436: internals,p2p: upgrade to workspace lint rules

d893038 p2p: upgrade to workspace lint rules (Nick Johnson)
d8de11d internals: upgrade to workspace lint rules (Nick Johnson)

Pull request description:

  These include all the clippy recommendations for the new rules. I had to ask the LLM the "but why" for a handful of these, but I think they all check out.


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


Tree-SHA512: 4749f19e32beaf4c7901f7d035f41d7317b76aaa3b4a20a9d6f09332e280081a69063f1891bb7698f8952e0a2b9680722da3ec29ec0dc1c1bd466514c365541a

Author: apoelstra

internals/Cargo.toml

@@ -34,9 +34,5 @@ bincode = { version = "1.3.1", optional = true }
 all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
-[lints.rust]
-unexpected_cfgs = { level = "deny", check-cfg = ['cfg(kani)'] }
-
-[lints.clippy]
-redundant_clone = "warn"
-use_self = "warn"
+[lints]
+workspace = true

internals/build.rs

@@ -7,17 +7,15 @@ use std::io;
 
 fn main() {
     let rustc = std::env::var_os("RUSTC");
-    let rustc = rustc.as_ref().map(std::path::Path::new).unwrap_or_else(|| "rustc".as_ref());
+    let rustc = rustc.as_ref().map_or_else(|| "rustc".as_ref(), std::path::Path::new);
     let output = std::process::Command::new(rustc)
         .arg("--version")
         .output()
         .unwrap_or_else(|error| panic!("failed to run `{:?} --version`: {:?}", rustc, error));
     assert!(output.status.success(), "{:?} -- version returned non-zero exit code", rustc);
     let stdout = String::from_utf8(output.stdout).expect("rustc produced non-UTF-8 output");
     let version_prefix = "rustc ";
-    if !stdout.starts_with(version_prefix) {
-        panic!("unexpected rustc output: {}", stdout);
-    }
+    assert!(stdout.starts_with(version_prefix), "unexpected rustc output: {}", stdout);
 
     let version = &stdout[version_prefix.len()..];
     let end = version.find(&[' ', '-'] as &[_]).unwrap_or(version.len());
@@ -32,7 +30,7 @@ fn main() {
         .expect("invalid Rust minor version");
 
     let msrv = std::env::var("CARGO_PKG_RUST_VERSION").unwrap();
-    let mut msrv = msrv.split(".");
+    let mut msrv = msrv.split('.');
     let msrv_major = msrv.next().unwrap();
     assert_eq!(msrv_major, "1", "unexpected Rust major version");
     let msrv_minor = msrv.next().unwrap().parse::<u64>().unwrap();
@@ -75,7 +73,7 @@ fn write_macro(mut macro_file: impl io::Write, msrv_minor: u64, minor: u64) -> i
         writeln!(macro_file, "        $($if_yes)*")?;
         writeln!(macro_file, "    }};")?;
     }
-    for version in (minor + 1)..(MAX_USED_VERSION + 1) {
+    for version in (minor + 1)..=MAX_USED_VERSION {
         writeln!(
             macro_file,
             "    (if >= 1.{} {{ $($if_yes:tt)* }} $(else {{ $($if_no:tt)* }})?) => {{",

internals/src/array.rs

@@ -77,7 +77,7 @@ impl<const N: usize, T> ArrayExt for [T; N] {
 
     fn sub_array<const OFFSET: usize, const LEN: usize>(&self) -> &[Self::Item; LEN] {
         #[allow(clippy::let_unit_value)]
-        let _ = Hack::<N, OFFSET, LEN>::IS_VALID_RANGE;
+        let () = Hack::<N, OFFSET, LEN>::IS_VALID_RANGE;
 
         self[OFFSET..(OFFSET + LEN)].try_into().expect("this is also compiler-checked above")
     }
@@ -86,7 +86,7 @@ impl<const N: usize, T> ArrayExt for [T; N] {
         &self,
     ) -> (&[Self::Item; LEFT], &[Self::Item; RIGHT]) {
         #[allow(clippy::let_unit_value)]
-        let _ = Hack2::<N, LEFT, RIGHT>::IS_FULL_RANGE;
+        let () = Hack2::<N, LEFT, RIGHT>::IS_FULL_RANGE;
 
         (self.sub_array::<0, LEFT>(), self.sub_array::<LEFT, RIGHT>())
     }

internals/src/array_vec.rs

@@ -73,7 +73,7 @@ mod safety_boundary {
         ///
         /// # Returns
         ///
-        /// None if the ArrayVec is empty.
+        /// None if the `ArrayVec` is empty.
         pub fn pop(&mut self) -> Option<T> {
             if self.len > 0 {
                 self.len -= 1;
@@ -114,6 +114,7 @@ impl<T: Copy, const CAP: usize> Default for ArrayVec<T, CAP> {
 /// Because we avoid copying the uninitialized part of the array this copies the value faster than
 /// memcpy.
 #[allow(clippy::non_canonical_clone_impl)]
+#[allow(clippy::expl_impl_clone_on_copy)]
 impl<T: Copy, const CAP: usize> Clone for ArrayVec<T, CAP> {
     fn clone(&self) -> Self { Self::from_slice(self) }
 }
@@ -190,14 +191,14 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
+    #[should_panic(expected = "assertion failed")]
     fn overflow_push() {
         let mut av = ArrayVec::<_, 0>::new();
         av.push(42);
     }
 
     #[test]
-    #[should_panic]
+    #[should_panic(expected = "buffer overflow")]
     fn overflow_extend() {
         let mut av = ArrayVec::<_, 0>::new();
         av.extend_from_slice(&[42]);

internals/src/compact_size.rs

@@ -44,7 +44,7 @@ pub const fn encoded_size_const(value: u64) -> usize {
     match value {
         0..=0xFC => 1,
         0xFD..=0xFFFF => 3,
-        0x10000..=0xFFFFFFFF => 5,
+        0x10000..=0xFFFF_FFFF => 5,
         _ => 9,
     }
 }
@@ -63,7 +63,7 @@ pub fn encode(value: impl ToU64) -> ArrayVec<u8, MAX_ENCODING_SIZE> {
             res.push(0xFD);
             res.extend_from_slice(&v.to_le_bytes());
         }
-        0x10000..=0xFFFFFFFF => {
+        0x10000..=0xFFFF_FFFF => {
             let v = value as u32; // Cast ok because of match.
             res.push(0xFE);
             res.extend_from_slice(&v.to_le_bytes());
@@ -88,16 +88,12 @@ pub fn encode(value: impl ToU64) -> ArrayVec<u8, MAX_ENCODING_SIZE> {
 /// * Panics in release mode if the `slice` does not contain a valid minimal compact size encoding.
 /// * Panics in debug mode if the encoding is not minimal (referred to as "non-canonical" in Core).
 pub fn decode_unchecked(slice: &mut &[u8]) -> u64 {
-    if slice.is_empty() {
-        panic!("tried to decode an empty slice");
-    }
+    assert!(!slice.is_empty(), "tried to decode an empty slice");
 
     match slice[0] {
         0xFF => {
             const SIZE: usize = 9;
-            if slice.len() < SIZE {
-                panic!("slice too short, expected at least 9 bytes");
-            };
+            assert!(slice.len() >= SIZE, "slice too short, expected at least 9 bytes");
 
             let mut bytes = [0_u8; SIZE - 1];
             bytes.copy_from_slice(&slice[1..SIZE]);
@@ -109,9 +105,7 @@ pub fn decode_unchecked(slice: &mut &[u8]) -> u64 {
         }
         0xFE => {
             const SIZE: usize = 5;
-            if slice.len() < SIZE {
-                panic!("slice too short, expected at least 5 bytes");
-            };
+            assert!(slice.len() >= SIZE, "slice too short, expected at least 5 bytes");
 
             let mut bytes = [0_u8; SIZE - 1];
             bytes.copy_from_slice(&slice[1..SIZE]);
@@ -123,9 +117,7 @@ pub fn decode_unchecked(slice: &mut &[u8]) -> u64 {
         }
         0xFD => {
             const SIZE: usize = 3;
-            if slice.len() < SIZE {
-                panic!("slice too short, expected at least 3 bytes");
-            };
+            assert!(slice.len() >= SIZE, "slice too short, expected at least 3 bytes");
 
             let mut bytes = [0_u8; SIZE - 1];
             bytes.copy_from_slice(&slice[1..SIZE]);
@@ -239,14 +231,14 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
+    #[should_panic(expected = "tried to decode an empty slice")]
     fn decode_from_empty_slice_panics() {
         let mut slice = [].as_slice();
         let _ = decode_unchecked(&mut slice);
     }
 
     #[test]
-    #[should_panic]
+    #[should_panic(expected = "slice too short")]
     // Non-minimal is referred to as non-canonical in Core (`bitcoin/src/serialize.h`).
     fn decode_non_minimal_panics() {
         let mut slice = [0xFE, 0xCD, 0xAB].as_slice();

internals/src/error/input_string.rs

@@ -68,6 +68,10 @@ impl InputString {
     ///     }
     /// }
     /// ```
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the write to the formatter fails.
     pub fn unknown_variant<T>(&self, what: &T, f: &mut fmt::Formatter) -> fmt::Result
     where
         T: fmt::Display + ?Sized,

internals/src/lib.rs

@@ -10,10 +10,6 @@
 #![warn(missing_docs)]
 #![warn(deprecated_in_future)]
 #![doc(test(attr(warn(unused))))]
-// Exclude lints we don't think are valuable.
-#![allow(clippy::needless_question_mark)] // https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
-#![allow(clippy::manual_range_contains)] // More readable than clippy's format.
-#![allow(clippy::uninlined_format_args)] // Allow `format!("{}", x)` instead of enforcing `format!("{x}")`
 
 #[cfg(feature = "alloc")]
 extern crate alloc;

internals/src/script.rs

@@ -6,6 +6,10 @@
 ///
 /// A script push data instruction includes the length of the data being pushed, this function reads
 /// that length from an iterator (encoded in either 1, 2, or 4 bytes).
+///
+/// # Errors
+///
+/// Returns an error if the iterator does not contain enough bytes to read the length.
 // We internally use implementation based on iterator so that it automatically advances as needed.
 pub fn read_push_data_len(
     data: &mut core::slice::Iter<'_, u8>,
@@ -52,22 +56,22 @@ mod tests {
         let bytes = [0x01, 0x23, 0x45, 0x67];
         let want = u32::from_le_bytes([0x01, 0x23, 0x45, 0x67]);
         let got = read_push_data_len(&mut bytes.iter(), PushDataLenLen::Four).unwrap();
-        assert_eq!(got, want as usize)
+        assert_eq!(got, want as usize);
     }
 
     #[test]
     fn reads_2_bytes() {
         let bytes = [0x01, 0x23];
         let want = u16::from_le_bytes([0x01, 0x23]);
         let got = read_push_data_len(&mut bytes.iter(), PushDataLenLen::Two).unwrap();
-        assert_eq!(got, want as usize)
+        assert_eq!(got, want as usize);
     }
 
     #[test]
     fn reads_1_byte() {
         let bytes = [0x01];
         let want = 0x01;
         let got = read_push_data_len(&mut bytes.iter(), PushDataLenLen::One).unwrap();
-        assert_eq!(got, want as usize)
+        assert_eq!(got, want as usize);
     }
 }

internals/src/serde.rs

@@ -18,6 +18,10 @@ pub trait IntoDeError: Sized {
     ///
     /// If the error type doesn't contain enough information to explain the error precisely this
     /// should return `Err(self)` allowing the caller to use its information instead.
+    ///
+    /// # Errors
+    ///
+    /// Returns `Err(self)` if the error cannot be converted to a deserializer error.
     fn try_into_de_error<E>(self, expected: Option<&dyn de::Expected>) -> Result<E, Self>
     where
         E: de::Error,
@@ -27,7 +31,7 @@ pub trait IntoDeError: Sized {
 }
 
 mod impls {
-    use super::*;
+    use super::{IntoDeError, de};
 
     impl IntoDeError for core::convert::Infallible {
         fn into_de_error<E: de::Error>(self, _expected: Option<&dyn de::Expected>) -> E {
@@ -124,7 +128,7 @@ macro_rules! serde_string_impl {
 }
 
 /// A combination macro where the human-readable serialization is done like
-/// serde_string_impl and the non-human-readable impl is done as a struct.
+/// `serde_string_impl` and the non-human-readable impl is done as a struct.
 #[macro_export]
 macro_rules! serde_struct_human_string_impl {
     ($name:ident, $expecting:literal, $($fe:ident),*) => (

internals/src/slice.rs

@@ -58,7 +58,7 @@ impl<T> SliceExt for [T] {
 
     fn bitcoin_as_chunks<const N: usize>(&self) -> (&[[Self::Item; N]], &[Self::Item]) {
         #[allow(clippy::let_unit_value)]
-        let _ = Hack::<N>::IS_NONZERO;
+        let () = Hack::<N>::IS_NONZERO;
 
         let chunks_count = self.len() / N;
         let total_left_len = chunks_count * N;
@@ -77,7 +77,7 @@ impl<T> SliceExt for [T] {
         &mut self,
     ) -> (&mut [[Self::Item; N]], &mut [Self::Item]) {
         #[allow(clippy::let_unit_value)]
-        let _ = Hack::<N>::IS_NONZERO;
+        let () = Hack::<N>::IS_NONZERO;
 
         let chunks_count = self.len() / N;
         let total_left_len = chunks_count * N;