Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

remove EthAbiType derive generated unwrap #2056

Merged
merged 2 commits into from
Jan 17, 2023

Conversation

zsluedem
Copy link
Contributor

Motivation

#2053

Solution

Replace unwrap with safer method

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog
  • Breaking changes

Comment on lines 43 to 49
quote_spanned! { f.span() => #name: iter
.next()
.ok_or_else(|| #core_crate::abi::InvalidOutputType(::std::format!(
"Struct field {:} doesn't have correspond token.", #name_str
)))
.and_then(|t| #core_crate::abi::Tokenizable::from_token(t))?
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just use an expect here, because there's already a len check so this is guaranteed to succeed, hence the unwrap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I don't have problems with expect, there is also https://rust-lang.github.io/rust-clippy/master/#expect_used lint kind which some people might want to warn about it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this error can never happen though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know.Just want to pass the lint check for unwrap in my situation. If you want expect, then expect it is.

@zsluedem zsluedem force-pushed the EthAbiType-remove-unwrap-used branch from 108ca1a to 8be05c3 Compare January 16, 2023 01:18
@zsluedem zsluedem requested a review from mattsse January 16, 2023 14:08
@zsluedem zsluedem force-pushed the EthAbiType-remove-unwrap-used branch from 8be05c3 to 9dcc5a2 Compare January 17, 2023 02:23
@gakonst gakonst merged commit c60990d into gakonst:master Jan 17, 2023
@zsluedem zsluedem deleted the EthAbiType-remove-unwrap-used branch January 18, 2023 01:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants