Skip to content

Commit

Permalink
Enable code linting with clippy pedantic (#149)
Browse files Browse the repository at this point in the history
* fix: clippy pedantic manual fixes

* fix: further manual fixes

* fix: some automated fixes and #[must_use] additions

* fix: final clippy pedantic fixes
  • Loading branch information
encody authored May 8, 2024
1 parent 0d1ae85 commit a82ae8f
Show file tree
Hide file tree
Showing 35 changed files with 397 additions and 121 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ pretty_assertions = "1"
tokio = "1"

[workspace.lints.clippy]
# pedantic = "warn"
pedantic = "warn"
module-name-repetitions = "allow"

[workspace.lints.rust]
missing-docs = "warn"
Expand Down
4 changes: 1 addition & 3 deletions macros/src/escrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ pub fn expand(meta: EscrowMeta) -> Result<TokenStream, darling::Error> {
}
});

let state = state
.map(|state| quote! { #state })
.unwrap_or_else(|| quote! { () });
let state = state.map_or_else(|| quote! { () }, |state| quote! { #state });

Ok(quote! {
impl #imp #me::escrow::EscrowInternal for #ident #ty #wher {
Expand Down
9 changes: 4 additions & 5 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::too_many_lines, clippy::unnecessary_wraps)]
//! Macros for near-sdk-contract-tools.
use darling::{ast::NestedMeta, FromDeriveInput, FromMeta};
Expand Down Expand Up @@ -45,8 +46,7 @@ where

FromDeriveInput::from_derive_input(&input)
.and_then(expand)
.map(Into::into)
.unwrap_or_else(|e| e.write_errors().into())
.map_or_else(|e| e.write_errors().into(), Into::into)
}

/// Use on a struct to emit NEP-297 event strings.
Expand Down Expand Up @@ -257,9 +257,8 @@ pub fn event(attr: TokenStream, item: TokenStream) -> TokenStream {
let item = parse_macro_input!(item as Item);

standard::event::EventAttributeMeta::from_list(&attr)
.and_then(|meta| standard::event::event_attribute(meta, item))
.map(Into::into)
.unwrap_or_else(|e| e.write_errors().into())
.and_then(|meta| standard::event::event_attribute(meta, &item))
.map_or_else(|e| e.write_errors().into(), Into::into)
}

/// Create an upgrade component. Does not expose any functions to the
Expand Down
7 changes: 4 additions & 3 deletions macros/src/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ pub fn expand(meta: MigrateMeta) -> Result<TokenStream, darling::Error> {

let (imp, ty, wh) = generics.split_for_impl();

let to = to
.map(|t| t.to_token_stream())
.unwrap_or_else(|| quote! { Self }.to_token_stream());
let to = to.map_or_else(
|| quote! { Self }.to_token_stream(),
|t| t.to_token_stream(),
);

Ok(quote! {
impl #imp #me::migrate::MigrateController for #ident #ty #wh {
Expand Down
2 changes: 1 addition & 1 deletion macros/src/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl RenameStrategy {
impl FromMeta for RenameStrategy {
fn from_string(value: &str) -> darling::Result<Self> {
RenameStrategy::try_from(value)
.map_err(|_| darling::Error::custom("Invalid rename strategy"))
.map_err(|()| darling::Error::custom("Invalid rename strategy"))
}
}

Expand Down
2 changes: 1 addition & 1 deletion macros/src/standard/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub struct EventAttributeMeta {

pub fn event_attribute(
attr: EventAttributeMeta,
item: Item,
item: &Item,
) -> Result<TokenStream, darling::Error> {
let EventAttributeMeta {
standard,
Expand Down
18 changes: 5 additions & 13 deletions macros/src/standard/nep141.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,11 @@ pub fn expand(meta: Nep141Meta) -> Result<TokenStream, darling::Error> {
}
});

let mint_hook = mint_hook
.map(|h| quote! { #h })
.unwrap_or_else(|| quote! { () });
let transfer_hook = transfer_hook
.map(|h| quote! { #h })
.unwrap_or_else(|| quote! { () });
let burn_hook = burn_hook
.map(|h| quote! { #h })
.unwrap_or_else(|| quote! { () });

let default_hook = all_hooks
.map(|h| quote! { #h })
.unwrap_or_else(|| quote! { () });
let mint_hook = mint_hook.map_or_else(|| quote! { () }, |h| quote! { #h });
let transfer_hook = transfer_hook.map_or_else(|| quote! { () }, |h| quote! { #h });
let burn_hook = burn_hook.map_or_else(|| quote! { () }, |h| quote! { #h });

let default_hook = all_hooks.map_or_else(|| quote! { () }, |h| quote! { #h });

Ok(quote! {
impl #imp #me::standard::nep141::Nep141ControllerInternal for #ident #ty #wher {
Expand Down
9 changes: 3 additions & 6 deletions macros/src/standard/nep145.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,9 @@ pub fn expand(meta: Nep145Meta) -> Result<TokenStream, darling::Error> {
}
});

let all_hooks = all_hooks
.map(|h| quote! { #h })
.unwrap_or_else(|| quote! { () });
let force_unregister_hook = force_unregister_hook
.map(|h| quote! { #h })
.unwrap_or_else(|| quote! { () });
let all_hooks = all_hooks.map_or_else(|| quote! { () }, |h| quote! { #h });
let force_unregister_hook =
force_unregister_hook.map_or_else(|| quote! { () }, |h| quote! { #h });

Ok(quote! {
impl #imp #me::standard::nep145::Nep145ControllerInternal for #ident #ty #wher {
Expand Down
4 changes: 2 additions & 2 deletions macros/src/standard/nep297.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@ pub fn expand(meta: Nep297Meta) -> Result<TokenStream, darling::Error> {
let mut e = darling::Error::accumulator();

let mut no_duplicate_names = HashSet::<&String>::new();
for used_name in used_names.iter() {
for used_name in &used_names {
let fresh_insertion = no_duplicate_names.insert(used_name);
if !fresh_insertion {
e.push(darling::Error::custom(format!(
"Event name collision: `{used_name}`",
)))
)));
}
}

Expand Down
21 changes: 12 additions & 9 deletions macros/src/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,18 @@ pub fn expand(meta: UpgradeMeta) -> Result<TokenStream, darling::Error> {
// Defaults are defined in main crate.
// I don't think these defaults can be easily defined using
// #[darling(default = "...")] because they are different types.
let migrate_method_name = migrate_method_name
.map(|e| quote! { #e })
.unwrap_or_else(|| quote! { #me::upgrade::DEFAULT_POST_UPGRADE_METHOD_NAME });
let migrate_method_args = migrate_method_args
.map(|e| quote! { #e })
.unwrap_or_else(|| quote! { #me::upgrade::DEFAULT_POST_UPGRADE_METHOD_ARGS });
let migrate_minimum_gas = migrate_minimum_gas
.map(|e| quote! { #e })
.unwrap_or_else(|| quote! { #me::upgrade::DEFAULT_POST_UPGRADE_MINIMUM_GAS });
let migrate_method_name = migrate_method_name.map_or_else(
|| quote! { #me::upgrade::DEFAULT_POST_UPGRADE_METHOD_NAME },
|e| quote! { #e },
);
let migrate_method_args = migrate_method_args.map_or_else(
|| quote! { #me::upgrade::DEFAULT_POST_UPGRADE_METHOD_ARGS },
|e| quote! { #e },
);
let migrate_minimum_gas = migrate_minimum_gas.map_or_else(
|| quote! { #me::upgrade::DEFAULT_POST_UPGRADE_MINIMUM_GAS },
|e| quote! { #e },
);

let hook_implementation = match &hook {
// Should we generate an UpgradeHook implementation with body?
Expand Down
71 changes: 57 additions & 14 deletions src/approval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,35 +26,51 @@ pub trait Action<Cont: ?Sized> {
fn execute(self, contract: &mut Cont) -> Self::Output;
}

/// Defines the operating parameters for an ApprovalManager and performs
/// approvals
/// Defines the operating parameters for an `ApprovalManager` and performs
/// approvals.
pub trait ApprovalConfiguration<A, S> {
/// Errors when approving a request
/// Errors when approving a request.
type ApprovalError;
/// Errors when removing a request
/// Errors when removing a request.
type RemovalError;
/// Errors when authorizing an account
/// Errors when authorizing an account.
type AuthorizationError;
/// Errors when evaluating a request for execution candidacy
/// Errors when evaluating a request for execution candidacy.
type ExecutionEligibilityError;

/// Has the request reached full approval?
///
/// # Errors
///
/// Returns an error if the request is not approved for execution.
fn is_approved_for_execution(
&self,
action_request: &ActionRequest<A, S>,
) -> Result<(), Self::ExecutionEligibilityError>;

/// Can this request be removed by an allowed account?
///
/// # Errors
///
/// Returns an error if the request cannot be removed.
fn is_removable(&self, action_request: &ActionRequest<A, S>) -> Result<(), Self::RemovalError>;

/// Is the account allowed to execute, approve, or remove this request?
///
/// # Errors
///
/// Returns an error if the account is not allowed to perform such an action.
fn is_account_authorized(
&self,
account_id: &AccountId,
action_request: &ActionRequest<A, S>,
) -> Result<(), Self::AuthorizationError>;

/// Modify action_request.approval_state in-place to increase approval
/// Modify `action_request.approval_state` in-place to increase approval.
///
/// # Errors
///
/// Returns an error if the request cannot be approved.
fn try_approve_with_authorized_account(
&self,
account_id: AccountId,
Expand Down Expand Up @@ -135,22 +151,26 @@ where
C: ApprovalConfiguration<A, S> + BorshDeserialize + BorshSerialize,
{
/// Storage root
#[must_use]
fn root() -> Slot<()> {
Slot::new(DefaultStorageKey::ApprovalManager)
}

/// Because requests will be deleted from the requests collection,
/// maintain a simple counter to guarantee unique IDs
#[must_use]
fn slot_next_request_id() -> Slot<u32> {
Self::root().field(ApprovalStorageKey::NextRequestId)
}

/// Approval context included in relevant approval-related calls
#[must_use]
fn slot_config() -> Slot<C> {
Self::root().field(ApprovalStorageKey::Config)
}

/// Current list of pending action requests.
#[must_use]
fn slot_request(request_id: u32) -> Slot<ActionRequest<A, S>> {
Self::root().field(ApprovalStorageKey::Request(request_id))
}
Expand All @@ -175,7 +195,11 @@ where
/// once.
fn init(config: C);

/// Creates a new action request initialized with the given approval state
/// Creates a new action request initialized with the given approval state.
///
/// # Errors
///
/// - If the acting account is unauthorized.
fn create_request(
&mut self,
action: A,
Expand All @@ -184,23 +208,42 @@ where

/// Executes an action request and removes it from the collection if the
/// approval state of the request is fulfilled.
///
/// # Errors
///
/// - If the acting account is unauthorized.
/// - If the request is ineligible for execution.
fn execute_request(
&mut self,
request_id: u32,
) -> Result<A::Output, ExecutionError<C::AuthorizationError, C::ExecutionEligibilityError>>;

/// Is the given request ID able to be executed if such a request were to
/// be initiated by an authorized account?
///
/// # Errors
///
/// - If the request is ineligible for execution.
fn is_approved_for_execution(request_id: u32) -> Result<(), C::ExecutionEligibilityError>;

/// Tries to approve the action request designated by the given request ID
/// with the given arguments. Panics if the request ID does not exist.
///
/// # Errors
///
/// - If the acting account is unauthorized.
/// - If another error was encountered when approving the request.
fn approve_request(
&mut self,
request_id: u32,
) -> Result<(), ApprovalError<C::AuthorizationError, C::ApprovalError>>;

/// Tries to remove the action request indicated by request_id.
/// Tries to remove the action request indicated by `request_id`.
///
/// # Errors
///
/// - If the acting account is unauthorized.
/// - If the request cannot be removed.
fn remove_request(
&mut self,
request_id: u32,
Expand Down Expand Up @@ -495,7 +538,7 @@ mod tests {

predecessor(&alice);
let request_id = contract
.create_request(MyAction::SayHello, Default::default())
.create_request(MyAction::SayHello, MultisigApprovalState::default())
.unwrap();

assert_eq!(request_id, 0);
Expand Down Expand Up @@ -524,7 +567,7 @@ mod tests {

predecessor(&alice);
let request_id = contract
.create_request(MyAction::SayHello, Default::default())
.create_request(MyAction::SayHello, MultisigApprovalState::default())
.unwrap();

contract.approve_request(request_id).unwrap();
Expand All @@ -544,7 +587,7 @@ mod tests {
predecessor(&alice);

let request_id = contract
.create_request(MyAction::SayHello, Default::default())
.create_request(MyAction::SayHello, MultisigApprovalState::default())
.unwrap();

contract.approve_request(request_id).unwrap();
Expand All @@ -565,7 +608,7 @@ mod tests {
predecessor(&alice);

let request_id = contract
.create_request(MyAction::SayHello, Default::default())
.create_request(MyAction::SayHello, MultisigApprovalState::default())
.unwrap();

contract.approve_request(request_id).unwrap();
Expand All @@ -589,7 +632,7 @@ mod tests {

predecessor(&alice);
let request_id = contract
.create_request(MyAction::SayGoodbye, Default::default())
.create_request(MyAction::SayGoodbye, MultisigApprovalState::default())
.unwrap();

contract.approve_request(request_id).unwrap();
Expand Down
6 changes: 3 additions & 3 deletions src/approval/native_transaction_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub enum PromiseAction {
}

/// A native protocol-level transaction that (de)serializes into many different
/// formats
/// formats.
#[derive(Eq, PartialEq, Clone, Debug)]
#[near(serializers = [borsh, json])]
pub struct NativeTransactionAction {
Expand Down Expand Up @@ -107,12 +107,12 @@ impl<C> super::Action<C> for NativeTransactionAction {
.unwrap_or(near_sdk::Allowance::Unlimited),
receiver_id,
function_names.join(","),
nonce.map(Into::into).unwrap_or(0),
nonce.map_or(0, Into::into),
),
PromiseAction::AddFullAccessKey { public_key, nonce } => promise
.add_full_access_key_with_nonce(
public_key.parse().unwrap(),
nonce.map(Into::into).unwrap_or(0),
nonce.map_or(0, Into::into),
),
PromiseAction::CreateAccount => promise.create_account(),
PromiseAction::DeployContract { code } => promise.deploy_contract(code.0),
Expand Down
Loading

0 comments on commit a82ae8f

Please sign in to comment.