Merge #127: Turn on a bunch of cippy ints

8e15fd3 clippy: invert != in if statements (Andrew Poelstra)
3fdacb5 clippy: replace flat_map with filter_map (Andrew Poelstra)
2c7b1fb clippy: don't use .iter and .iter_mut where not necessary (Andrew Poelstra)
9d5a796 clippy: don't use lossless casts (Andrew Poelstra)
53f5610 clippy: use built-in conversion from bool to numeric (Andrew Poelstra)
5130441 clippy: turn on all lints that don't trigger anywhere (Andrew Poelstra)
7a7d16b clippy: copy giant list of lints from bitcoin-units, with all "allow"ed (Andrew Poelstra)
1c2fa51 clippy: whitelist lints that fire on current nightly (Andrew Poelstra)

Pull request description:

  This is the first part of a project to turn on a ton of pedantic Clippy lints. We can choose not to do some if they're too annoying. There are about 20 left after this.

ACKs for top commit:
  uncomputable:
    ACK 8e15fd3 Looks good to me

Tree-SHA512: 3399a80ada2d28e79afbdc484b35c9cc4f3ff053899fa956d51c6bfe64560f98e5d07d46f8566bdc6f204ee047b7c56e552d023b58e90b54c573d86c65c7ff16

Author: apoelstra

Cargo.toml

@@ -38,3 +38,132 @@ getrandom = { version = "0.2", features = ["js"] }
 [workspace]
 members = ["codegen"]
 exclude = ["fuzz", "bitcoind-tests"]
+
+[lints.clippy]
+# Exclude lints we don't think are valuable.
+large_enum_variant = "allow" # docs say "measure before paying attention to this"; why is it on by default??
+similar_names = "allow" # Too many (subjectively) false positives.
+uninlined_format_args = "allow" # This is a subjective style choice.
+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.
+# Whitelist the cast lints because sometimes casts are unavoidable. But
+# every cast should contain a code comment!
+cast_possible_truncation = "allow"
+cast_possible_wrap = "allow"
+cast_sign_loss = "allow"
+# Exhaustive list of pedantic clippy lints
+assigning_clones = "warn"
+bool_to_int_with_if = "warn"
+borrow_as_ptr = "warn"
+case_sensitive_file_extension_comparisons = "warn"
+cast_lossless = "warn"
+cast_precision_loss = "warn"
+cast_ptr_alignment = "warn"
+checked_conversions = "warn"
+cloned_instead_of_copied = "warn"
+copy_iterator = "warn"
+default_trait_access = "warn"
+doc_link_with_quotes = "warn"
+doc_markdown = "allow"
+empty_enum = "warn"
+enum_glob_use = "allow"
+expl_impl_clone_on_copy = "warn"
+explicit_deref_methods = "warn"
+explicit_into_iter_loop = "warn"
+explicit_iter_loop = "warn"
+filter_map_next = "warn"
+flat_map_option = "warn"
+float_cmp = "warn"
+fn_params_excessive_bools = "warn"
+from_iter_instead_of_collect = "warn"
+if_not_else = "warn"
+ignored_unit_patterns = "allow"
+implicit_clone = "warn"
+implicit_hasher = "warn"
+inconsistent_struct_constructor = "warn"
+index_refutable_slice = "warn"
+inefficient_to_string = "allow"
+inline_always = "warn"
+into_iter_without_iter = "warn"
+invalid_upcast_comparisons = "warn"
+items_after_statements = "allow"
+iter_filter_is_ok = "warn"
+iter_filter_is_some = "warn"
+iter_not_returning_iterator = "warn"
+iter_without_into_iter = "warn"
+large_digit_groups = "warn"
+large_futures = "warn"
+large_stack_arrays = "warn"
+large_types_passed_by_value = "warn"
+linkedlist = "warn"
+macro_use_imports = "warn"
+manual_assert = "allow"
+manual_instant_elapsed = "warn"
+manual_is_power_of_two = "warn"
+manual_is_variant_and = "warn"
+manual_let_else = "allow"
+manual_ok_or = "warn"
+manual_string_new = "warn"
+many_single_char_names = "warn"
+map_unwrap_or = "allow"
+match_on_vec_items = "warn"
+match_wild_err_arm = "warn"
+match_wildcard_for_single_variants = "allow"
+maybe_infinite_iter = "warn"
+mismatching_type_param_order = "warn"
+missing_errors_doc = "allow"
+missing_fields_in_debug = "warn"
+missing_panics_doc = "allow"
+mut_mut = "warn"
+naive_bytecount = "warn"
+needless_bitwise_bool = "warn"
+needless_continue = "allow"
+needless_for_each = "warn"
+needless_pass_by_value = "allow"
+needless_raw_string_hashes = "allow"
+no_effect_underscore_binding = "warn"
+no_mangle_with_rust_abi = "warn"
+option_as_ref_cloned = "warn"
+option_option = "allow"
+ptr_as_ptr = "warn"
+ptr_cast_constness = "warn"
+pub_underscore_fields = "warn"
+range_minus_one = "warn"
+range_plus_one = "warn"
+redundant_closure_for_method_calls = "allow"
+redundant_else = "warn"
+ref_as_ptr = "warn"
+ref_binding_to_reference = "warn"
+ref_option = "warn"
+ref_option_ref = "warn"
+return_self_not_must_use = "allow"
+same_functions_in_if_condition = "warn"
+semicolon_if_nothing_returned = "allow"
+should_panic_without_expect = "warn"
+single_char_pattern = "warn"
+single_match_else = "allow"
+stable_sort_primitive = "warn"
+str_split_at_newline = "warn"
+string_add_assign = "warn"
+struct_excessive_bools = "warn"
+struct_field_names = "warn"
+too_many_lines = "allow"
+transmute_ptr_to_ptr = "warn"
+trivially_copy_pass_by_ref = "warn"
+unchecked_duration_subtraction = "warn"
+unicode_not_nfc = "warn"
+unnecessary_box_returns = "warn"
+unnecessary_join = "warn"
+unnecessary_literal_bound = "warn"
+unnecessary_wraps = "warn"
+unnested_or_patterns = "allow"
+unreadable_literal = "allow"
+unsafe_derive_deserialize = "warn"
+unused_async = "warn"
+unused_self = "warn"
+used_underscore_binding = "warn"
+used_underscore_items = "warn"
+verbose_bit_mask = "warn"
+wildcard_imports = "allow"
+zero_sized_map_values = "warn"

