From 7e4e8e200aeb0af8a766e78daa61bac5b233e5e9 Mon Sep 17 00:00:00 2001 From: James Prestwich <10149425+prestwich@users.noreply.github.com> Date: Wed, 2 Feb 2022 12:44:53 -0800 Subject: [PATCH] refactor: replace anyhow with eyre (#858) * refactor: replace anyhow with eyre * chore: update changelog * refactor: simplify bail logic for duplicate method signatures * refactor: simplify bail logic throughout * refactor: use eyre::ensure * refactor: more idiomatic use of eyre --- CHANGELOG.md | 2 + Cargo.lock | 18 ++++++- .../ethers-contract-abigen/Cargo.toml | 2 +- .../ethers-contract-abigen/src/contract.rs | 6 +-- .../src/contract/events.rs | 2 +- .../src/contract/methods.rs | 52 +++++++++---------- .../src/contract/structs.rs | 26 ++++------ .../src/contract/types.rs | 10 ++-- .../ethers-contract-abigen/src/lib.rs | 6 +-- .../ethers-contract-abigen/src/multi.rs | 12 ++--- .../ethers-contract-abigen/src/rustfmt.rs | 18 +++---- .../ethers-contract-abigen/src/source.rs | 16 +++--- .../ethers-contract-abigen/src/util.rs | 10 ++-- 13 files changed, 94 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b0bc972..85885018 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ - Significantly refactor `MultiAbigen` module generation. Now allows for lib generation, and does not make unnecessary disk writes. [#854](https://github.com/gakonst/ethers-rs/pull/852) +- Refactor `ethers-contract-abigen` to use `eyre` instead of `anyhow` via + [#858](https://github.com/gakonst/ethers-rs/pull/858) ## ethers-contract-abigen diff --git a/Cargo.lock b/Cargo.lock index 93a6a512..833b7f38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1179,10 +1179,10 @@ name = "ethers-contract-abigen" version = "0.6.0" dependencies = [ "Inflector", - "anyhow", "cfg-if 1.0.0", "dunce", "ethers-core", + "eyre", "getrandom 0.2.4", "hex", "once_cell", @@ -1418,6 +1418,16 @@ dependencies = [ "wee_alloc", ] +[[package]] +name = "eyre" +version = "0.6.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc225d8f637923fe585089fcf03e705c222131232d2c1fb622e84ecf725d0eb8" +dependencies = [ + "indenter", + "once_cell", +] + [[package]] name = "fake-simd" version = "0.1.2" @@ -1881,6 +1891,12 @@ dependencies = [ "syn", ] +[[package]] +name = "indenter" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" + [[package]] name = "indexmap" version = "1.8.0" diff --git a/ethers-contract/ethers-contract-abigen/Cargo.toml b/ethers-contract/ethers-contract-abigen/Cargo.toml index ef9b3a9a..4c25310c 100644 --- a/ethers-contract/ethers-contract-abigen/Cargo.toml +++ b/ethers-contract/ethers-contract-abigen/Cargo.toml @@ -12,7 +12,6 @@ keywords = ["ethereum", "web3", "celo", "ethers"] [dependencies] ethers-core = { version = "^0.6.0", path = "../../ethers-core", features = ["macros"] } -anyhow = "1.0.37" Inflector = "0.11" proc-macro2 = "1.0" quote = "1.0" @@ -26,6 +25,7 @@ once_cell = "1.8.0" cfg-if = "1.0.0" dunce = "1.0.2" walkdir = "2.3.2" +eyre = "0.6.6" [target.'cfg(target_arch = "wasm32")'.dependencies] # NOTE: this enables wasm compatibility for getrandom indirectly diff --git a/ethers-contract/ethers-contract-abigen/src/contract.rs b/ethers-contract/ethers-contract-abigen/src/contract.rs index 42c1feba..813bf575 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract.rs @@ -7,11 +7,11 @@ mod types; use super::{util, Abigen}; use crate::{contract::structs::InternalStructs, rawabi::RawAbi}; -use anyhow::{anyhow, Context as _, Result}; use ethers_core::{ abi::{Abi, AbiParser}, macros::{ethers_contract_crate, ethers_core_crate, ethers_providers_crate}, }; +use eyre::{eyre, Context as _, Result}; use crate::contract::methods::MethodAlias; use proc_macro2::{Ident, Literal, TokenStream}; @@ -153,7 +153,7 @@ impl Context { pub fn from_abigen(args: Abigen) -> Result { // get the actual ABI string let mut abi_str = - args.abi_source.get().map_err(|e| anyhow!("failed to get ABI JSON: {}", e))?; + args.abi_source.get().map_err(|e| eyre!("failed to get ABI JSON: {}", e))?; let (abi, human_readable, abi_parser) = parse_abi(&abi_str)?; @@ -199,7 +199,7 @@ impl Context { }; if method_aliases.insert(signature.clone(), alias).is_some() { - return Err(anyhow!("duplicate method signature '{}' in method aliases", signature,)) + eyre::bail!("duplicate method signature '{}' in method aliases", signature) } } diff --git a/ethers-contract/ethers-contract-abigen/src/contract/events.rs b/ethers-contract/ethers-contract-abigen/src/contract/events.rs index d78787d4..c0d29bc2 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract/events.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract/events.rs @@ -1,9 +1,9 @@ use super::{types, util, Context}; -use anyhow::Result; use ethers_core::{ abi::{Event, EventExt, EventParam, ParamType, SolStruct}, macros::{ethers_contract_crate, ethers_core_crate}, }; +use eyre::Result; use inflector::Inflector; use proc_macro2::{Ident, Literal, TokenStream}; use quote::quote; diff --git a/ethers-contract/ethers-contract-abigen/src/contract/methods.rs b/ethers-contract/ethers-contract-abigen/src/contract/methods.rs index 344d4eef..42e34c28 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract/methods.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract/methods.rs @@ -1,6 +1,6 @@ use std::collections::{btree_map::Entry, BTreeMap, HashMap}; -use anyhow::{Context as _, Result}; +use eyre::{Context as _, Result}; use inflector::Inflector; use proc_macro2::{Literal, TokenStream}; use quote::quote; @@ -428,33 +428,33 @@ impl Context { 0 => { // this may happen if there are functions with different casing, // like `INDEX`and `index` - if overloaded_fun.name != first_fun.name { - let overloaded_id = overloaded_fun.name.to_snake_case(); - let first_fun_id = first_fun.name.to_snake_case(); - if first_fun_id != overloaded_id { - // no conflict - overloaded_id - } else { - let overloaded_alias = MethodAlias { - function_name: util::safe_ident(&overloaded_fun.name), - struct_name: util::safe_ident(&overloaded_fun.name), - }; - aliases.insert(overloaded_fun.abi_signature(), overloaded_alias); - let first_fun_alias = MethodAlias { - function_name: util::safe_ident(&first_fun.name), - struct_name: util::safe_ident(&first_fun.name), - }; - aliases.insert(first_fun.abi_signature(), first_fun_alias); - continue - } + // this should not happen since functions with same + // name and inputs are illegal + eyre::ensure!( + overloaded_fun.name != first_fun.name, + "Function with same name and parameter types defined twice: {}", + overloaded_fun.name + ); + + let overloaded_id = overloaded_fun.name.to_snake_case(); + let first_fun_id = first_fun.name.to_snake_case(); + if first_fun_id != overloaded_id { + // no conflict + overloaded_id } else { - // this should not happen since functions with same name and inputs are - // illegal - anyhow::bail!( - "Function with same name and parameter types defined twice: {}", - overloaded_fun.name - ); + let overloaded_alias = MethodAlias { + function_name: util::safe_ident(&overloaded_fun.name), + struct_name: util::safe_ident(&overloaded_fun.name), + }; + aliases.insert(overloaded_fun.abi_signature(), overloaded_alias); + + let first_fun_alias = MethodAlias { + function_name: util::safe_ident(&first_fun.name), + struct_name: util::safe_ident(&first_fun.name), + }; + aliases.insert(first_fun.abi_signature(), first_fun_alias); + continue } } 1 => { diff --git a/ethers-contract/ethers-contract-abigen/src/contract/structs.rs b/ethers-contract/ethers-contract-abigen/src/contract/structs.rs index ec5fa69a..305db7ee 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract/structs.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract/structs.rs @@ -1,7 +1,7 @@ //! Methods for expanding structs use std::collections::{HashMap, VecDeque}; -use anyhow::{Context as _, Result}; +use eyre::{eyre, Result}; use inflector::Inflector; use proc_macro2::TokenStream; use quote::quote; @@ -55,17 +55,18 @@ impl Context { /// Generates the type definition for the name that matches the given identifier fn generate_internal_struct(&self, id: &str) -> Result { - let sol_struct = self.internal_structs.structs.get(id).context("struct not found")?; + let sol_struct = + self.internal_structs.structs.get(id).ok_or_else(|| eyre!("struct not found"))?; let struct_name = self .internal_structs .rust_type_names .get(id) - .context(format!("No types found for {}", id))?; + .ok_or_else(|| eyre!("No types found for {}", id))?; let tuple = self .internal_structs .struct_tuples .get(id) - .context(format!("No types found for {}", id))? + .ok_or_else(|| eyre!("No types found for {}", id))? .clone(); self.expand_internal_struct(struct_name, sol_struct, tuple) } @@ -102,11 +103,7 @@ impl Context { fields.push(quote! { pub #field_name: #ty }); } FieldType::Mapping(_) => { - return Err(anyhow::anyhow!( - "Mapping types in struct `{}` are not supported {:?}", - name, - field - )) + eyre::bail!("Mapping types in struct `{}` are not supported {:?}", name, field) } } } @@ -137,7 +134,8 @@ impl Context { } fn generate_human_readable_struct(&self, name: &str) -> Result { - let sol_struct = self.abi_parser.structs.get(name).context("struct not found")?; + let sol_struct = + self.abi_parser.structs.get(name).ok_or_else(|| eyre!("struct not found"))?; let mut fields = Vec::with_capacity(sol_struct.fields().len()); let mut param_types = Vec::with_capacity(sol_struct.fields().len()); for field in sol_struct.fields() { @@ -157,18 +155,14 @@ impl Context { .abi_parser .struct_tuples .get(name) - .context(format!("No types found for {}", name))? + .ok_or_else(|| eyre!("No types found for {}", name))? .clone(); let tuple = ParamType::Tuple(tuple); param_types.push(struct_ty.as_param(tuple)); } FieldType::Mapping(_) => { - return Err(anyhow::anyhow!( - "Mapping types in struct `{}` are not supported {:?}", - name, - field - )) + eyre::bail!("Mapping types in struct `{}` are not supported {:?}", name, field) } } } diff --git a/ethers-contract/ethers-contract-abigen/src/contract/types.rs b/ethers-contract/ethers-contract-abigen/src/contract/types.rs index 4100c35f..acde39b3 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract/types.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract/types.rs @@ -1,5 +1,5 @@ -use anyhow::{anyhow, Result}; use ethers_core::{abi::ParamType, macros::ethers_core_crate}; +use eyre::{bail, Result}; use proc_macro2::{Literal, TokenStream}; use quote::quote; @@ -16,7 +16,7 @@ pub(crate) fn expand(kind: &ParamType) -> Result { 5..=8 => Ok(quote! { i64 }), 9..=16 => Ok(quote! { i128 }), 17..=32 => Ok(quote! { I256 }), - _ => Err(anyhow!("unsupported solidity type int{}", n)), + _ => bail!("unsupported solidity type int{}", n), }, ParamType::Uint(n) => match n / 8 { 1 => Ok(quote! { u8 }), @@ -25,7 +25,7 @@ pub(crate) fn expand(kind: &ParamType) -> Result { 5..=8 => Ok(quote! { u64 }), 9..=16 => Ok(quote! { u128 }), 17..=32 => Ok(quote! { #ethers_core::types::U256 }), - _ => Err(anyhow!("unsupported solidity type uint{}", n)), + _ => bail!("unsupported solidity type uint{}", n), }, ParamType::Bool => Ok(quote! { bool }), ParamType::String => Ok(quote! { String }), @@ -46,9 +46,7 @@ pub(crate) fn expand(kind: &ParamType) -> Result { Ok(quote! { [#inner; #size] }) } ParamType::Tuple(members) => { - if members.is_empty() { - return Err(anyhow!("Tuple must have at least 1 member")) - } + eyre::ensure!(!members.is_empty(), "Tuple must have at least 1 member"); let members = members.iter().map(expand).collect::, _>>()?; Ok(quote! { (#(#members,)*) }) diff --git a/ethers-contract/ethers-contract-abigen/src/lib.rs b/ethers-contract/ethers-contract-abigen/src/lib.rs index 85a4abd4..a9721b20 100644 --- a/ethers-contract/ethers-contract-abigen/src/lib.rs +++ b/ethers-contract/ethers-contract-abigen/src/lib.rs @@ -26,7 +26,7 @@ pub use ethers_core::types::Address; pub use source::Source; pub use util::parse_address; -use anyhow::Result; +use eyre::Result; use inflector::Inflector; use proc_macro2::TokenStream; use std::{collections::HashMap, fs::File, io::Write, path::Path}; @@ -90,9 +90,9 @@ impl Abigen { let name = path .as_ref() .file_stem() - .ok_or_else(|| anyhow::format_err!("Missing file stem in path"))? + .ok_or_else(|| eyre::format_err!("Missing file stem in path"))? .to_str() - .ok_or_else(|| anyhow::format_err!("Unable to convert file stem to string"))?; + .ok_or_else(|| eyre::format_err!("Unable to convert file stem to string"))?; Self::new(name, std::fs::read_to_string(path.as_ref())?) } diff --git a/ethers-contract/ethers-contract-abigen/src/multi.rs b/ethers-contract/ethers-contract-abigen/src/multi.rs index 19446d61..013c930e 100644 --- a/ethers-contract/ethers-contract-abigen/src/multi.rs +++ b/ethers-contract/ethers-contract-abigen/src/multi.rs @@ -1,6 +1,6 @@ //! TODO -use anyhow::Result; +use eyre::Result; use inflector::Inflector; use std::{collections::BTreeMap, fs, io::Write, path::Path}; @@ -388,7 +388,7 @@ impl MultiBindings { /// # Returns /// /// `Ok(())` if the freshly generated bindings match with the - /// existing bindings. Otherwise an `Err(_)` containing an `anyhow::Report` + /// existing bindings. Otherwise an `Err(_)` containing an `eyre::Report` /// with more information. /// /// # Example @@ -437,7 +437,7 @@ impl MultiBindings { /// # Returns /// /// `Ok(())` if the freshly generated bindings match with the - /// existing bindings. Otherwise an `Err(_)` containing an `anyhow::Report` + /// existing bindings. Otherwise an `Err(_)` containing an `eyre::Report` /// with more information. /// /// # Example @@ -468,13 +468,13 @@ impl MultiBindings { } fn check_file_in_dir(dir: &Path, file_name: &str, expected_contents: &[u8]) -> Result<()> { - anyhow::ensure!(dir.is_dir(), "Not a directory: {}", dir.display()); + eyre::ensure!(dir.is_dir(), "Not a directory: {}", dir.display()); let file_path = dir.join(file_name); - anyhow::ensure!(file_path.is_file(), "Not a file: {}", file_path.display()); + eyre::ensure!(file_path.is_file(), "Not a file: {}", file_path.display()); let contents = fs::read(file_path).expect("Unable to read file"); - anyhow::ensure!(contents == expected_contents, "file contents do not match"); + eyre::ensure!(contents == expected_contents, "file contents do not match"); Ok(()) } diff --git a/ethers-contract/ethers-contract-abigen/src/rustfmt.rs b/ethers-contract/ethers-contract-abigen/src/rustfmt.rs index c64b7f3e..c9bb546d 100644 --- a/ethers-contract/ethers-contract-abigen/src/rustfmt.rs +++ b/ethers-contract/ethers-contract-abigen/src/rustfmt.rs @@ -1,6 +1,6 @@ //! This module implements basic `rustfmt` code formatting. -use anyhow::{anyhow, Result}; +use eyre::{eyre, Result}; use std::{ io::Write, process::{Command, Stdio}, @@ -18,18 +18,18 @@ where let stdin = rustfmt .stdin .as_mut() - .ok_or_else(|| anyhow!("stdin was not created for `rustfmt` child process"))?; + .ok_or_else(|| eyre!("stdin was not created for `rustfmt` child process"))?; stdin.write_all(source.as_ref().as_bytes())?; } let output = rustfmt.wait_with_output()?; - if !output.status.success() { - return Err(anyhow!( - "`rustfmt` exited with code {}:\n{}", - output.status, - String::from_utf8_lossy(&output.stderr), - )) - } + + eyre::ensure!( + output.status.success(), + "`rustfmt` exited with code {}:\n{}", + output.status, + String::from_utf8_lossy(&output.stderr), + ); let stdout = String::from_utf8(output.stdout)?; Ok(stdout) diff --git a/ethers-contract/ethers-contract-abigen/src/source.rs b/ethers-contract/ethers-contract-abigen/src/source.rs index 1b8ab08b..d68df2a1 100644 --- a/ethers-contract/ethers-contract-abigen/src/source.rs +++ b/ethers-contract/ethers-contract-abigen/src/source.rs @@ -3,8 +3,8 @@ use super::util; use ethers_core::types::Address; use crate::util::resolve_path; -use anyhow::{anyhow, Context, Error, Result}; use cfg_if::cfg_if; +use eyre::{eyre, Context, Error, Result}; use std::{env, fs, path::Path, str::FromStr}; use url::Url; @@ -88,10 +88,10 @@ impl Source { format!("{}", root.display()) }; let base = Url::parse(&root) - .map_err(|_| anyhow!("root path '{}' is not absolute", root))?; + .map_err(|_| eyre!("root path '{}' is not absolute", root))?; } else { let base = Url::from_directory_path(root) - .map_err(|_| anyhow!("root path '{}' is not absolute", root.display()))?; + .map_err(|_| eyre!("root path '{}' is not absolute", root.display()))?; } } let url = base.join(source)?; @@ -103,19 +103,19 @@ impl Source { url.path() .rsplit('/') .next() - .ok_or_else(|| anyhow!("HTTP URL does not have a path"))?, + .ok_or_else(|| eyre!("HTTP URL does not have a path"))?, ), Some("polygonscan.com") => Source::polygonscan( url.path() .rsplit('/') .next() - .ok_or_else(|| anyhow!("HTTP URL does not have a path"))?, + .ok_or_else(|| eyre!("HTTP URL does not have a path"))?, ), Some("snowtrace.io") => Source::snowtrace( url.path() .rsplit('/') .next() - .ok_or_else(|| anyhow!("HTTP URL does not have a path"))?, + .ok_or_else(|| eyre!("HTTP URL does not have a path"))?, ), _ => Ok(Source::Http(url)), }, @@ -123,7 +123,7 @@ impl Source { "polygonscan" => Source::polygonscan(url.path()), "snowtrace" => Source::snowtrace(url.path()), "npm" => Ok(Source::npm(url.path())), - _ => Err(anyhow!("unsupported URL '{}'", url)), + _ => Err(eyre!("unsupported URL '{}'", url)), } } @@ -232,7 +232,7 @@ fn get_local_contract(path: impl AsRef) -> Result { contract_path = dunce::canonicalize(&path)?; } if !contract_path.exists() { - anyhow::bail!("Unable to find local contract \"{}\"", path.display()) + eyre::bail!("Unable to find local contract \"{}\"", path.display()) } contract_path } else { diff --git a/ethers-contract/ethers-contract-abigen/src/util.rs b/ethers-contract/ethers-contract-abigen/src/util.rs index 7a779990..4c9b6657 100644 --- a/ethers-contract/ethers-contract-abigen/src/util.rs +++ b/ethers-contract/ethers-contract-abigen/src/util.rs @@ -1,7 +1,7 @@ use ethers_core::types::Address; use std::path::PathBuf; -use anyhow::{anyhow, Result}; +use eyre::Result; use inflector::Inflector; use proc_macro2::{Ident, Literal, Span, TokenStream}; use quote::quote; @@ -76,9 +76,7 @@ where S: AsRef, { let address_str = address_str.as_ref(); - if !address_str.starts_with("0x") { - return Err(anyhow!("address must start with '0x'")) - } + eyre::ensure!(address_str.starts_with("0x"), "address must start with '0x'"); Ok(address_str[2..].parse()?) } @@ -89,7 +87,7 @@ pub fn http_get(_url: &str) -> Result { if #[cfg(feature = "reqwest")]{ Ok(reqwest::blocking::get(_url)?.text()?) } else { - Err(anyhow!("HTTP is unsupported")) + eyre::bail!("HTTP is unsupported") } } } @@ -110,7 +108,7 @@ pub fn resolve_path(raw: &str) -> Result { unprocessed = rest; } None => { - anyhow::bail!("Unable to parse a variable from \"{}\"", tail) + eyre::bail!("Unable to parse a variable from \"{}\"", tail) } } }