From 79a229b39115399729bad41db138a71d09b473c3 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 9 Dec 2024 08:57:46 +1100 Subject: [PATCH 1/2] units: Add pedantic lint return_self_not_must_use As part of the effort to polish the `units` create ready for the 1.0 release; enable the pedantic lint `return_self_not_must_use`. --- units/src/amount/mod.rs | 1 + units/src/amount/signed.rs | 3 +++ units/src/amount/unsigned.rs | 2 ++ units/src/lib.rs | 2 ++ 4 files changed, 8 insertions(+) diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs index 21eb9ed8c..db027a191 100644 --- a/units/src/amount/mod.rs +++ b/units/src/amount/mod.rs @@ -527,6 +527,7 @@ pub struct Display { impl Display { /// Makes subsequent calls to `Display::fmt` display denomination. + #[must_use] pub fn show_denomination(mut self) -> Self { match &mut self.style { DisplayStyle::FixedDenomination { show_denomination, .. } => *show_denomination = true, diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 4237ba0bb..1061a575a 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -206,6 +206,7 @@ impl SignedAmount { // Some arithmetic that doesn't fit in [`core::ops`] traits. /// Get the absolute value of this [`SignedAmount`]. + #[must_use] pub fn abs(self) -> SignedAmount { SignedAmount(self.0.abs()) } /// Gets the absolute value of this [`SignedAmount`] returning [`Amount`]. @@ -307,6 +308,7 @@ impl SignedAmount { /// # Panics /// /// On overflow, panics in debug mode, wraps in release mode. + #[must_use] pub fn unchecked_add(self, rhs: SignedAmount) -> SignedAmount { Self(self.0 + rhs.0) } /// Unchecked subtraction. @@ -316,6 +318,7 @@ impl SignedAmount { /// # Panics /// /// On overflow, panics in debug mode, wraps in release mode. + #[must_use] pub fn unchecked_sub(self, rhs: SignedAmount) -> SignedAmount { Self(self.0 - rhs.0) } /// Subtraction that doesn't allow negative [`SignedAmount`]s. diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 9b96c4d51..af43e631b 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -318,6 +318,7 @@ impl Amount { /// # Panics /// /// On overflow, panics in debug mode, wraps in release mode. + #[must_use] pub fn unchecked_add(self, rhs: Amount) -> Amount { Self(self.0 + rhs.0) } /// Unchecked subtraction. @@ -327,6 +328,7 @@ impl Amount { /// # Panics /// /// On overflow, panics in debug mode, wraps in release mode. + #[must_use] pub fn unchecked_sub(self, rhs: Amount) -> Amount { Self(self.0 - rhs.0) } /// Converts to a signed amount. diff --git a/units/src/lib.rs b/units/src/lib.rs index b9e7e960a..59bc16205 100644 --- a/units/src/lib.rs +++ b/units/src/lib.rs @@ -11,6 +11,8 @@ #![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. From 2fd8614f5d091377f179440b04ec71f4853fd187 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 9 Dec 2024 09:19:12 +1100 Subject: [PATCH 2/2] units: Add additional must_use Use the `must_use_candidate` clippy lint to find all functions that are candidates for having `must_use`. Add `must_use` attribute but exclude obvious functions like `from_`, `to_`, and `new`. This patch is subjective. --- units/src/amount/signed.rs | 11 +++++++++++ units/src/amount/unsigned.rs | 9 +++++++++ units/src/fee_rate.rs | 7 +++++++ units/src/locktime/relative.rs | 2 ++ units/src/weight.rs | 4 ++++ 5 files changed, 33 insertions(+) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 1061a575a..375ca6653 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -170,6 +170,7 @@ impl SignedAmount { } /// Constructs a new object that implements [`fmt::Display`] using specified denomination. + #[must_use] pub fn display_in(self, denomination: Denomination) -> Display { Display { sats_abs: self.unsigned_abs().to_sat(), @@ -182,6 +183,7 @@ impl SignedAmount { /// /// This will use BTC for values greater than or equal to 1 BTC and satoshis otherwise. To /// avoid confusion the denomination is always shown. + #[must_use] pub fn display_dynamic(self) -> Display { Display { sats_abs: self.unsigned_abs().to_sat(), @@ -210,6 +212,7 @@ impl SignedAmount { pub fn abs(self) -> SignedAmount { SignedAmount(self.0.abs()) } /// Gets the absolute value of this [`SignedAmount`] returning [`Amount`]. + #[must_use] pub fn unsigned_abs(self) -> Amount { Amount::from_sat(self.0.unsigned_abs()) } /// Returns a number representing sign of this [`SignedAmount`]. @@ -217,6 +220,7 @@ impl SignedAmount { /// - `0` if the amount is zero /// - `1` if the amount is positive /// - `-1` if the amount is negative + #[must_use] pub fn signum(self) -> i64 { self.0.signum() } /// Checks if this [`SignedAmount`] is positive. @@ -236,6 +240,7 @@ impl SignedAmount { /// Consider using `unsigned_abs` which is often more practical. /// /// Returns [`None`] if overflow occurred. (`self == MIN`) + #[must_use] pub const fn checked_abs(self) -> Option { // No `map()` in const context. match self.0.checked_abs() { @@ -247,6 +252,7 @@ impl SignedAmount { /// Checked addition. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_add(self, rhs: SignedAmount) -> Option { // No `map()` in const context. match self.0.checked_add(rhs.0) { @@ -258,6 +264,7 @@ impl SignedAmount { /// Checked subtraction. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_sub(self, rhs: SignedAmount) -> Option { // No `map()` in const context. match self.0.checked_sub(rhs.0) { @@ -269,6 +276,7 @@ impl SignedAmount { /// Checked multiplication. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_mul(self, rhs: i64) -> Option { // No `map()` in const context. match self.0.checked_mul(rhs) { @@ -282,6 +290,7 @@ impl SignedAmount { /// Be aware that integer division loses the remainder if no exact division can be made. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_div(self, rhs: i64) -> Option { // No `map()` in const context. match self.0.checked_div(rhs) { @@ -293,6 +302,7 @@ impl SignedAmount { /// Checked remainder. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_rem(self, rhs: i64) -> Option { // No `map()` in const context. match self.0.checked_rem(rhs) { @@ -324,6 +334,7 @@ impl SignedAmount { /// Subtraction that doesn't allow negative [`SignedAmount`]s. /// /// Returns [`None`] if either `self`, `rhs` or the result is strictly negative. + #[must_use] pub fn positive_sub(self, rhs: SignedAmount) -> Option { if self.is_negative() || rhs.is_negative() || rhs > self { None diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index af43e631b..f51ae4af3 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -178,6 +178,7 @@ impl Amount { } /// Constructs a new object that implements [`fmt::Display`] using specified denomination. + #[must_use] pub fn display_in(self, denomination: Denomination) -> Display { Display { sats_abs: self.to_sat(), @@ -190,6 +191,7 @@ impl Amount { /// /// This will use BTC for values greater than or equal to 1 BTC and satoshis otherwise. To /// avoid confusion the denomination is always shown. + #[must_use] pub fn display_dynamic(self) -> Display { Display { sats_abs: self.to_sat(), @@ -216,6 +218,7 @@ impl Amount { /// Checked addition. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_add(self, rhs: Amount) -> Option { // No `map()` in const context. match self.0.checked_add(rhs.0) { @@ -227,6 +230,7 @@ impl Amount { /// Checked subtraction. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_sub(self, rhs: Amount) -> Option { // No `map()` in const context. match self.0.checked_sub(rhs.0) { @@ -238,6 +242,7 @@ impl Amount { /// Checked multiplication. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_mul(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_mul(rhs) { @@ -251,6 +256,7 @@ impl Amount { /// Be aware that integer division loses the remainder if no exact division can be made. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_div(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_div(rhs) { @@ -267,6 +273,7 @@ impl Amount { /// /// Returns [`None`] if overflow occurred. #[cfg(feature = "alloc")] + #[must_use] pub const fn checked_div_by_weight_ceil(self, rhs: Weight) -> Option { let wu = rhs.to_wu(); // No `?` operator in const context. @@ -289,6 +296,7 @@ impl Amount { /// /// Returns [`None`] if overflow occurred. #[cfg(feature = "alloc")] + #[must_use] pub const fn checked_div_by_weight_floor(self, rhs: Weight) -> Option { // No `?` operator in const context. match self.0.checked_mul(1_000) { @@ -303,6 +311,7 @@ impl Amount { /// Checked remainder. /// /// Returns [`None`] if overflow occurred. + #[must_use] pub const fn checked_rem(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_rem(rhs) { diff --git a/units/src/fee_rate.rs b/units/src/fee_rate.rs index c6f3f76fb..d6b4c33cb 100644 --- a/units/src/fee_rate.rs +++ b/units/src/fee_rate.rs @@ -79,6 +79,7 @@ impl FeeRate { /// Checked multiplication. /// /// Computes `self * rhs` returning [`None`] if overflow occurred. + #[must_use] pub const fn checked_mul(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_mul(rhs) { @@ -90,6 +91,7 @@ impl FeeRate { /// Checked division. /// /// Computes `self / rhs` returning [`None`] if `rhs == 0`. + #[must_use] pub const fn checked_div(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_div(rhs) { @@ -106,6 +108,7 @@ impl FeeRate { /// rounded down. /// /// [`None`] is returned if an overflow occurred. + #[must_use] pub const fn checked_mul_by_weight(self, rhs: Weight) -> Option { // No `?` operator in const context. match self.0.checked_mul(rhs.to_wu()) { @@ -120,6 +123,7 @@ impl FeeRate { /// Checked addition. /// /// Computes `self + rhs` returning [`None`] if overflow occured. + #[must_use] pub const fn checked_add(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_add(rhs) { @@ -131,6 +135,7 @@ impl FeeRate { /// Checked subtraction. /// /// Computes `self - rhs` returning [`None`] if overflow occured. + #[must_use] pub const fn checked_sub(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_sub(rhs) { @@ -143,6 +148,7 @@ impl FeeRate { /// if an overflow occurred. /// /// This is equivalent to `Self::checked_mul_by_weight()`. + #[must_use] pub fn fee_wu(self, weight: Weight) -> Option { self.checked_mul_by_weight(weight) } /// Calculates the fee by multiplying this fee rate by weight, in virtual bytes, returning [`None`] @@ -150,6 +156,7 @@ impl FeeRate { /// /// This is equivalent to converting `vb` to [`Weight`] using [`Weight::from_vb`] and then calling /// `Self::fee_wu(weight)`. + #[must_use] pub fn fee_vb(self, vb: u64) -> Option { Weight::from_vb(vb).and_then(|w| self.fee_wu(w)) } diff --git a/units/src/locktime/relative.rs b/units/src/locktime/relative.rs index 9b98e38c4..c0313dc4f 100644 --- a/units/src/locktime/relative.rs +++ b/units/src/locktime/relative.rs @@ -28,6 +28,7 @@ impl Height { /// Returns the inner `u16` value. #[inline] + #[must_use] pub const fn value(self) -> u16 { self.0 } /// Returns the `u32` value used to encode this locktime in an nSequence field or @@ -108,6 +109,7 @@ impl Time { /// Returns the inner `u16` value. #[inline] + #[must_use] pub const fn value(self) -> u16 { self.0 } /// Returns the `u32` value used to encode this locktime in an nSequence field or diff --git a/units/src/weight.rs b/units/src/weight.rs index 6f72bc670..4e0b5026d 100644 --- a/units/src/weight.rs +++ b/units/src/weight.rs @@ -106,6 +106,7 @@ impl Weight { /// Checked addition. /// /// Computes `self + rhs` returning [`None`] if an overflow occurred. + #[must_use] pub const fn checked_add(self, rhs: Self) -> Option { // No `map()` in const context. match self.0.checked_add(rhs.0) { @@ -117,6 +118,7 @@ impl Weight { /// Checked subtraction. /// /// Computes `self - rhs` returning [`None`] if an overflow occurred. + #[must_use] pub const fn checked_sub(self, rhs: Self) -> Option { // No `map()` in const context. match self.0.checked_sub(rhs.0) { @@ -128,6 +130,7 @@ impl Weight { /// Checked multiplication. /// /// Computes `self * rhs` returning [`None`] if an overflow occurred. + #[must_use] pub const fn checked_mul(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_mul(rhs) { @@ -139,6 +142,7 @@ impl Weight { /// Checked division. /// /// Computes `self / rhs` returning [`None`] if `rhs == 0`. + #[must_use] pub const fn checked_div(self, rhs: u64) -> Option { // No `map()` in const context. match self.0.checked_div(rhs) {