From 1bec1a9ab7b40fdbc5897efd6aebf2dc771ac21c Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 29 Sep 2023 06:29:09 +0200 Subject: [PATCH] feat(sol-macro): add support for overloaded events --- crates/sol-macro/src/expand/contract.rs | 13 +- crates/sol-macro/src/expand/event.rs | 3 +- crates/sol-macro/src/expand/mod.rs | 189 ++++++++++++++------- crates/sol-types/tests/ui/function.rs | 33 ---- crates/sol-types/tests/ui/function.stderr | 60 ------- crates/sol-types/tests/ui/overloads.rs | 110 ++++++++++++ crates/sol-types/tests/ui/overloads.stderr | 119 +++++++++++++ crates/syn-solidity/src/item/event.rs | 24 +++ 8 files changed, 395 insertions(+), 156 deletions(-) create mode 100644 crates/sol-types/tests/ui/overloads.rs create mode 100644 crates/sol-types/tests/ui/overloads.stderr diff --git a/crates/sol-macro/src/expand/contract.rs b/crates/sol-macro/src/expand/contract.rs index 8f925171a3..c3e701cc07 100644 --- a/crates/sol-macro/src/expand/contract.rs +++ b/crates/sol-macro/src/expand/contract.rs @@ -170,7 +170,7 @@ impl<'a> CallLikeExpander<'a> { ) -> Self { let variants: Vec<_> = functions .iter() - .map(|f| cx.function_name_ident(f).0) + .map(|&f| cx.overloaded_name(f.into()).0) .collect(); let types: Vec<_> = variants.iter().map(|name| cx.raw_call_name(name)).collect(); @@ -211,13 +211,18 @@ impl<'a> CallLikeExpander<'a> { } fn from_events(cx: &'a ExpCtxt<'a>, contract_name: &SolIdent, events: Vec<&ItemEvent>) -> Self { + let variants: Vec<_> = events + .iter() + .map(|&event| cx.overloaded_name(event.into()).0) + .collect(); + let mut selectors: Vec<_> = events.iter().map(|e| cx.event_selector(e)).collect(); selectors.sort_unstable_by_key(|a| a.array); Self { cx, name: format_ident!("{contract_name}Events"), - variants: events.iter().map(|event| event.name.0.clone()).collect(), + variants, min_data_len: events .iter() .map(|event| ty::params_base_data_size(cx, &event.params())) @@ -228,8 +233,8 @@ impl<'a> CallLikeExpander<'a> { } } - /// Type name overrides. Currently only functions support this through - /// overloading. + /// Type name overrides. Currently only functions support because of the + /// `Call` suffix. fn types(&self) -> &[Ident] { match &self.data { CallLikeExpanderData::Function { types, .. } => types, diff --git a/crates/sol-macro/src/expand/event.rs b/crates/sol-macro/src/expand/event.rs index 57473754c4..7a6ae38604 100644 --- a/crates/sol-macro/src/expand/event.rs +++ b/crates/sol-macro/src/expand/event.rs @@ -18,7 +18,7 @@ use syn::Result; /// } /// ``` pub(super) fn expand(cx: &ExpCtxt<'_>, event: &ItemEvent) -> Result { - let ItemEvent { name, attrs, .. } = event; + let ItemEvent { attrs, .. } = event; let params = event.params(); let (_sol_attrs, mut attrs) = crate::attr::SolAttrs::parse(attrs)?; @@ -27,6 +27,7 @@ pub(super) fn expand(cx: &ExpCtxt<'_>, event: &ItemEvent) -> Result cx.assert_resolved(¶ms)?; event.assert_valid()?; + let name = cx.overloaded_name(event.into()); let signature = cx.signature(name.as_string(), ¶ms); let selector = crate::utils::event_selector(&signature); let anonymous = event.is_anonymous(); diff --git a/crates/sol-macro/src/expand/mod.rs b/crates/sol-macro/src/expand/mod.rs index 375aa06c72..83f2e0bb44 100644 --- a/crates/sol-macro/src/expand/mod.rs +++ b/crates/sol-macro/src/expand/mod.rs @@ -38,10 +38,10 @@ struct ExpCtxt<'ast> { all_items: Vec<&'ast Item>, custom_types: HashMap, - /// `name => functions` - functions: HashMap>, - /// `function_signature => new_name` - function_overloads: HashMap, + /// `name => item` + overloaded_items: HashMap>>, + /// `signature => new_name` + overloads: HashMap, attrs: SolAttrs, ast: &'ast File, @@ -53,8 +53,8 @@ impl<'ast> ExpCtxt<'ast> { Self { all_items: Vec::new(), custom_types: HashMap::new(), - functions: HashMap::new(), - function_overloads: HashMap::new(), + overloaded_items: HashMap::new(), + overloads: HashMap::new(), attrs: SolAttrs::default(), ast, } @@ -102,7 +102,7 @@ impl<'ast> ExpCtxt<'ast> { } // resolve -impl ExpCtxt<'_> { +impl<'ast> ExpCtxt<'ast> { fn parse_file_attributes(&mut self) -> Result<()> { let (attrs, others) = attr::SolAttrs::parse(&self.ast.attrs)?; self.attrs = attrs; @@ -165,23 +165,26 @@ impl ExpCtxt<'_> { } fn mk_overloads_map(&mut self) -> Result<()> { - let all_orig_names: Vec = self - .functions + let all_orig_names: Vec<_> = self + .overloaded_items .values() .flatten() - .filter_map(|f| f.name.clone()) + .filter_map(|f| f.name()) .collect(); - let mut overloads_map = std::mem::take(&mut self.function_overloads); + let mut overloads_map = std::mem::take(&mut self.overloads); // report all errors at the end let mut errors = Vec::new(); - for functions in self.functions.values().filter(|fs| fs.len() >= 2) { + for functions in self.overloaded_items.values().filter(|fs| fs.len() >= 2) { // check for same parameters for (i, &a) in functions.iter().enumerate() { for &b in functions.iter().skip(i + 1) { - if a.arguments.types().eq(b.arguments.types()) { - let msg = "function with same name and parameter types defined twice"; + if a.eq_by_types(b) { + let msg = format!( + "{} with same name and parameter types defined twice", + a.desc() + ); let mut err = syn::Error::new(a.span(), msg); let msg = "other declaration is here"; @@ -193,15 +196,16 @@ impl ExpCtxt<'_> { } } - for (i, &function) in functions.iter().enumerate() { - let Some(old_name) = function.name.as_ref() else { + for (i, &item) in functions.iter().enumerate() { + let Some(old_name) = item.name() else { continue }; let new_name = format!("{old_name}_{i}"); if let Some(other) = all_orig_names.iter().find(|x| x.0 == new_name) { let msg = format!( - "function `{old_name}` is overloaded, \ - but the generated name `{new_name}` is already in use" + "{} `{old_name}` is overloaded, \ + but the generated name `{new_name}` is already in use", + item.desc() ); let mut err = syn::Error::new(old_name.span(), msg); @@ -212,12 +216,12 @@ impl ExpCtxt<'_> { errors.push(err); } - overloads_map.insert(self.function_signature(function), new_name); + overloads_map.insert(item.signature(self), new_name); } } utils::combine_errors(errors).map(|()| { - self.function_overloads = overloads_map; + self.overloads = overloads_map; }) } } @@ -230,17 +234,81 @@ impl<'ast> Visit<'ast> for ExpCtxt<'ast> { fn visit_item_function(&mut self, function: &'ast ItemFunction) { if let Some(name) = &function.name { - self.functions + self.overloaded_items .entry(name.as_string()) .or_default() - .push(function); + .push(OverloadedItem::Function(function)); } ast::visit::visit_item_function(self, function); } + + fn visit_item_event(&mut self, event: &'ast ItemEvent) { + self.overloaded_items + .entry(event.name.as_string()) + .or_default() + .push(OverloadedItem::Event(event)); + ast::visit::visit_item_event(self, event); + } +} + +#[derive(Clone, Copy)] +enum OverloadedItem<'a> { + Function(&'a ItemFunction), + Event(&'a ItemEvent), +} + +impl<'ast> From<&'ast ItemFunction> for OverloadedItem<'ast> { + fn from(f: &'ast ItemFunction) -> Self { + Self::Function(f) + } +} + +impl<'ast> From<&'ast ItemEvent> for OverloadedItem<'ast> { + fn from(e: &'ast ItemEvent) -> Self { + Self::Event(e) + } +} + +impl<'a> OverloadedItem<'a> { + fn name(self) -> Option<&'a SolIdent> { + match self { + Self::Function(f) => f.name.as_ref(), + Self::Event(e) => Some(&e.name), + } + } + + fn desc(&self) -> &'static str { + match self { + Self::Function(_) => "function", + Self::Event(_) => "event", + } + } + + fn eq_by_types(self, other: Self) -> bool { + match (self, other) { + (Self::Function(a), Self::Function(b)) => a.arguments.types().eq(b.arguments.types()), + (Self::Event(a), Self::Event(b)) => a.param_types().eq(b.param_types()), + _ => false, + } + } + + fn span(self) -> Span { + match self { + Self::Function(f) => f.span(), + Self::Event(e) => e.span(), + } + } + + fn signature(self, cx: &ExpCtxt<'a>) -> String { + match self { + Self::Function(f) => cx.function_signature(f), + Self::Event(e) => cx.event_signature(e), + } + } } // utils -impl ExpCtxt<'_> { +impl<'ast> ExpCtxt<'ast> { #[allow(dead_code)] fn get_item(&self, name: &SolPath) -> &Item { match self.try_get_item(name) { @@ -266,59 +334,44 @@ impl ExpCtxt<'_> { /// Returns the name of the function, adjusted for overloads. fn function_name(&self, function: &ItemFunction) -> String { - let sig = self.function_signature(function); - match self.function_overloads.get(&sig) { - Some(name) => name.clone(), - None => function.name().as_string(), - } + self.overloaded_name(function.into()).as_string() } - /// Returns the name of the function, adjusted for overloads. - fn function_name_ident(&self, function: &ItemFunction) -> SolIdent { - let sig = self.function_signature(function); - match self.function_overloads.get(&sig) { - Some(name) => SolIdent::new_spanned(name, function.name().span()), - None => function.name().clone(), + /// Returns the name of the given item, adjusted for overloads. + /// + /// Use `.into()` to convert from `&ItemFunction` or `&ItemEvent`. + fn overloaded_name(&self, item: OverloadedItem<'ast>) -> SolIdent { + let original_ident = item.name().expect("item has no name"); + let sig = item.signature(self); + match self.overloads.get(&sig) { + Some(name) => SolIdent::new_spanned(name, original_ident.span()), + None => original_ident.clone(), } } - fn raw_call_name(&self, function_name: impl quote::IdentFragment + std::fmt::Display) -> Ident { - format_ident!("{function_name}Call") - } - + /// Returns the name of the function's call Rust struct. fn call_name(&self, function: &ItemFunction) -> Ident { let function_name = self.function_name(function); self.raw_call_name(function_name) } - fn raw_return_name( - &self, - function_name: impl quote::IdentFragment + std::fmt::Display, - ) -> Ident { - format_ident!("{function_name}Return") + /// Formats the given name as a function's call Rust struct name. + fn raw_call_name(&self, function_name: impl quote::IdentFragment + std::fmt::Display) -> Ident { + format_ident!("{function_name}Call") } + /// Returns the name of the function's return Rust struct. fn return_name(&self, function: &ItemFunction) -> Ident { let function_name = self.function_name(function); self.raw_return_name(function_name) } - fn signature<'a, I: IntoIterator>( + /// Formats the given name as a function's return Rust struct name. + fn raw_return_name( &self, - mut name: String, - params: I, - ) -> String { - name.push('('); - let mut first = true; - for param in params { - if !first { - name.push(','); - } - write!(name, "{}", ty::TypePrinter::new(self, ¶m.ty)).unwrap(); - first = false; - } - name.push(')'); - name + function_name: impl quote::IdentFragment + std::fmt::Display, + ) -> Ident { + format_ident!("{function_name}Return") } fn function_signature(&self, function: &ItemFunction) -> String { @@ -347,6 +400,25 @@ impl ExpCtxt<'_> { utils::event_selector(self.event_signature(event)) } + /// Formats the name and parameters of the function as a Solidity signature. + fn signature<'a, I: IntoIterator>( + &self, + mut name: String, + params: I, + ) -> String { + name.push('('); + let mut first = true; + for param in params { + if !first { + name.push(','); + } + write!(name, "{}", ty::TypePrinter::new(self, ¶m.ty)).unwrap(); + first = false; + } + name.push(')'); + name + } + /// Extends `attrs` with all possible derive attributes for the given type /// if `#[sol(all_derives)]` was passed. /// @@ -374,6 +446,7 @@ impl ExpCtxt<'_> { self.type_derives(attrs, params.into_iter().map(|p| &p.ty), derive_default); } + /// Implementation of [`derives`](Self::derives). fn type_derives(&self, attrs: &mut Vec, types: I, mut derive_default: bool) where I: IntoIterator, diff --git a/crates/sol-types/tests/ui/function.rs b/crates/sol-types/tests/ui/function.rs index 0a359e0d8c..ab38d47d47 100644 --- a/crates/sol-types/tests/ui/function.rs +++ b/crates/sol-types/tests/ui/function.rs @@ -72,37 +72,4 @@ sol! { function n() public pure returns (uint256,); } -// OK -sol! { - function overloaded(); - function overloaded(uint256); - function overloaded(uint256, address); - function overloaded(address); - function overloaded(address, string); -} - -sol! { - function overloadTaken(); - function overloadTaken(uint256); - - function overloadTaken_0(); - function overloadTaken_1(); - function overloadTaken_2(); -} - -sol! { - function sameFnOverload(); - function sameFnOverload(); -} - -sol! { - function sameFnTysOverload1(uint256[] memory a); - function sameFnTysOverload1(uint256[] storage b); -} - -sol! { - function sameFnTysOverload2(string memory, string storage); - function sameFnTysOverload2(string storage b, string calldata); -} - fn main() {} diff --git a/crates/sol-types/tests/ui/function.stderr b/crates/sol-types/tests/ui/function.stderr index 5a2ae455b3..7b53f8633e 100644 --- a/crates/sol-types/tests/ui/function.stderr +++ b/crates/sol-types/tests/ui/function.stderr @@ -51,63 +51,3 @@ error: expected at least one return type | 50 | function badReturn2() returns(); | ^^ - -error: function `overloadTaken` is overloaded, but the generated name `overloadTaken_0` is already in use - --> tests/ui/function.rs:85:14 - | -85 | function overloadTaken(); - | ^^^^^^^^^^^^^ - -error: other declaration is here - --> tests/ui/function.rs:88:14 - | -88 | function overloadTaken_0(); - | ^^^^^^^^^^^^^^^ - -error: function `overloadTaken` is overloaded, but the generated name `overloadTaken_1` is already in use - --> tests/ui/function.rs:86:14 - | -86 | function overloadTaken(uint256); - | ^^^^^^^^^^^^^ - -error: other declaration is here - --> tests/ui/function.rs:89:14 - | -89 | function overloadTaken_1(); - | ^^^^^^^^^^^^^^^ - -error: function with same name and parameter types defined twice - --> tests/ui/function.rs:94:14 - | -94 | function sameFnOverload(); - | ^^^^^^^^^^^^^^ - -error: other declaration is here - --> tests/ui/function.rs:95:14 - | -95 | function sameFnOverload(); - | ^^^^^^^^^^^^^^ - -error: function with same name and parameter types defined twice - --> tests/ui/function.rs:99:14 - | -99 | function sameFnTysOverload1(uint256[] memory a); - | ^^^^^^^^^^^^^^^^^^ - -error: other declaration is here - --> tests/ui/function.rs:100:14 - | -100 | function sameFnTysOverload1(uint256[] storage b); - | ^^^^^^^^^^^^^^^^^^ - -error: function with same name and parameter types defined twice - --> tests/ui/function.rs:104:14 - | -104 | function sameFnTysOverload2(string memory, string storage); - | ^^^^^^^^^^^^^^^^^^ - -error: other declaration is here - --> tests/ui/function.rs:105:14 - | -105 | function sameFnTysOverload2(string storage b, string calldata); - | ^^^^^^^^^^^^^^^^^^ diff --git a/crates/sol-types/tests/ui/overloads.rs b/crates/sol-types/tests/ui/overloads.rs new file mode 100644 index 0000000000..605656a9da --- /dev/null +++ b/crates/sol-types/tests/ui/overloads.rs @@ -0,0 +1,110 @@ +use alloy_sol_types::sol; + +mod function { + use super::*; + + sol! { + function overloaded(); + function overloaded(uint256); + function overloaded(uint256,address); + function overloaded(address); + function overloaded(address,string); + } + + sol! { + function overloadTaken(); + function overloadTaken(uint256); + function overloadTaken_0(); + function overloadTaken_1(); + function overloadTaken_2(); + } + + sol! { + function sameOverload(); + function sameOverload(); + } + + sol! { + function sameTysOverload1(uint256[]memory a); + function sameTysOverload1(uint256[]storage b); + } + + sol! { + function sameTysOverload2(string memory,string storage); + function sameTysOverload2(string storage b,string calldata); + } +} + +mod event { + use super::*; + + sol! { + event overloaded(); + event overloaded(uint256); + event overloaded(uint256,address); + event overloaded(address); + event overloaded(address,string); + } + + sol! { + event overloadTaken(); + event overloadTaken(uint256); + event overloadTaken_0(); + event overloadTaken_1(); + event overloadTaken_2(); + } + + sol! { + event sameOverload(); + event sameOverload(); + } + + sol! { + event sameTysOverload1(uint256[] a); + event sameTysOverload1(uint256[] b); + } + + sol! { + event sameTysOverload2(string, string); + event sameTysOverload2(string, string); + } +} + +/* +mod error { + use super::*; + + sol! { + error overloaded(); + error overloaded(uint256); + error overloaded(uint256,address); + error overloaded(address); + error overloaded(address,string); + } + + sol! { + error overloadTaken(); + error overloadTaken(uint256); + error overloadTaken_0(); + error overloadTaken_1(); + error overloadTaken_2(); + } + + sol! { + error sameOverload(); + error sameOverload(); + } + + sol! { + error sameTysOverload1(uint256[] a); + error sameTysOverload1(uint256[] b); + } + + sol! { + error sameTysOverload2(string, string); + error sameTysOverload2(string, string); + } +} +*/ + +fn main() {} diff --git a/crates/sol-types/tests/ui/overloads.stderr b/crates/sol-types/tests/ui/overloads.stderr new file mode 100644 index 0000000000..8fae12d9e0 --- /dev/null +++ b/crates/sol-types/tests/ui/overloads.stderr @@ -0,0 +1,119 @@ +error: function `overloadTaken` is overloaded, but the generated name `overloadTaken_0` is already in use + --> tests/ui/overloads.rs:15:18 + | +15 | function overloadTaken(); + | ^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:17:18 + | +17 | function overloadTaken_0(); + | ^^^^^^^^^^^^^^^ + +error: function `overloadTaken` is overloaded, but the generated name `overloadTaken_1` is already in use + --> tests/ui/overloads.rs:16:18 + | +16 | function overloadTaken(uint256); + | ^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:18:18 + | +18 | function overloadTaken_1(); + | ^^^^^^^^^^^^^^^ + +error: function with same name and parameter types defined twice + --> tests/ui/overloads.rs:23:18 + | +23 | function sameOverload(); + | ^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:24:18 + | +24 | function sameOverload(); + | ^^^^^^^^^^^^ + +error: function with same name and parameter types defined twice + --> tests/ui/overloads.rs:28:18 + | +28 | function sameTysOverload1(uint256[]memory a); + | ^^^^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:29:18 + | +29 | function sameTysOverload1(uint256[]storage b); + | ^^^^^^^^^^^^^^^^ + +error: function with same name and parameter types defined twice + --> tests/ui/overloads.rs:33:18 + | +33 | function sameTysOverload2(string memory,string storage); + | ^^^^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:34:18 + | +34 | function sameTysOverload2(string storage b,string calldata); + | ^^^^^^^^^^^^^^^^ + +error: event `overloadTaken` is overloaded, but the generated name `overloadTaken_0` is already in use + --> tests/ui/overloads.rs:50:15 + | +50 | event overloadTaken(); + | ^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:52:15 + | +52 | event overloadTaken_0(); + | ^^^^^^^^^^^^^^^ + +error: event `overloadTaken` is overloaded, but the generated name `overloadTaken_1` is already in use + --> tests/ui/overloads.rs:51:15 + | +51 | event overloadTaken(uint256); + | ^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:53:15 + | +53 | event overloadTaken_1(); + | ^^^^^^^^^^^^^^^ + +error: event with same name and parameter types defined twice + --> tests/ui/overloads.rs:58:15 + | +58 | event sameOverload(); + | ^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:59:15 + | +59 | event sameOverload(); + | ^^^^^^^^^^^^ + +error: event with same name and parameter types defined twice + --> tests/ui/overloads.rs:63:15 + | +63 | event sameTysOverload1(uint256[] a); + | ^^^^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:64:15 + | +64 | event sameTysOverload1(uint256[] b); + | ^^^^^^^^^^^^^^^^ + +error: event with same name and parameter types defined twice + --> tests/ui/overloads.rs:68:15 + | +68 | event sameTysOverload2(string, string); + | ^^^^^^^^^^^^^^^^ + +error: other declaration is here + --> tests/ui/overloads.rs:69:15 + | +69 | event sameTysOverload2(string, string); + | ^^^^^^^^^^^^^^^^ diff --git a/crates/syn-solidity/src/item/event.rs b/crates/syn-solidity/src/item/event.rs index d9fa4090a5..382cbd1c36 100644 --- a/crates/syn-solidity/src/item/event.rs +++ b/crates/syn-solidity/src/item/event.rs @@ -107,6 +107,30 @@ impl ItemEvent { .collect() } + pub fn param_types( + &self, + ) -> impl ExactSizeIterator + DoubleEndedIterator + Clone { + self.parameters.iter().map(|var| &var.ty) + } + + pub fn param_types_mut( + &mut self, + ) -> impl ExactSizeIterator + DoubleEndedIterator { + self.parameters.iter_mut().map(|var| &mut var.ty) + } + + pub fn param_types_and_names( + &self, + ) -> impl ExactSizeIterator)> + DoubleEndedIterator { + self.parameters.iter().map(|p| (&p.ty, p.name.as_ref())) + } + + pub fn param_type_strings( + &self, + ) -> impl ExactSizeIterator + DoubleEndedIterator + Clone + '_ { + self.parameters.iter().map(|var| var.ty.to_string()) + } + pub fn non_indexed_params(&self) -> impl Iterator { self.parameters.iter().filter(|p| !p.is_indexed()) }