chore: clippy fixes

.github/workflows/rust.yml

@@ -64,7 +64,7 @@ jobs:
       with:
         name: cargo-hack
         version: 0.5.16
-    - run: cargo hack --feature-powerset check --locked --target ${{ matrix.sys.target }}
+    - run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }} -- -Dwarnings -Dclippy::pedantic -Aclippy::module_name_repetitions -Aclippy::needless_pass_by_value -Aclippy::too_many_lines -Aclippy::must_use_candidate -Aclippy::missing_errors_doc -Aclippy::missing_safety_doc -Aclippy::inline_always -Aclippy::cast_possible_truncation -Aclippy::cast_sign_loss -Aclippy::cast_possible_wrap -Aclippy::similar_names -Aclippy::doc_markdown -Aclippy::default_trait_access -Aclippy::missing_safety_doc -Aclippy::struct_excessive_bools -Aclippy::missing_panics_doc -Aclippy::cast_lossless -Aclippy::trivially_copy_pass_by_ref -Aclippy::wrong_self_convention -Aclippy::unused_self -Aclippy::enum_glob_use -Aclippy::return_self_not_must_use -Aclippy::map_entry -Aclippy::match_same_arms -Aclippy::iter_not_returning_iterator -Aclippy::unnecessary_wraps
     - if: matrix.sys.test
       run: cargo hack --feature-powerset test --locked --target ${{ matrix.sys.target }}
 

soroban-env-common/src/compare.rs

@@ -97,7 +97,7 @@ impl<T, C: Compare<T>> Compare<Box<T>> for C {
     type Error = C::Error;
 
     fn compare(&self, a: &Box<T>, b: &Box<T>) -> Result<Ordering, Self::Error> {
-        <Self as Compare<T>>::compare(self, &*a, &*b)
+        <Self as Compare<T>>::compare(self, a, b)
     }
 }
 
@@ -106,7 +106,7 @@ impl<T, C: Compare<T>> Compare<Rc<T>> for C {
     type Error = C::Error;
 
     fn compare(&self, a: &Rc<T>, b: &Rc<T>) -> Result<Ordering, Self::Error> {
-        <Self as Compare<T>>::compare(self, &*a, &*b)
+        <Self as Compare<T>>::compare(self, a, b)
     }
 }
 
