From 5290a93a38980cac2cb9e176a1a26c7537647dc6 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 9 Dec 2024 09:31:20 +1100 Subject: [PATCH 1/3] units: Add all pedantic lints Add all the pedantic lints to the repository by way of the repository manifest. Then enable these lints in the `units` manifest. Some things worth mentioning: - Fix `needless_pass_by_value` by adding derives to `FormatOptions`. - Fix lint `cast_lossless` using `cargo clippy --fix`` - While fixing `lint enum_glob_use` introduce a new style to the codebase; import enums using a single character. Doing so prevents namespace clashes, improves clarity, and maintains terseness. Audit: Use the following lints locally and audit all the warnings, they produce many false positives so we can't enable them permentently. - `cast_possible_truncation` - `cast_possible_lint` - `cast_sign_loss` --- units/Cargo.toml | 125 +++++++++++++++++++++++++++++++++ units/src/amount/error.rs | 41 +++++------ units/src/amount/mod.rs | 27 +++---- units/src/amount/signed.rs | 6 +- units/src/amount/tests.rs | 76 +++++++++++--------- units/src/amount/unsigned.rs | 5 +- units/src/block.rs | 4 +- units/src/fee_rate.rs | 2 +- units/src/lib.rs | 4 +- units/src/locktime/absolute.rs | 34 ++++----- units/src/locktime/relative.rs | 4 +- units/src/parse.rs | 28 ++++---- units/src/weight.rs | 4 +- 13 files changed, 247 insertions(+), 113 deletions(-) diff --git a/units/Cargo.toml b/units/Cargo.toml index 21d5c9b17..e3bd679a8 100644 --- a/units/Cargo.toml +++ b/units/Cargo.toml @@ -34,3 +34,128 @@ rustdoc-args = ["--cfg", "docsrs"] [lints.rust] unexpected_cfgs = { level = "deny", check-cfg = ['cfg(kani)'] } + +[lints.clippy] +# 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_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). +checked_conversions = "warn" +cloned_instead_of_copied = "warn" +copy_iterator = "warn" +default_trait_access = "warn" +doc_link_with_quotes = "warn" +doc_markdown = "warn" +empty_enum = "warn" +enum_glob_use = "warn" +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 = "allow" # Bitcoin floats are typically limited to 8 decimal places and we want them exact. +fn_params_excessive_bools = "warn" +from_iter_instead_of_collect = "warn" +if_not_else = "warn" +ignored_unit_patterns = "warn" +implicit_clone = "warn" +implicit_hasher = "warn" +inconsistent_struct_constructor = "warn" +index_refutable_slice = "warn" +inefficient_to_string = "warn" +inline_always = "warn" +into_iter_without_iter = "warn" +invalid_upcast_comparisons = "warn" +items_after_statements = "warn" +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 = "warn" +manual_instant_elapsed = "warn" +manual_is_power_of_two = "warn" +manual_is_variant_and = "warn" +manual_let_else = "warn" +manual_ok_or = "warn" +manual_string_new = "warn" +many_single_char_names = "warn" +map_unwrap_or = "warn" +match_bool = "allow" # Adds extra indentation and LOC. +match_on_vec_items = "warn" +match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other. +match_wild_err_arm = "warn" +match_wildcard_for_single_variants = "warn" +maybe_infinite_iter = "warn" +mismatching_type_param_order = "warn" +missing_errors_doc = "allow" # TODO: Still needs considering. +missing_fields_in_debug = "warn" +missing_panics_doc = "allow" # TODO: Still needs considering. +must_use_candidate = "allow" # Useful for audit but many false positives. +mut_mut = "warn" +naive_bytecount = "warn" +needless_bitwise_bool = "warn" +needless_continue = "warn" +needless_for_each = "warn" +needless_pass_by_value = "warn" +needless_raw_string_hashes = "warn" +no_effect_underscore_binding = "warn" +no_mangle_with_rust_abi = "warn" +option_as_ref_cloned = "warn" +option_option = "warn" +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 = "warn" +redundant_else = "warn" +ref_as_ptr = "warn" +ref_binding_to_reference = "warn" +ref_option = "allow" # TODO: Still neds considering. +ref_option_ref = "warn" +return_self_not_must_use = "warn" +same_functions_in_if_condition = "warn" +semicolon_if_nothing_returned = "warn" +should_panic_without_expect = "warn" +similar_names = "allow" # Too many (subjectively) false positives. +single_char_pattern = "warn" +single_match_else = "warn" +stable_sort_primitive = "warn" +str_split_at_newline = "warn" +string_add_assign = "warn" +struct_excessive_bools = "warn" +struct_field_names = "warn" +too_many_lines = "warn" +transmute_ptr_to_ptr = "warn" +trivially_copy_pass_by_ref = "warn" +unchecked_duration_subtraction = "warn" +unicode_not_nfc = "warn" +uninlined_format_args = "allow" # This is a subjective style choice. +unnecessary_box_returns = "warn" +unnecessary_join = "warn" +unnecessary_literal_bound = "warn" +unnecessary_wraps = "warn" +unnested_or_patterns = "warn" +unreadable_literal = "warn" +unsafe_derive_deserialize = "warn" +unused_async = "warn" +unused_self = "warn" +used_underscore_binding = "warn" +used_underscore_items = "warn" +verbose_bit_mask = "warn" +wildcard_imports = "warn" +zero_sized_map_values = "warn" diff --git a/units/src/amount/error.rs b/units/src/amount/error.rs index 55939b5ae..d90e70c5a 100644 --- a/units/src/amount/error.rs +++ b/units/src/amount/error.rs @@ -119,14 +119,15 @@ internals::impl_from_infallible!(ParseAmountErrorInner); impl fmt::Display for ParseAmountError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use ParseAmountErrorInner::*; + use ParseAmountErrorInner as E; match self.0 { - OutOfRange(ref error) => write_err!(f, "amount out of range"; error), - TooPrecise(ref error) => write_err!(f, "amount has a too high precision"; error), - MissingDigits(ref error) => write_err!(f, "the input has too few digits"; error), - InputTooLarge(ref error) => write_err!(f, "the input is too large"; error), - InvalidCharacter(ref error) => write_err!(f, "invalid character in the input"; error), + E::OutOfRange(ref error) => write_err!(f, "amount out of range"; error), + E::TooPrecise(ref error) => write_err!(f, "amount has a too high precision"; error), + E::MissingDigits(ref error) => write_err!(f, "the input has too few digits"; error), + E::InputTooLarge(ref error) => write_err!(f, "the input is too large"; error), + E::InvalidCharacter(ref error) => + write_err!(f, "invalid character in the input"; error), } } } @@ -134,14 +135,14 @@ impl fmt::Display for ParseAmountError { #[cfg(feature = "std")] impl std::error::Error for ParseAmountError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use ParseAmountErrorInner::*; + use ParseAmountErrorInner as E; match self.0 { - TooPrecise(ref error) => Some(error), - InputTooLarge(ref error) => Some(error), - OutOfRange(ref error) => Some(error), - MissingDigits(ref error) => Some(error), - InvalidCharacter(ref error) => Some(error), + E::TooPrecise(ref error) => Some(error), + E::InputTooLarge(ref error) => Some(error), + E::OutOfRange(ref error) => Some(error), + E::MissingDigits(ref error) => Some(error), + E::InvalidCharacter(ref error) => Some(error), } } } @@ -157,7 +158,7 @@ impl OutOfRangeError { /// Returns the minimum and maximum allowed values for the type that was parsed. /// /// This can be used to give a hint to the user which values are allowed. - pub fn valid_range(&self) -> (i64, u64) { + pub fn valid_range(self) -> (i64, u64) { match self.is_signed { true => (i64::MIN, i64::MAX as u64), false => (0, u64::MAX), @@ -165,10 +166,10 @@ impl OutOfRangeError { } /// Returns true if the input value was large than the maximum allowed value. - pub fn is_above_max(&self) -> bool { self.is_greater_than_max } + pub fn is_above_max(self) -> bool { self.is_greater_than_max } /// Returns true if the input value was smaller than the minimum allowed value. - pub fn is_below_min(&self) -> bool { !self.is_greater_than_max } + pub fn is_below_min(self) -> bool { !self.is_greater_than_max } pub(crate) fn too_big(is_signed: bool) -> Self { Self { is_signed, is_greater_than_max: true } } @@ -321,11 +322,11 @@ internals::impl_from_infallible!(ParseDenominationError); impl fmt::Display for ParseDenominationError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use ParseDenominationError::*; + use ParseDenominationError as E; match *self { - Unknown(ref e) => write_err!(f, "denomination parse error"; e), - PossiblyConfusing(ref e) => write_err!(f, "denomination parse error"; e), + E::Unknown(ref e) => write_err!(f, "denomination parse error"; e), + E::PossiblyConfusing(ref e) => write_err!(f, "denomination parse error"; e), } } } @@ -333,10 +334,10 @@ impl fmt::Display for ParseDenominationError { #[cfg(feature = "std")] impl std::error::Error for ParseDenominationError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use ParseDenominationError::*; + use ParseDenominationError as E; match *self { - Unknown(_) | PossiblyConfusing(_) => None, + E::Unknown(_) | E::PossiblyConfusing(_) => None, } } } diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs index d47410e61..9a364fd08 100644 --- a/units/src/amount/mod.rs +++ b/units/src/amount/mod.rs @@ -124,8 +124,8 @@ impl Denomination { } } -/// These form are ambigous and could have many meanings. For example, M could denote Mega or Milli. -/// If any of these forms are used, an error type PossiblyConfusingDenomination is returned. +/// These form are ambigous and could have many meanings. For example, M could denote Mega or Milli. +/// If any of these forms are used, an error type `PossiblyConfusingDenomination` is returned. const CONFUSING_FORMS: [&str; 6] = ["CBTC", "Cbtc", "MBTC", "Mbtc", "UBTC", "Ubtc"]; impl fmt::Display for Denomination { @@ -143,15 +143,15 @@ impl FromStr for Denomination { /// letter that could be confused with centi, milli, or micro-bitcoin. /// - [`ParseDenominationError::Unknown`]: If an unknown denomination is used. fn from_str(s: &str) -> Result { - use self::ParseDenominationError::*; + use self::ParseDenominationError as E; if CONFUSING_FORMS.contains(&s) { - return Err(PossiblyConfusing(PossiblyConfusingDenominationError(s.into()))); + return Err(E::PossiblyConfusing(PossiblyConfusingDenominationError(s.into()))); }; let form = self::Denomination::forms(s); - form.ok_or_else(|| Unknown(UnknownDenominationError(s.into()))) + form.ok_or_else(|| E::Unknown(UnknownDenominationError(s.into()))) } } @@ -217,7 +217,7 @@ fn parse_signed_to_satoshi( Ok(0) => return Ok((is_negative, 0)), _ => return Err(InnerParseError::TooPrecise(TooPreciseError { - position: position + is_negative as usize, + position: position + usize::from(is_negative), })), } } @@ -236,7 +236,7 @@ fn parse_signed_to_satoshi( // Do `value = 10 * value + digit`, catching overflows. match 10_u64.checked_mul(value) { None => return Err(InnerParseError::Overflow { is_negative }), - Some(val) => match val.checked_add((c as u8 - b'0') as u64) { + Some(val) => match val.checked_add(u64::from(c as u8 - b'0')) { None => return Err(InnerParseError::Overflow { is_negative }), Some(val) => value = val, }, @@ -247,7 +247,7 @@ fn parse_signed_to_satoshi( Some(d) if d < max_decimals => Some(d + 1), _ => return Err(InnerParseError::TooPrecise(TooPreciseError { - position: i + is_negative as usize, + position: i + usize::from(is_negative), })), }; } @@ -258,13 +258,13 @@ fn parse_signed_to_satoshi( _ => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: '.', - position: i + is_negative as usize, + position: i + usize::from(is_negative), })), }, c => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: c, - position: i + is_negative as usize, + position: i + usize::from(is_negative), })), } } @@ -319,6 +319,7 @@ fn split_amount_and_denomination(s: &str) -> Result<(&str, Denomination), ParseE } /// Options given by `fmt::Formatter` +#[derive(Debug, Clone, Copy, Eq, PartialEq)] struct FormatOptions { fill: char, align: Option, @@ -394,7 +395,7 @@ fn fmt_satoshi_in( // We add the number of zeroes to the end Ordering::Greater => { if satoshi > 0 { - exp = precision as usize; + exp = precision as usize; // Cast ok, checked not negative above. } trailing_decimal_zeros = options.precision.unwrap_or(0); } @@ -406,7 +407,7 @@ fn fmt_satoshi_in( if usize::from(precision) > format_precision { // precision is u8 so in this branch options.precision() < 255 which fits in u32 let rounding_divisor = - 10u64.pow(u32::from(precision) - format_precision as u32); + 10u64.pow(u32::from(precision) - format_precision as u32); // Cast ok, commented above. let remainder = satoshi % rounding_divisor; satoshi -= remainder; if remainder / (rounding_divisor / 10) >= 5 { @@ -424,7 +425,7 @@ fn fmt_satoshi_in( norm_nb_decimals = usize::from(precision); while num_after_decimal_point % 10 == 0 { norm_nb_decimals -= 1; - num_after_decimal_point /= 10 + num_after_decimal_point /= 10; } } // compute requested precision diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 42d15cfea..a817566e2 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -111,11 +111,11 @@ impl SignedAmount { (false, sat) if sat > SignedAmount::MAX.to_sat() as u64 => Err(ParseAmountError( ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_big(true)), )), - (false, sat) => Ok(SignedAmount(sat as i64)), + (false, sat) => Ok(SignedAmount(sat as i64)), // Cast ok, value in this arm does not wrap. (true, sat) if sat > SignedAmount::MIN.to_sat().unsigned_abs() => Err( ParseAmountError(ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_small())), ), - (true, sat) => Ok(SignedAmount(-(sat as i64))), + (true, sat) => Ok(SignedAmount(-(sat as i64))), // Cast ok, value in this arm does not wrap. } } @@ -349,7 +349,7 @@ impl SignedAmount { if self.is_negative() { Err(OutOfRangeError::negative()) } else { - Ok(Amount::from_sat(self.to_sat() as u64)) + Ok(Amount::from_sat(self.to_sat() as u64)) // Cast ok, checked not negative above. } } diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 6996dd8af..c5dd5330f 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -114,7 +114,7 @@ fn test_overflows() { // panic on overflow let result = panic::catch_unwind(|| Amount::MAX + Amount::from_sat(1)); assert!(result.is_err()); - let result = panic::catch_unwind(|| Amount::from_sat(8446744073709551615) * 3); + let result = panic::catch_unwind(|| Amount::from_sat(8_446_744_073_709_551_615) * 3); assert!(result.is_err()); } @@ -205,16 +205,19 @@ fn floating_point() { let sat = Amount::from_sat; let ssat = SignedAmount::from_sat; - assert_eq!(f(11.22, D::Bitcoin), Ok(sat(1122000000))); - assert_eq!(sf(-11.22, D::MilliBitcoin), Ok(ssat(-1122000))); + assert_eq!(f(11.22, D::Bitcoin), Ok(sat(1_122_000_000))); + assert_eq!(sf(-11.22, D::MilliBitcoin), Ok(ssat(-1_122_000))); assert_eq!(f(11.22, D::Bit), Ok(sat(1122))); - assert_eq!(f(0.0001234, D::Bitcoin), Ok(sat(12340))); - assert_eq!(sf(-0.00012345, D::Bitcoin), Ok(ssat(-12345))); + assert_eq!(f(0.000_123_4, D::Bitcoin), Ok(sat(12_340))); + assert_eq!(sf(-0.000_123_45, D::Bitcoin), Ok(ssat(-12_345))); assert_eq!(f(11.22, D::Satoshi), Err(TooPreciseError { position: 3 }.into())); - assert_eq!(f(42.123456781, D::Bitcoin), Err(TooPreciseError { position: 11 }.into())); - assert_eq!(sf(-184467440738.0, D::Bitcoin), Err(OutOfRangeError::too_small().into())); - assert_eq!(f(18446744073709551617.0, D::Satoshi), Err(OutOfRangeError::too_big(false).into())); + assert_eq!(f(42.123_456_781, D::Bitcoin), Err(TooPreciseError { position: 11 }.into())); + assert_eq!(sf(-184_467_440_738.0, D::Bitcoin), Err(OutOfRangeError::too_small().into())); + assert_eq!( + f(18_446_744_073_709_551_617.0, D::Satoshi), + Err(OutOfRangeError::too_big(false).into()) + ); assert_eq!( f(Amount::MAX.to_float_in(D::Satoshi) + 1.0, D::Satoshi), @@ -230,10 +233,10 @@ fn floating_point() { assert_eq!(btc(2.5).to_float_in(D::Bitcoin), 2.5); assert_eq!(btc(-2.5).to_float_in(D::CentiBitcoin), -250.0); assert_eq!(btc(-2.5).to_float_in(D::MilliBitcoin), -2500.0); - assert_eq!(btc(2.5).to_float_in(D::Satoshi), 250000000.0); + assert_eq!(btc(2.5).to_float_in(D::Satoshi), 250_000_000.0); let btc = move |f| Amount::from_btc(f).unwrap(); - assert_eq!(&btc(0.0012).to_float_in(D::Bitcoin).to_string(), "0.0012") + assert_eq!(&btc(0.0012).to_float_in(D::Bitcoin).to_string(), "0.0012"); } #[test] @@ -508,6 +511,7 @@ fn test_unsigned_signed_conversion() { #[test] #[allow(clippy::inconsistent_digit_grouping)] // Group to show 100,000,000 sats per bitcoin. +#[allow(clippy::items_after_statements)] // Define functions where we use them. fn from_str() { use ParseDenominationError::*; @@ -577,7 +581,7 @@ fn from_str() { ok_case(".5 bits", Amount::from_sat(50)); ok_scase("-.5 bits", SignedAmount::from_sat(-50)); - ok_case("0.00253583 BTC", Amount::from_sat(253583)); + ok_case("0.00253583 BTC", Amount::from_sat(253_583)); ok_scase("-5 satoshi", SignedAmount::from_sat(-5)); ok_case("0.10000000 BTC", Amount::from_sat(100_000_00)); ok_scase("-100 bits", SignedAmount::from_sat(-10_000)); @@ -596,23 +600,25 @@ fn to_from_string_in() { let sa_str = SignedAmount::from_str_in; let sa_sat = SignedAmount::from_sat; - assert_eq!("0.00253583", Amount::from_sat(253583).to_string_in(D::Bitcoin)); - assert_eq!("-0.00253583", SignedAmount::from_sat(-253583).to_string_in(D::Bitcoin)); + assert_eq!("0.5", Amount::from_sat(50).to_string_in(D::Bit)); + assert_eq!("-0.5", SignedAmount::from_sat(-50).to_string_in(D::Bit)); + assert_eq!("0.00253583", Amount::from_sat(253_583).to_string_in(D::Bitcoin)); + assert_eq!("-5", SignedAmount::from_sat(-5).to_string_in(D::Satoshi)); assert_eq!("0.1", Amount::from_sat(100_000_00).to_string_in(D::Bitcoin)); assert_eq!("-0.1", SignedAmount::from_sat(-100_000_00).to_string_in(D::Bitcoin)); - assert_eq!("0.253583", Amount::from_sat(253583).to_string_in(D::CentiBitcoin)); - assert_eq!("-0.253583", SignedAmount::from_sat(-253583).to_string_in(D::CentiBitcoin)); + assert_eq!("0.253583", Amount::from_sat(253_583).to_string_in(D::CentiBitcoin)); + assert_eq!("-0.253583", SignedAmount::from_sat(-253_583).to_string_in(D::CentiBitcoin)); assert_eq!("10", Amount::from_sat(100_000_00).to_string_in(D::CentiBitcoin)); assert_eq!("-10", SignedAmount::from_sat(-100_000_00).to_string_in(D::CentiBitcoin)); - assert_eq!("2.53583", Amount::from_sat(253583).to_string_in(D::MilliBitcoin)); - assert_eq!("-2.53583", SignedAmount::from_sat(-253583).to_string_in(D::MilliBitcoin)); + assert_eq!("2.53583", Amount::from_sat(253_583).to_string_in(D::MilliBitcoin)); + assert_eq!("-2.53583", SignedAmount::from_sat(-253_583).to_string_in(D::MilliBitcoin)); assert_eq!("100", Amount::from_sat(100_000_00).to_string_in(D::MilliBitcoin)); assert_eq!("-100", SignedAmount::from_sat(-100_000_00).to_string_in(D::MilliBitcoin)); - assert_eq!("2535.83", Amount::from_sat(253583).to_string_in(D::MicroBitcoin)); - assert_eq!("-2535.83", SignedAmount::from_sat(-253583).to_string_in(D::MicroBitcoin)); + assert_eq!("2535.83", Amount::from_sat(253_583).to_string_in(D::MicroBitcoin)); + assert_eq!("-2535.83", SignedAmount::from_sat(-253_583).to_string_in(D::MicroBitcoin)); assert_eq!("100000", Amount::from_sat(100_000_00).to_string_in(D::MicroBitcoin)); assert_eq!("-100000", SignedAmount::from_sat(-100_000_00).to_string_in(D::MicroBitcoin)); @@ -699,13 +705,13 @@ fn serde_as_sat() { } serde_test::assert_tokens( - &T { amt: Amount::from_sat(123456789), samt: SignedAmount::from_sat(-123456789) }, + &T { amt: Amount::from_sat(123_456_789), samt: SignedAmount::from_sat(-123_456_789) }, &[ serde_test::Token::Struct { name: "T", len: 2 }, serde_test::Token::Str("amt"), - serde_test::Token::U64(123456789), + serde_test::Token::U64(123_456_789), serde_test::Token::Str("samt"), - serde_test::Token::I64(-123456789), + serde_test::Token::I64(-123_456_789), serde_test::Token::StructEnd, ], ); @@ -763,7 +769,7 @@ fn serde_as_str() { } serde_test::assert_tokens( - &T { amt: Amount::from_sat(123456789), samt: SignedAmount::from_sat(-123456789) }, + &T { amt: Amount::from_sat(123_456_789), samt: SignedAmount::from_sat(-123_456_789) }, &[ serde_test::Token::Struct { name: "T", len: 2 }, serde_test::Token::String("amt"), @@ -797,10 +803,10 @@ fn serde_as_btc_opt() { let without = T { amt: None, samt: None }; // Test Roundtripping - for s in [&with, &without].iter() { + for s in [&with, &without] { let v = serde_json::to_string(s).unwrap(); let w: T = serde_json::from_str(&v).unwrap(); - assert_eq!(w, **s); + assert_eq!(w, *s); } let t: T = serde_json::from_str("{\"amt\": 2.5, \"samt\": -2.5}").unwrap(); @@ -839,10 +845,10 @@ fn serde_as_sat_opt() { let without = T { amt: None, samt: None }; // Test Roundtripping - for s in [&with, &without].iter() { + for s in [&with, &without] { let v = serde_json::to_string(s).unwrap(); let w: T = serde_json::from_str(&v).unwrap(); - assert_eq!(w, **s); + assert_eq!(w, *s); } let t: T = serde_json::from_str("{\"amt\": 250000000, \"samt\": -250000000}").unwrap(); @@ -875,16 +881,16 @@ fn serde_as_str_opt() { } let with = T { - amt: Some(Amount::from_sat(123456789)), - samt: Some(SignedAmount::from_sat(-123456789)), + amt: Some(Amount::from_sat(123_456_789)), + samt: Some(SignedAmount::from_sat(-123_456_789)), }; let without = T { amt: None, samt: None }; // Test Roundtripping - for s in [&with, &without].iter() { + for s in [&with, &without] { let v = serde_json::to_string(s).unwrap(); let w: T = serde_json::from_str(&v).unwrap(); - assert_eq!(w, **s); + assert_eq!(w, *s); } let t: T = @@ -953,7 +959,7 @@ fn denomination_string_acceptable_forms() { "BTC", "btc", "cBTC", "cbtc", "mBTC", "mbtc", "uBTC", "ubtc", "bit", "bits", "BIT", "BITS", "SATOSHI", "satoshi", "SATOSHIS", "satoshis", "SAT", "sat", "SATS", "sats", ]; - for denom in valid.iter() { + for denom in valid { assert!(denom.parse::().is_ok()); } } @@ -961,7 +967,7 @@ fn denomination_string_acceptable_forms() { #[test] fn disallow_confusing_forms() { let confusing = ["CBTC", "Cbtc", "MBTC", "Mbtc", "UBTC", "Ubtc"]; - for denom in confusing.iter() { + for denom in confusing { match denom.parse::() { Ok(_) => panic!("from_str should error for {}", denom), Err(ParseDenominationError::PossiblyConfusing(_)) => {} @@ -974,7 +980,7 @@ fn disallow_confusing_forms() { fn disallow_unknown_denomination() { // Non-exhaustive list of unknown forms. let unknown = ["NBTC", "ABC", "abc", "mSat", "msat"]; - for denom in unknown.iter() { + for denom in unknown { match denom.parse::() { Ok(_) => panic!("from_str should error for {}", denom), Err(ParseDenominationError::Unknown(_)) => (), @@ -986,7 +992,7 @@ fn disallow_unknown_denomination() { #[test] #[cfg(feature = "alloc")] fn trailing_zeros_for_amount() { - assert_eq!(format!("{}", Amount::from_sat(1000000)), "0.01 BTC"); + assert_eq!(format!("{}", Amount::from_sat(1_000_000)), "0.01 BTC"); assert_eq!(format!("{}", Amount::ONE_SAT), "0.00000001 BTC"); assert_eq!(format!("{}", Amount::ONE_BTC), "1 BTC"); assert_eq!(format!("{}", Amount::from_sat(1)), "0.00000001 BTC"); diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 93bde1de6..be18ea8f2 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -414,11 +414,12 @@ impl Amount { /// # Errors /// /// If the amount is too big. + #[rustfmt::skip] // Moves code comments to the wrong line. pub fn to_signed(self) -> Result { - if self.to_sat() > SignedAmount::MAX.to_sat() as u64 { + if self.to_sat() > SignedAmount::MAX.to_sat() as u64 { // Cast ok, signed max is positive and fits in u64. Err(OutOfRangeError::too_big(true)) } else { - Ok(SignedAmount::from_sat(self.to_sat() as i64)) + Ok(SignedAmount::from_sat(self.to_sat() as i64)) // Cast ok, checked not too big above. } } diff --git a/units/src/block.rs b/units/src/block.rs index bdbf886b0..10e9208d1 100644 --- a/units/src/block.rs +++ b/units/src/block.rs @@ -148,10 +148,10 @@ impl TryFrom for relative::Height { /// [`BlockInterval`] is a thin wrapper around a `u32`, the two types are not interchangeable. fn try_from(h: BlockInterval) -> Result { let h = h.to_u32(); - if h > u16::MAX as u32 { + if h > u32::from(u16::MAX) { return Err(TooBigForRelativeBlockHeightError(h)); } - Ok(relative::Height::from(h as u16)) // Cast ok, value checked above + Ok(relative::Height::from(h as u16)) // Cast ok, value checked above. } } diff --git a/units/src/fee_rate.rs b/units/src/fee_rate.rs index 6700434cd..9f84a766c 100644 --- a/units/src/fee_rate.rs +++ b/units/src/fee_rate.rs @@ -356,7 +356,7 @@ mod tests { #[test] #[cfg(debug_assertions)] - #[should_panic] + #[should_panic = "attempt to multiply with overflow"] fn from_sat_per_vb_unchecked_panic() { FeeRate::from_sat_per_vb_unchecked(u64::MAX); } #[test] diff --git a/units/src/lib.rs b/units/src/lib.rs index ffcbfa3d7..d53bc76d0 100644 --- a/units/src/lib.rs +++ b/units/src/lib.rs @@ -11,8 +11,6 @@ #![warn(missing_docs)] #![warn(deprecated_in_future)] #![doc(test(attr(warn(unused))))] -// Pedantic lints that we enforce. -#![warn(clippy::return_self_not_must_use)] // 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. @@ -27,7 +25,7 @@ mod internal_macros; #[doc(hidden)] pub mod _export { - /// A re-export of core::* + /// A re-export of `core::*`. pub mod _core { pub use core::*; } diff --git a/units/src/locktime/absolute.rs b/units/src/locktime/absolute.rs index 4ee23e3dd..1ab4b82cf 100644 --- a/units/src/locktime/absolute.rs +++ b/units/src/locktime/absolute.rs @@ -285,11 +285,13 @@ enum LockTimeUnit { impl fmt::Display for LockTimeUnit { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use LockTimeUnit::*; + use LockTimeUnit as L; match *self { - Blocks => write!(f, "expected lock-by-blockheight (must be < {})", LOCK_TIME_THRESHOLD), - Seconds => write!(f, "expected lock-by-blocktime (must be >= {})", LOCK_TIME_THRESHOLD), + L::Blocks => + write!(f, "expected lock-by-blockheight (must be < {})", LOCK_TIME_THRESHOLD), + L::Seconds => + write!(f, "expected lock-by-blocktime (must be >= {})", LOCK_TIME_THRESHOLD), } } } @@ -321,10 +323,10 @@ impl ParseError { ) -> fmt::Result { use core::num::IntErrorKind; - use ParseError::*; + use ParseError as E; match self { - ParseInt(ParseIntError { input, bits: _, is_signed: _, source }) + E::ParseInt(ParseIntError { input, bits: _, is_signed: _, source }) if *source.kind() == IntErrorKind::PosOverflow => { // Outputs "failed to parse as absolute Height/Time ( is above limit )" @@ -336,7 +338,7 @@ impl ParseError { upper_bound ) } - ParseInt(ParseIntError { input, bits: _, is_signed: _, source }) + E::ParseInt(ParseIntError { input, bits: _, is_signed: _, source }) if *source.kind() == IntErrorKind::NegOverflow => { // Outputs "failed to parse as absolute Height/Time ( is below limit )" @@ -348,13 +350,13 @@ impl ParseError { lower_bound ) } - ParseInt(ParseIntError { input, bits: _, is_signed: _, source: _ }) => { + E::ParseInt(ParseIntError { input, bits: _, is_signed: _, source: _ }) => { write!(f, "{} ({})", input.display_cannot_parse("absolute Height/Time"), subject) } - Conversion(value) if *value < i64::from(lower_bound) => { + E::Conversion(value) if *value < i64::from(lower_bound) => { write!(f, "{} {} is below limit {}", subject, value, lower_bound) } - Conversion(value) => { + E::Conversion(value) => { write!(f, "{} {} is above limit {}", subject, value, upper_bound) } } @@ -365,17 +367,17 @@ impl ParseError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { use core::num::IntErrorKind; - use ParseError::*; + use ParseError as E; match self { - ParseInt(ParseIntError { source, .. }) + E::ParseInt(ParseIntError { source, .. }) if *source.kind() == IntErrorKind::PosOverflow => None, - ParseInt(ParseIntError { source, .. }) + E::ParseInt(ParseIntError { source, .. }) if *source.kind() == IntErrorKind::NegOverflow => None, - ParseInt(ParseIntError { source, .. }) => Some(source), - Conversion(_) => None, + E::ParseInt(ParseIntError { source, .. }) => Some(source), + E::Conversion(_) => None, } } } @@ -394,14 +396,14 @@ mod tests { #[test] fn time_from_str_hex_happy_path() { let actual = Time::from_hex("0x6289C350").unwrap(); - let expected = Time::from_consensus(0x6289C350).unwrap(); + let expected = Time::from_consensus(0x6289_C350).unwrap(); assert_eq!(actual, expected); } #[test] fn time_from_str_hex_no_prefix_happy_path() { let time = Time::from_hex("6289C350").unwrap(); - assert_eq!(time, Time(0x6289C350)); + assert_eq!(time, Time(0x6289_C350)); } #[test] diff --git a/units/src/locktime/relative.rs b/units/src/locktime/relative.rs index 401ac2ced..283ec1aaa 100644 --- a/units/src/locktime/relative.rs +++ b/units/src/locktime/relative.rs @@ -84,7 +84,7 @@ impl Time { pub const fn from_seconds_floor(seconds: u32) -> Result { let interval = seconds / 512; if interval <= u16::MAX as u32 { // infallible cast, needed by const code - Ok(Time::from_512_second_intervals(interval as u16)) // cast checked above, needed by const code + Ok(Time::from_512_second_intervals(interval as u16)) // Cast checked above, needed by const code. } else { Err(TimeOverflowError { seconds }) } @@ -101,7 +101,7 @@ impl Time { pub const fn from_seconds_ceil(seconds: u32) -> Result { if seconds <= u16::MAX as u32 * 512 { let interval = (seconds + 511) / 512; - Ok(Time::from_512_second_intervals(interval as u16)) // cast checked above, needed by const code + Ok(Time::from_512_second_intervals(interval as u16)) // Cast checked above, needed by const code. } else { Err(TimeOverflowError { seconds }) } diff --git a/units/src/parse.rs b/units/src/parse.rs index 3dc01f8b0..ec36d4b94 100644 --- a/units/src/parse.rs +++ b/units/src/parse.rs @@ -318,11 +318,11 @@ internals::impl_from_infallible!(PrefixedHexErrorInner); impl fmt::Display for PrefixedHexError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use PrefixedHexErrorInner::*; + use PrefixedHexErrorInner as E; match self.0 { - MissingPrefix(ref e) => write_err!(f, "hex string is missing prefix"; e), - ParseInt(ref e) => write_err!(f, "prefixed hex string invalid int"; e), + E::MissingPrefix(ref e) => write_err!(f, "hex string is missing prefix"; e), + E::ParseInt(ref e) => write_err!(f, "prefixed hex string invalid int"; e), } } } @@ -330,11 +330,11 @@ impl fmt::Display for PrefixedHexError { #[cfg(feature = "std")] impl std::error::Error for PrefixedHexError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use PrefixedHexErrorInner::*; + use PrefixedHexErrorInner as E; match self.0 { - MissingPrefix(ref e) => Some(e), - ParseInt(ref e) => Some(e), + E::MissingPrefix(ref e) => Some(e), + E::ParseInt(ref e) => Some(e), } } } @@ -364,11 +364,11 @@ internals::impl_from_infallible!(UnprefixedHexErrorInner); impl fmt::Display for UnprefixedHexError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use UnprefixedHexErrorInner::*; + use UnprefixedHexErrorInner as E; match self.0 { - ContainsPrefix(ref e) => write_err!(f, "hex string is contains prefix"; e), - ParseInt(ref e) => write_err!(f, "hex string parse int"; e), + E::ContainsPrefix(ref e) => write_err!(f, "hex string is contains prefix"; e), + E::ParseInt(ref e) => write_err!(f, "hex string parse int"; e), } } } @@ -376,11 +376,11 @@ impl fmt::Display for UnprefixedHexError { #[cfg(feature = "std")] impl std::error::Error for UnprefixedHexError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use UnprefixedHexErrorInner::*; + use UnprefixedHexErrorInner as E; match self.0 { - ContainsPrefix(ref e) => Some(e), - ParseInt(ref e) => Some(e), + E::ContainsPrefix(ref e) => Some(e), + E::ParseInt(ref e) => Some(e), } } } @@ -453,14 +453,14 @@ mod tests { #[test] fn parse_u128_from_hex_prefixed() { - let want = 3735928559; + let want = 3_735_928_559; let got = hex_u128("0xdeadbeef").expect("failed to parse prefixed hex"); assert_eq!(got, want); } #[test] fn parse_u128_from_hex_no_prefix() { - let want = 3735928559; + let want = 3_735_928_559; let got = hex_u128("deadbeef").expect("failed to parse non-prefixed hex"); assert_eq!(got, want); } diff --git a/units/src/weight.rs b/units/src/weight.rs index 9c8b9d8a6..73bed18f1 100644 --- a/units/src/weight.rs +++ b/units/src/weight.rs @@ -228,7 +228,7 @@ impl<'a> core::iter::Sum<&'a Weight> for Weight { where I: Iterator, { - iter.cloned().sum() + iter.copied().sum() } } @@ -281,7 +281,7 @@ mod tests { #[test] #[cfg(debug_assertions)] - #[should_panic] + #[should_panic = "attempt to multiply with overflow"] fn from_vb_unchecked_panic() { Weight::from_vb_unchecked(u64::MAX); } #[test] From 9619f689566d21956ec3d14b92fa79d66434e9fe Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 17 Dec 2024 18:04:55 +1100 Subject: [PATCH 2/3] units: Move excluded lints to manifest Might as well put these with the pedantic ones. --- units/Cargo.toml | 3 +++ units/src/amount/serde.rs | 4 ++-- units/src/lib.rs | 3 --- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/units/Cargo.toml b/units/Cargo.toml index e3bd679a8..021ed1f58 100644 --- a/units/Cargo.toml +++ b/units/Cargo.toml @@ -36,6 +36,9 @@ rustdoc-args = ["--cfg", "docsrs"] unexpected_cfgs = { level = "deny", check-cfg = ['cfg(kani)'] } [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. # Exhaustive list of pedantic clippy lints assigning_clones = "warn" bool_to_int_with_if = "warn" diff --git a/units/src/amount/serde.rs b/units/src/amount/serde.rs index 46fa2f92f..7eb35f6e4 100644 --- a/units/src/amount/serde.rs +++ b/units/src/amount/serde.rs @@ -405,7 +405,7 @@ mod tests { assert_eq!(json, want); let rinsed: HasAmount = serde_json::from_str(&json).expect("failed to deser"); - assert_eq!(rinsed, orig) + assert_eq!(rinsed, orig); } #[test] @@ -424,7 +424,7 @@ mod tests { assert_eq!(json, want); let rinsed: HasAmount = serde_json::from_str(&json).expect("failed to deser"); - assert_eq!(rinsed, orig) + assert_eq!(rinsed, orig); } #[test] diff --git a/units/src/lib.rs b/units/src/lib.rs index d53bc76d0..e4bd1254b 100644 --- a/units/src/lib.rs +++ b/units/src/lib.rs @@ -11,9 +11,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. #[cfg(feature = "alloc")] extern crate alloc; From 2513e05501e3a014c097f24eb9178c291785db81 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 17 Dec 2024 18:05:19 +1100 Subject: [PATCH 3/3] api: Run just check-api --- api/units/all-features.txt | 6 +++--- api/units/alloc-only.txt | 6 +++--- api/units/no-features.txt | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/units/all-features.txt b/api/units/all-features.txt index 4ec25b597..d0c77286c 100644 --- a/api/units/all-features.txt +++ b/api/units/all-features.txt @@ -917,9 +917,9 @@ pub fn bitcoin_units::amount::error::MissingDigitsError::fmt(&self, f: &mut core pub fn bitcoin_units::amount::error::OutOfRangeError::clone(&self) -> bitcoin_units::amount::error::OutOfRangeError pub fn bitcoin_units::amount::error::OutOfRangeError::eq(&self, other: &bitcoin_units::amount::error::OutOfRangeError) -> bool pub fn bitcoin_units::amount::error::OutOfRangeError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result -pub fn bitcoin_units::amount::error::OutOfRangeError::is_above_max(&self) -> bool -pub fn bitcoin_units::amount::error::OutOfRangeError::is_below_min(&self) -> bool -pub fn bitcoin_units::amount::error::OutOfRangeError::valid_range(&self) -> (i64, u64) +pub fn bitcoin_units::amount::error::OutOfRangeError::is_above_max(self) -> bool +pub fn bitcoin_units::amount::error::OutOfRangeError::is_below_min(self) -> bool +pub fn bitcoin_units::amount::error::OutOfRangeError::valid_range(self) -> (i64, u64) pub fn bitcoin_units::amount::error::ParseAmountError::clone(&self) -> bitcoin_units::amount::error::ParseAmountError pub fn bitcoin_units::amount::error::ParseAmountError::eq(&self, other: &bitcoin_units::amount::error::ParseAmountError) -> bool pub fn bitcoin_units::amount::error::ParseAmountError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result diff --git a/api/units/alloc-only.txt b/api/units/alloc-only.txt index 691aeca99..ae1768dec 100644 --- a/api/units/alloc-only.txt +++ b/api/units/alloc-only.txt @@ -853,9 +853,9 @@ pub fn bitcoin_units::amount::error::MissingDigitsError::fmt(&self, f: &mut core pub fn bitcoin_units::amount::error::OutOfRangeError::clone(&self) -> bitcoin_units::amount::error::OutOfRangeError pub fn bitcoin_units::amount::error::OutOfRangeError::eq(&self, other: &bitcoin_units::amount::error::OutOfRangeError) -> bool pub fn bitcoin_units::amount::error::OutOfRangeError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result -pub fn bitcoin_units::amount::error::OutOfRangeError::is_above_max(&self) -> bool -pub fn bitcoin_units::amount::error::OutOfRangeError::is_below_min(&self) -> bool -pub fn bitcoin_units::amount::error::OutOfRangeError::valid_range(&self) -> (i64, u64) +pub fn bitcoin_units::amount::error::OutOfRangeError::is_above_max(self) -> bool +pub fn bitcoin_units::amount::error::OutOfRangeError::is_below_min(self) -> bool +pub fn bitcoin_units::amount::error::OutOfRangeError::valid_range(self) -> (i64, u64) pub fn bitcoin_units::amount::error::ParseAmountError::clone(&self) -> bitcoin_units::amount::error::ParseAmountError pub fn bitcoin_units::amount::error::ParseAmountError::eq(&self, other: &bitcoin_units::amount::error::ParseAmountError) -> bool pub fn bitcoin_units::amount::error::ParseAmountError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result diff --git a/api/units/no-features.txt b/api/units/no-features.txt index 86c9be563..197e438eb 100644 --- a/api/units/no-features.txt +++ b/api/units/no-features.txt @@ -823,9 +823,9 @@ pub fn bitcoin_units::amount::error::MissingDigitsError::fmt(&self, f: &mut core pub fn bitcoin_units::amount::error::OutOfRangeError::clone(&self) -> bitcoin_units::amount::error::OutOfRangeError pub fn bitcoin_units::amount::error::OutOfRangeError::eq(&self, other: &bitcoin_units::amount::error::OutOfRangeError) -> bool pub fn bitcoin_units::amount::error::OutOfRangeError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result -pub fn bitcoin_units::amount::error::OutOfRangeError::is_above_max(&self) -> bool -pub fn bitcoin_units::amount::error::OutOfRangeError::is_below_min(&self) -> bool -pub fn bitcoin_units::amount::error::OutOfRangeError::valid_range(&self) -> (i64, u64) +pub fn bitcoin_units::amount::error::OutOfRangeError::is_above_max(self) -> bool +pub fn bitcoin_units::amount::error::OutOfRangeError::is_below_min(self) -> bool +pub fn bitcoin_units::amount::error::OutOfRangeError::valid_range(self) -> (i64, u64) pub fn bitcoin_units::amount::error::ParseAmountError::clone(&self) -> bitcoin_units::amount::error::ParseAmountError pub fn bitcoin_units::amount::error::ParseAmountError::eq(&self, other: &bitcoin_units::amount::error::ParseAmountError) -> bool pub fn bitcoin_units::amount::error::ParseAmountError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result