Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: clippy fixes #728

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
- if: matrix.sys.test
run: cargo hack --feature-powerset test --locked --target ${{ matrix.sys.target }}

Expand Down
121 changes: 56 additions & 65 deletions soroban-env-common/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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),
}
}
}
9 changes: 5 additions & 4 deletions soroban-env-common/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_
Expand Down Expand Up @@ -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>;

Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
{
$(
Expand Down
17 changes: 7 additions & 10 deletions soroban-env-common/src/env_val.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
dmkozh marked this conversation as resolved.
Show resolved Hide resolved
Ok(u as i128)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 6 additions & 5 deletions soroban-env-common/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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))]
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-common/src/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 10 additions & 10 deletions soroban-env-common/src/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand All @@ -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())
}
}

Expand All @@ -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())
}
}

Expand Down Expand Up @@ -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()
}
}

Expand All @@ -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 {
Expand Down
Loading