@@ -135,74 +135,65 @@ impl<E: Env> Compare<RawVal> for E {
         if a.is_object() || b.is_object() {
             // Delegate any object-comparing to environment.
             let v = self.obj_cmp(*a, *b)?;
-            return if v == 0 {
-                Ok(Ordering::Equal)
-            } else if v < 0 {
-                Ok(Ordering::Less)
-            } else {
-                Ok(Ordering::Greater)
-            };
+            return Ok(v.cmp(&0));
         }
         let a_tag = a.get_tag();
         let b_tag = b.get_tag();
-        if a_tag < b_tag {
-            Ok(Ordering::Less)
-        } else if a_tag > b_tag {
-            Ok(Ordering::Greater)
-        } else {
+        match a_tag.cmp(&b_tag) {
+            Ordering::Equal =>
             // Tags are equal so we only have to switch on one.
-            match a_tag {
-                Tag::False => Ok(Ordering::Equal),
-                Tag::True => Ok(Ordering::Equal),
-                Tag::Void => Ok(Ordering::Equal),
-
-                Tag::Status => delegate_compare_to_wrapper!(Status, a, b, self),
-
-                Tag::U32Val => delegate_compare_to_wrapper!(U32Val, a, b, self),
-                Tag::I32Val => delegate_compare_to_wrapper!(I32Val, a, b, self),
-
-                Tag::U64Small => delegate_compare_to_wrapper!(U64Small, a, b, self),
-                Tag::I64Small => delegate_compare_to_wrapper!(I64Small, a, b, self),
-
-                Tag::TimepointSmall => delegate_compare_to_wrapper!(TimepointSmall, a, b, self),
-                Tag::DurationSmall => delegate_compare_to_wrapper!(DurationSmall, a, b, self),
-
-                Tag::U128Small => delegate_compare_to_wrapper!(U128Small, a, b, self),
-                Tag::I128Small => delegate_compare_to_wrapper!(I128Small, a, b, self),
-
-                Tag::U256Small => delegate_compare_to_wrapper!(U256Small, a, b, self),
-                Tag::I256Small => delegate_compare_to_wrapper!(I256Small, a, b, self),
-
-                Tag::SymbolSmall => delegate_compare_to_wrapper!(SymbolSmall, a, b, self),
-
-                Tag::LedgerKeyContractExecutable => Ok(Ordering::Equal),
-
-                Tag::SmallCodeUpperBound => Ok(Ordering::Equal),
-                Tag::ObjectCodeLowerBound => Ok(Ordering::Equal),
-
-                // None of the object cases should be reachable, they
-                // should all have been handled by the is_object() branch
-                // above.
-                Tag::U64Object
-                | Tag::I64Object
-                | Tag::TimepointObject
-                | Tag::DurationObject
-                | Tag::U128Object
-                | Tag::I128Object
-                | Tag::U256Object
-                | Tag::I256Object
-                | Tag::BytesObject
-                | Tag::StringObject
-                | Tag::SymbolObject
-                | Tag::VecObject
-                | Tag::MapObject
-                | Tag::ContractExecutableObject
-                | Tag::AddressObject
-                | Tag::LedgerKeyNonceObject => unreachable!(),
-
-                Tag::ObjectCodeUpperBound => Ok(Ordering::Equal),
-                Tag::Bad => Ok(Ordering::Equal),
+            {
+                match a_tag {
+                    Tag::False
+                    | Tag::True
+                    | Tag::Void
+                    | Tag::LedgerKeyContractExecutable
+                    | Tag::SmallCodeUpperBound
+                    | Tag::ObjectCodeLowerBound
+                    | Tag::ObjectCodeUpperBound
+                    | Tag::Bad => Ok(Ordering::Equal),
+
+                    Tag::Status => delegate_compare_to_wrapper!(Status, a, b, self),
+
+                    Tag::U32Val => delegate_compare_to_wrapper!(U32Val, a, b, self),
+                    Tag::I32Val => delegate_compare_to_wrapper!(I32Val, a, b, self),
+
+                    Tag::U64Small => delegate_compare_to_wrapper!(U64Small, a, b, self),
+                    Tag::I64Small => delegate_compare_to_wrapper!(I64Small, a, b, self),
+
+                    Tag::TimepointSmall => delegate_compare_to_wrapper!(TimepointSmall, a, b, self),
+                    Tag::DurationSmall => delegate_compare_to_wrapper!(DurationSmall, a, b, self),
+
+                    Tag::U128Small => delegate_compare_to_wrapper!(U128Small, a, b, self),
+                    Tag::I128Small => delegate_compare_to_wrapper!(I128Small, a, b, self),
+
+                    Tag::U256Small => delegate_compare_to_wrapper!(U256Small, a, b, self),
+                    Tag::I256Small => delegate_compare_to_wrapper!(I256Small, a, b, self),
+
+                    Tag::SymbolSmall => delegate_compare_to_wrapper!(SymbolSmall, a, b, self),
+
+                    // None of the object cases should be reachable, they
+                    // should all have been handled by the is_object() branch
+                    // above.
+                    Tag::U64Object
+                    | Tag::I64Object
+                    | Tag::TimepointObject
+                    | Tag::DurationObject
+                    | Tag::U128Object
+                    | Tag::I128Object
+                    | Tag::U256Object
+                    | Tag::I256Object
+                    | Tag::BytesObject
+                    | Tag::StringObject
+                    | Tag::SymbolObject
+                    | Tag::VecObject
+                    | Tag::MapObject
+                    | Tag::ContractExecutableObject
+                    | Tag::AddressObject
+                    | Tag::LedgerKeyNonceObject => unreachable!(),
+                }
             }
+            greater_or_less => Ok(greater_or_less),
         }
     }
 }

soroban-env-common/src/env.rs

@@ -47,6 +47,7 @@ pub trait EnvBase: Sized + Clone {
     fn check_same_env(&self, other: &Self);
 
     /// Used to clone an environment deeply, not just a handle to it.
+    #[allow(clippy::return_self_not_must_use)]
     fn deep_clone(&self) -> Self;
 
     // Helpers for methods that wish to pass Rust lifetime-qualified _slices_
@@ -160,7 +161,7 @@ pub trait EnvBase: Sized + Clone {
 
     /// Log a formatted debugging message to the debug log (if present), passing
     /// a simplified format string (supporting only positional `{}` markers) and
-    /// a single [RawVal] argument that will be inserted at the marker in the
+    /// a single [`RawVal`] argument that will be inserted at the marker in the
     /// format string.
     fn log_static_fmt_val(&self, fmt: &'static str, v: RawVal) -> Result<(), Self::Error>;
 
@@ -176,7 +177,7 @@ pub trait EnvBase: Sized + Clone {
 
     /// Log a formatted debugging message to the debug log (if present), passing
     /// a simplified format string (supporting only positional `{}` markers) and
-    /// both a [RawVal] and a string-slice argument, that will each be inserted
+    /// both a [`RawVal`] and a string-slice argument, that will each be inserted
     /// at markers in the format string.
     fn log_static_fmt_val_static_str(
         &self,
@@ -187,7 +188,7 @@ pub trait EnvBase: Sized + Clone {
 
     /// Log a formatted debugging message to the debug log (if present), passing
     /// a simplified format string (supporting only positional `{}` markers) and
-    /// both a slice of [RawVal]s and a slice of string-slice argument, that
+    /// both a slice of [`RawVal`]s and a slice of string-slice argument, that
     /// will be sequentially inserted at markers in the format string.
     fn log_static_fmt_general(
         &self,
@@ -285,7 +286,7 @@ macro_rules! generate_env_trait {
         /// This trait represents the interface between Host and Guest, used by
         /// client contract code and implemented (via [Env](crate::Env)) by the host.
         /// It consists of functions that take or return only 64-bit values such
-        /// as [RawVal] or [u64].
+        /// as [`RawVal`] or [`u64`].
         pub trait Env: EnvBase
         {
             $(

soroban-env-common/src/env_val.rs

@@ -143,7 +143,7 @@ impl<E: Env> TryFromVal<E, RawVal> for i128 {
             let obj = v.try_into()?;
             let lo = env.obj_to_i128_lo64(obj).map_err(|_| ConversionError)?;
             let hi = env.obj_to_i128_hi64(obj).map_err(|_| ConversionError)?;
-            let u: u128 = (lo as u128) | ((hi as u128) << 64);
+            let u: u128 = u128::from(lo) | (u128::from(hi) << 64);
             Ok(u as i128)
         }
     }
@@ -177,7 +177,7 @@ impl<E: Env> TryFromVal<E, RawVal> for u128 {
             let obj = v.try_into()?;
             let lo = env.obj_to_u128_lo64(obj).map_err(|_| ConversionError)?;
             let hi = env.obj_to_u128_hi64(obj).map_err(|_| ConversionError)?;
-            let u: u128 = (lo as u128) | ((hi as u128) << 64);
+            let u: u128 = u128::from(lo) | (u128::from(hi) << 64);
             Ok(u)
         }
     }
@@ -241,11 +241,11 @@ where
                 lo: val.get_signed_body() as u64,
             })),
             Tag::U256Small => {
-                let val = U256::new(val.get_body() as u128);
+                let val = U256::new(u128::from(val.get_body()));
                 Ok(ScVal::U256(Uint256(val.to_be_bytes())))
             }
             Tag::I256Small => {
-                let val = I256::new(val.get_signed_body() as i128);
+                let val = I256::new(i128::from(val.get_signed_body()));
                 Ok(ScVal::I256(Uint256(val.to_be_bytes())))
             }
             Tag::SymbolSmall => {
@@ -327,20 +327,17 @@ where
                 unsafe { RawVal::from_body_and_tag((i as i64) as u64, Tag::I128Small) }
             }
             ScVal::U256(u) => {
-                let u: U256 = U256::from_be_bytes(u.0.clone());
+                let u: U256 = U256::from_be_bytes(u.0);
                 assert!(num::is_small_u256(&u));
                 unsafe { RawVal::from_body_and_tag(u.as_u64(), Tag::U256Small) }
             }
             ScVal::I256(i) => {
-                let i: I256 = I256::from_be_bytes(i.0.clone());
+                let i: I256 = I256::from_be_bytes(i.0);
                 assert!(num::is_small_i256(&i));
                 unsafe { RawVal::from_body_and_tag(i.as_i64() as u64, Tag::I256Small) }
             }
             ScVal::Symbol(bytes) => {
-                let ss = match std::str::from_utf8(bytes.as_slice()) {
-                    Ok(ss) => ss,
-                    Err(_) => return Err(ConversionError),
-                };
+                let Ok(ss) = std::str::from_utf8(bytes.as_slice()) else { return Err(ConversionError) };
                 SymbolSmall::try_from_str(ss)?.into()
             }
             ScVal::LedgerKeyContractExecutable => unsafe {

soroban-env-common/src/lib.rs

@@ -1,22 +1,22 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 //! The environment-common crate contains three families of types:
 //!
-//!   - The [RawVal] type, a 64-bit value type that is a union between several
+//!   - The [`RawVal`] type, a 64-bit value type that is a union between several
 //!     different types (numbers, booleans, symbols, object references), encoded
 //!     via careful bit-packing.
 //!   - Wrapper types ([Object], [Symbol], [Status]) that
-//!     contain [RawVal] in a specific, known union state. These are also 64-bit
+//!     contain [`RawVal`] in a specific, known union state. These are also 64-bit
 //!     values, but offer methods specific to the union state (eg. [Symbol] will
 //!     interconvert with Rust strings).
 //!   - The [Env] trait, which describes the _interface_ between guest and host
 //!     code. In other words, `Env` describes a set of _host functions_ that
 //!     must be implemented in a contract host, and can be called from a guest
 //!     (or by the SDK). Methods on the [Env] trait can only pass 64-bit values,
-//!     which are usually [RawVal] or one of the wrapper types.
+//!     which are usually [`RawVal`] or one of the wrapper types.
 //!
 //! The crate additionally contains functions for interconversion between the
-//! [RawVal] type and XDR types, and re-exports the XDR definitions from
-//! [stellar_xdr] under the module [xdr].
+//! [`RawVal`] type and XDR types, and re-exports the XDR definitions from
+//! [`stellar_xdr`] under the module [xdr].
 
 #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -91,6 +91,7 @@ pub use status::Status;
 pub use string::StringObject;
 pub use symbol::{Symbol, SymbolError, SymbolObject, SymbolSmall, SymbolSmallIter, SymbolStr};
 
+#[allow(clippy::missing_panics_doc)]
 #[inline(always)]
 // Awkward: this is a free function rather than a trait call because
 // you can't have const trait calls. It calls panic! rather than

soroban-env-common/src/meta.rs

@@ -55,7 +55,7 @@
 // a contract, since the same contract will be expected to run against multliple
 // implementations over a long period of time.
 
-pub const ENV_META_V0_SECTION_NAME: &'static str = "contractenvmetav0";
+pub const ENV_META_V0_SECTION_NAME: &str = "contractenvmetav0";
 
 soroban_env_macros::generate_env_meta_consts!(
     interface_version: 32,

soroban-env-common/src/num.rs

@@ -94,7 +94,7 @@ impl From<U64Small> for u64 {
 
 impl From<I64Small> for i64 {
     fn from(value: I64Small) -> Self {
-        value.0.get_signed_body() as i64
+        value.0.get_signed_body()
     }
 }
 
@@ -112,13 +112,13 @@ impl From<DurationSmall> for u64 {
 
 impl From<U128Small> for u128 {
     fn from(value: U128Small) -> Self {
-        value.0.get_body() as u128
+        u128::from(value.0.get_body())
     }
 }
 
 impl From<I128Small> for i128 {
     fn from(value: I128Small) -> Self {
-        (value.0.get_signed_body() as i64) as i128
+        i128::from(value.0.get_signed_body())
     }
 }
 
@@ -130,7 +130,7 @@ impl From<U256Small> for U256 {
 
 impl From<I256Small> for I256 {
     fn from(value: I256Small) -> Self {
-        I256::from(value.0.get_signed_body() as i64)
+        I256::from(value.0.get_signed_body())
     }
 }
 
@@ -215,30 +215,30 @@ impl TryFrom<I256> for I256Small {
 impl TryFrom<U256Small> for ScVal {
     type Error = ConversionError;
     fn try_from(value: U256Small) -> Result<Self, Self::Error> {
-        let val = U256::new(value.as_raw().get_body() as u128);
+        let val = U256::new(u128::from(value.as_raw().get_body()));
         Ok(ScVal::U256(Uint256(val.to_be_bytes())))
     }
 }
 
 impl TryFrom<&U256Small> for ScVal {
     type Error = ConversionError;
     fn try_from(value: &U256Small) -> Result<Self, Self::Error> {
-        value.clone().try_into()
+        (*value).try_into()
     }
 }
 
 impl TryFrom<I256Small> for ScVal {
     type Error = ConversionError;
     fn try_from(value: I256Small) -> Result<Self, Self::Error> {
-        let val = I256::new(value.as_raw().get_signed_body() as i128);
+        let val = I256::new(i128::from(value.as_raw().get_signed_body()));
         Ok(ScVal::I256(Uint256(val.to_be_bytes())))
     }
 }
 
 impl TryFrom<&I256Small> for ScVal {
     type Error = ConversionError;
     fn try_from(value: &I256Small) -> Result<Self, Self::Error> {
-        value.clone().try_into()
+        (*value).try_into()
     }
 }
 
@@ -252,12 +252,12 @@ pub const fn is_small_i64(i: i64) -> bool {
 
 pub fn is_small_u128(u: u128) -> bool {
     let word = u as u64;
-    is_small_u64(word) && u == (word as u128)
+    is_small_u64(word) && u == u128::from(word)
 }
 
 pub fn is_small_i128(i: i128) -> bool {
     let word = i as i64;
-    is_small_i64(word) && i == (word as i128)
+    is_small_i64(word) && i == i128::from(word)
 }
 
 pub fn is_small_u256(u: &U256) -> bool {