clippy.toml

@@ -1 +1,3 @@
 msrv = "1.78.0"
+# We have an error type, `RichError`, of size 144. This is pushing it but probably fine.
+large-error-threshold = 145

src/ast.rs

@@ -1039,27 +1039,27 @@ impl AbstractSyntaxTree for Call {
             parse_args: &[parse::Expression],
             expected_tys: &[ResolvedType],
         ) -> Result<(), Error> {
-            if parse_args.len() != expected_tys.len() {
+            if parse_args.len() == expected_tys.len() {
+                Ok(())
+            } else {
                 Err(Error::InvalidNumberOfArguments(
                     expected_tys.len(),
                     parse_args.len(),
                 ))
-            } else {
-                Ok(())
             }
         }
 
         fn check_output_type(
             observed_ty: &ResolvedType,
             expected_ty: &ResolvedType,
         ) -> Result<(), Error> {
-            if observed_ty != expected_ty {
+            if observed_ty == expected_ty {
+                Ok(())
+            } else {
                 Err(Error::ExpressionTypeMismatch(
                     expected_ty.clone(),
                     observed_ty.clone(),
                 ))
-            } else {
-                Ok(())
             }
         }
 

src/num.rs

@@ -298,8 +298,8 @@ impl fmt::Display for U256 {
             is_zero = true;
 
             // Divide by 10, starting at the most significant bytes
-            for byte in bytes.iter_mut() {
-                let value = carry * 256 + *byte as u32;
+            for byte in &mut bytes {
+                let value = carry * 256 + u32::from(*byte);
                 *byte = (value / 10) as u8;
                 carry = value % 10;
 
@@ -334,7 +334,7 @@ impl FromStr for U256 {
 
             // Add to the least significant bytes first
             for byte in bytes.iter_mut().rev() {
-                let value = *byte as u32 * 10 + carry;
+                let value = u32::from(*byte) * 10 + carry;
                 *byte = (value % 256) as u8;
                 carry = value / 256;
             }

src/pattern.rs

@@ -259,7 +259,7 @@ impl BasePattern {
 
     /// Get an iterator over all identifiers inside the pattern.
     pub fn identifiers(&self) -> impl Iterator<Item = &Identifier> {
-        self.pre_order_iter().flat_map(BasePattern::as_identifier)
+        self.pre_order_iter().filter_map(BasePattern::as_identifier)
     }
 
     /// Check if all `identifiers` are contained inside the pattern.

src/serde.rs

@@ -144,7 +144,7 @@ impl<'a> Serialize for WitnessMapSerializer<'a> {
         S: Serializer,
     {
         let mut map = serializer.serialize_map(Some(self.0.len()))?;
-        for (name, value) in self.0.iter() {
+        for (name, value) in self.0 {
             map.serialize_entry(name.as_inner(), &ValueMapSerializer(value))?;
         }
         map.end()

src/value.rs

@@ -155,7 +155,7 @@ impl UIntValue {
             let mut byte = 0u8;
             for _ in 0..8 {
                 let bit = padded_bits.next().unwrap();
-                byte = (byte << 1) | if bit == '1' { 1 } else { 0 };
+                byte = (byte << 1) | u8::from(bit == '1');
             }
             bytes.push(byte);
         }
@@ -934,7 +934,7 @@ impl ValueConstructible for StructuralValue {
 
     fn array<I: IntoIterator<Item = Self>>(elements: I, ty: Self::Type) -> Self {
         let elements: Vec<Self> = elements.into_iter().collect();
-        for element in elements.iter() {
+        for element in &elements {
             assert!(
                 element.is_of_type(&ty),
                 "Element {element} is not of expected type {ty}"
@@ -954,7 +954,7 @@ impl ValueConstructible for StructuralValue {
             elements.len() < bound.get(),
             "There must be fewer list elements than the bound {bound}"
         );
-        for element in elements.iter() {
+        for element in &elements {
             assert!(
                 element.is_of_type(&ty),
                 "Element {element} is not of expected type {ty}"