From 6538a4347fd1c358b959c386470081b023e01d21 Mon Sep 17 00:00:00 2001 From: moana Date: Mon, 4 May 2026 12:21:04 +0200 Subject: [PATCH 1/3] macro: Replace ad-hoc directive parsers with unified ContractDirectives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapse the four hand-rolled `#[contract(...)]` parsers (`feeds`, `expose`, `emits`, `no_event`) into a single typed pass that returns a `ContractDirectives` struct. The old parsers each walked the token stream from scratch and silently dropped malformed input; the unified parser uses syn's `Parse` trait throughout and surfaces shape mismatches as compile errors with spans. Strengthening (intentional, observable only on previously-malformed input): * Typo'd keyword (e.g. `no_events`, `emit`, `expoze`) — error with `unknown contract directive ...; expected one of: feeds, expose, emits, no_event` plus a `did you mean` suggestion for common near-misses. * `feeds = X` without quotes, `feeds` with no value, or `feeds = ""` — error pointing at the offending token. * `expose = (...)` with parens instead of brackets — error. * Malformed `emits` tuples — error. Happy-path expansion is byte-identical to main: validated by diffing `make -C tests/test-contract expand` and `expand-dd` output against a baseline captured before the refactor. Closes #24. --- contract-macro/src/lib.rs | 10 +- contract-macro/src/parse/directives.rs | 755 ++++++++++++++++--------- contract-macro/src/parse/events.rs | 31 +- contract-macro/src/parse/functions.rs | 23 +- contract-macro/src/parse/mod.rs | 2 +- contract-macro/src/parse/module.rs | 45 +- 6 files changed, 554 insertions(+), 312 deletions(-) diff --git a/contract-macro/src/lib.rs b/contract-macro/src/lib.rs index 4b5a702..50a0e5a 100644 --- a/contract-macro/src/lib.rs +++ b/contract-macro/src/lib.rs @@ -213,7 +213,10 @@ pub fn contract(_attr: TokenStream, item: TokenStream) -> TokenStream { } events.extend(parse::emit_calls(impl_block)); // Include events from method-level #[contract(emits = [...])] attributes - events.extend(parse::inherent_method_emits(impl_block)); + match parse::inherent_method_emits(impl_block) { + Ok(method_events) => events.extend(method_events), + Err(e) => return e.to_compile_error().into(), + } } // Extract functions and events from trait impl blocks with expose lists @@ -224,7 +227,10 @@ pub fn contract(_attr: TokenStream, item: TokenStream) -> TokenStream { } events.extend(parse::emit_calls(trait_impl.impl_block)); // Include events from method-level #[contract(emits = [...])] attributes - events.extend(parse::trait_method_emits(trait_impl)); + match parse::trait_method_emits(trait_impl) { + Ok(method_events) => events.extend(method_events), + Err(e) => return e.to_compile_error().into(), + } } // Deduplicate events by topic — first-seen wins. diff --git a/contract-macro/src/parse/directives.rs b/contract-macro/src/parse/directives.rs index d16da47..33efc45 100644 --- a/contract-macro/src/parse/directives.rs +++ b/contract-macro/src/parse/directives.rs @@ -4,346 +4,579 @@ // // Copyright (c) DUSK NETWORK. All rights reserved. -//! Parsers for the `#[contract(...)]` directive on impls and methods. +//! Parser for the `#[contract(...)]` directive. //! -//! These are four ad-hoc parsers (`expose`, `emits`, `feeds`, `no_event`), -//! collected here pending consolidation into a single typed parser. +//! A single typed pass over the directive list yields a [`ContractDirectives`] +//! struct with one field per supported keyword (`feeds`, `expose`, `emits`, +//! `no_event`). Shape mismatches surface as span-anchored `syn::Error`s. use proc_macro2::TokenStream as TokenStream2; use quote::quote; -use syn::Attribute; - -/// Check if method has `#[contract(no_event)]` attribute to suppress the emit -/// validation. -pub(super) fn event_suppressed(attrs: &[Attribute]) -> bool { - attrs.iter().any(|attr| { - if attr.path().is_ident("contract") - && let Ok(meta) = attr.meta.require_list() - { - let tokens = meta.tokens.to_string(); - return tokens.contains("no_event"); - } - false - }) -} +use syn::parse::{Parse, ParseStream}; +use syn::punctuated::Punctuated; +use syn::{ + Attribute, Error as SynError, Ident, LitStr, Path, Token, Type, bracketed, parenthesized, + parse_str, parse2, token, +}; + +use crate::EventInfo; -/// Extract the `feeds` type from a `#[contract(feeds = "Type")]` attribute. +/// Parsed `#[contract(...)]` directives, aggregated across every contract +/// attribute on a given item. /// -/// This attribute specifies the type fed via `abi::feed()` for streaming -/// functions. When present, the data-driver uses this type for -/// `decode_output_fn` instead of the function's return type. +/// Fields default to `None` / `false` when the directive is absent. +#[derive(Default)] +pub(crate) struct ContractDirectives { + /// Type tokens parsed from `feeds = ""`. + pub feeds: Option, + /// Method names from `expose = [m1, m2, ...]`. + pub expose: Option>, + /// Event metadata from `emits = [(topic, EventType), ...]`. + pub emits: Option>, + /// Set when `no_event` appears. + pub no_event: bool, +} + +/// Parse all `#[contract(...)]` attributes on an item into a single typed +/// struct. /// -/// Returns `Some(TokenStream2)` with the feed type if found, `None` otherwise. -pub(super) fn extract_feeds_attribute(attrs: &[Attribute]) -> Option { +/// Unrelated attributes (e.g. `#[doc]`, `#[cfg]`) are ignored. Malformed +/// directive shapes (typo'd keyword, missing value, wrong delimiter) and +/// duplicate directives (whether within one attribute or across multiple) +/// return a `syn::Error` whose span points at the offending token. +pub(crate) fn parse_contract_directives( + attrs: &[Attribute], +) -> Result { + let mut out = ContractDirectives::default(); + for attr in attrs { if !attr.path().is_ident("contract") { continue; } - let Ok(meta) = attr.meta.require_list() else { - continue; - }; + let list = attr.meta.require_list()?; + let parsed: DirectiveList = parse2(list.tokens.clone())?; + for item in parsed.items { + apply_directive(&mut out, item)?; + } + } - // Parse: feeds = "Type" - let tokens = meta.tokens.clone(); - let mut iter = tokens.into_iter().peekable(); + Ok(out) +} - // Look for "feeds" - let Some(proc_macro2::TokenTree::Ident(ident)) = iter.next() else { - continue; - }; - if ident != "feeds" { - continue; +fn apply_directive(out: &mut ContractDirectives, item: DirectiveItem) -> Result<(), SynError> { + let DirectiveItem { keyword, kind } = item; + match kind { + DirectiveKind::Feeds(ts) => set_once(&mut out.feeds, ts, &keyword), + DirectiveKind::Expose(names) => set_once(&mut out.expose, names, &keyword), + DirectiveKind::Emits(events) => set_once(&mut out.emits, events, &keyword), + DirectiveKind::NoEvent => { + if out.no_event { + return Err(duplicate_directive(&keyword)); + } + out.no_event = true; + Ok(()) } + } +} - // Expect "=" - let Some(proc_macro2::TokenTree::Punct(punct)) = iter.next() else { - continue; - }; - if punct.as_char() != '=' { - continue; - } +fn set_once(slot: &mut Option, value: T, keyword: &Ident) -> Result<(), SynError> { + if slot.is_some() { + return Err(duplicate_directive(keyword)); + } + *slot = Some(value); + Ok(()) +} - // Expect string literal with type - let Some(proc_macro2::TokenTree::Literal(lit)) = iter.next() else { - continue; - }; - let lit_str = lit.to_string(); - // Remove quotes from the literal - let type_str = lit_str.trim_matches('"'); +fn duplicate_directive(keyword: &Ident) -> SynError { + SynError::new(keyword.span(), format!("duplicate `{keyword}` directive")) +} - // Parse the type string into tokens - if let Ok(ty) = syn::parse_str::(type_str) { - return Some(quote! { #ty }); - } +struct DirectiveList { + items: Vec, +} + +impl Parse for DirectiveList { + fn parse(input: ParseStream) -> Result { + let items = Punctuated::::parse_terminated(input)? + .into_iter() + .collect(); + Ok(Self { items }) } +} - None +struct DirectiveItem { + /// The keyword identifier (`feeds`, `expose`, `emits`, `no_event`) — kept + /// so duplicate-directive errors can span at the offending occurrence. + keyword: Ident, + kind: DirectiveKind, } -/// Extract the `expose = [method1, method2, ...]` list from a -/// `#[contract(...)]` attribute. -/// -/// Returns `None` if there's no `#[contract(expose = [...])]` attribute. -/// Returns `Some(vec![...])` with the method names if found. -pub(super) fn expose_list(attrs: &[Attribute]) -> Option> { - for attr in attrs { - if !attr.path().is_ident("contract") { - continue; - } +enum DirectiveKind { + Feeds(TokenStream2), + Expose(Vec), + Emits(Vec), + NoEvent, +} - let Ok(meta) = attr.meta.require_list() else { - continue; +impl Parse for DirectiveItem { + fn parse(input: ParseStream) -> Result { + let keyword: Ident = input.parse()?; + let name = keyword.to_string(); + let kind = match name.as_str() { + "no_event" => DirectiveKind::NoEvent, + "feeds" => DirectiveKind::Feeds(parse_feeds(input, &keyword)?), + "expose" => DirectiveKind::Expose(parse_expose(input, &keyword)?), + "emits" => DirectiveKind::Emits(parse_emits(input, &keyword)?), + unknown => { + return Err(SynError::new( + keyword.span(), + unknown_directive_msg(unknown), + )); + } }; + Ok(Self { keyword, kind }) + } +} + +fn parse_feeds(input: ParseStream, kw: &Ident) -> Result { + // For a missing `=`, the most informative span is the keyword; for a + // wrong-shape value (`feeds = SomeType`), syn's parse failure already + // points at the offending token — we just rewrite the message so the + // user sees the directive's expected shape. + let expected = "expected `feeds = \"\"`"; + input + .parse::() + .map_err(|_| SynError::new(kw.span(), expected))?; + let lit: LitStr = input + .parse() + .map_err(|err| SynError::new(err.span(), expected))?; + let ty: Type = parse_str(&lit.value()).map_err(|_| { + SynError::new( + lit.span(), + format!("feeds type `{}` is not a valid Rust type", lit.value()), + ) + })?; + Ok(quote! { #ty }) +} - // Parse: expose = [method1, method2, ...] - let tokens = meta.tokens.clone(); - let mut iter = tokens.into_iter().peekable(); +fn parse_expose(input: ParseStream, kw: &Ident) -> Result, SynError> { + let expected = "expected `expose = [m1, m2, ...]`"; + input + .parse::() + .map_err(|_| SynError::new(kw.span(), expected))?; + if !input.peek(token::Bracket) { + return Err(SynError::new(input.span(), expected)); + } + let content; + bracketed!(content in input); + let names = Punctuated::::parse_terminated(&content)? + .into_iter() + .map(|i| i.to_string()) + .collect(); + Ok(names) +} - // Look for "expose" - let Some(proc_macro2::TokenTree::Ident(ident)) = iter.next() else { - continue; - }; - if ident != "expose" { - continue; - } +fn parse_emits(input: ParseStream, kw: &Ident) -> Result, SynError> { + let expected = "expected `emits = [(topic, EventType), ...]`"; + input + .parse::() + .map_err(|_| SynError::new(kw.span(), expected))?; + if !input.peek(token::Bracket) { + return Err(SynError::new(input.span(), expected)); + } + let content; + bracketed!(content in input); + let tuples = Punctuated::::parse_terminated(&content)?; + Ok(tuples + .into_iter() + .map(|t| EventInfo { + topic: t.topic, + data_type: t.data_type, + }) + .collect()) +} - // Expect "=" - let Some(proc_macro2::TokenTree::Punct(punct)) = iter.next() else { - continue; - }; - if punct.as_char() != '=' { - continue; - } +struct EventTuple { + topic: String, + data_type: TokenStream2, +} - // Expect "[...]" - let Some(proc_macro2::TokenTree::Group(group)) = iter.next() else { - continue; +impl Parse for EventTuple { + fn parse(input: ParseStream) -> Result { + let content; + parenthesized!(content in input); + let topic = if content.peek(LitStr) { + let lit: LitStr = content.parse()?; + lit.value() + } else { + let path: Path = content.parse()?; + path.segments + .iter() + .map(|s| s.ident.to_string()) + .collect::>() + .join("::") }; - if group.delimiter() != proc_macro2::Delimiter::Bracket { - continue; + content.parse::()?; + let ty: Type = content.parse()?; + if !content.is_empty() { + return Err(content.error("expected `(topic, EventType)`")); } + Ok(Self { + topic, + data_type: quote! { #ty }, + }) + } +} - // Parse the method names from the group - let mut methods = Vec::new(); - for token in group.stream() { - if let proc_macro2::TokenTree::Ident(method_ident) = token { - methods.push(method_ident.to_string()); - } - // Skip commas and other punctuation - } +fn unknown_directive_msg(unknown: &str) -> String { + let suggestion = match unknown { + "emit" => Some("emits"), + "no_events" => Some("no_event"), + "exposes" => Some("expose"), + "feed" => Some("feeds"), + _ => None, + }; + match suggestion { + Some(s) => format!("unknown contract directive `{unknown}`; did you mean `{s}`?"), + None => format!( + "unknown contract directive `{unknown}`; expected one of: feeds, expose, emits, no_event" + ), + } +} + +#[cfg(test)] +mod tests { + use syn::{ImplItemFn, ItemImpl, parse_quote}; + + use super::*; - return Some(methods); + fn parse_impl(impl_block: &ItemImpl) -> Result { + parse_contract_directives(&impl_block.attrs) } - None -} + fn parse_method(method: &ImplItemFn) -> Result { + parse_contract_directives(&method.attrs) + } -/// Extract the `emits = [(topic, Type), ...]` list from a `#[contract(...)]` -/// attribute. -/// -/// Returns `None` if there's no `#[contract(emits = [...])]` attribute. -/// Returns `Some(vec![...])` with (topic, `data_type`) pairs if found. -/// -/// Supports two topic formats: -/// - Const path: `(events::OwnershipTransferred::TOPIC, -/// events::OwnershipTransferred)` -/// - String literal: `("my_topic", MyEventType)` -pub(super) fn emits_list(attrs: &[Attribute]) -> Option> { - for attr in attrs { - if !attr.path().is_ident("contract") { - continue; + fn expect_err(result: Result) -> SynError { + match result { + Ok(_) => panic!("expected error"), + Err(e) => e, } + } - let Ok(meta) = attr.meta.require_list() else { - continue; + #[test] + fn empty_attrs_yield_default() { + let impl_block: ItemImpl = parse_quote! { + impl Foo for MyContract {} }; + let d = parse_impl(&impl_block).unwrap(); + assert!(d.feeds.is_none()); + assert!(d.expose.is_none()); + assert!(d.emits.is_none()); + assert!(!d.no_event); + } - // Parse the token stream to find emits = [...] - let tokens = meta.tokens.clone(); - let mut iter = tokens.into_iter().peekable(); + #[test] + fn unrelated_attribute_passes_through() { + let method: ImplItemFn = parse_quote! { + #[doc = "hello"] + #[allow(dead_code)] + #[cfg(test)] + fn foo(&self) {} + }; + let d = parse_method(&method).unwrap(); + assert!(d.feeds.is_none()); + assert!(d.expose.is_none()); + assert!(d.emits.is_none()); + assert!(!d.no_event); + } - // Look through all tokens for "emits" - while let Some(token) = iter.next() { - let proc_macro2::TokenTree::Ident(ident) = token else { - continue; - }; + #[test] + fn parses_feeds() { + let method: ImplItemFn = parse_quote! { + #[contract(feeds = "MyType")] + fn stream(&self) {} + }; + let d = parse_method(&method).unwrap(); + let feeds = d.feeds.expect("feeds field set"); + assert_eq!(feeds.to_string().replace(' ', ""), "MyType"); + } - if ident != "emits" { - continue; - } + #[test] + fn parses_feeds_generic_type() { + let method: ImplItemFn = parse_quote! { + #[contract(feeds = "Vec")] + fn stream(&self) {} + }; + let d = parse_method(&method).unwrap(); + let feeds = d.feeds.expect("feeds field set"); + assert_eq!(feeds.to_string().replace(' ', ""), "Vec"); + } - // Expect "=" - let Some(proc_macro2::TokenTree::Punct(punct)) = iter.next() else { - continue; - }; - if punct.as_char() != '=' { - continue; - } + #[test] + fn parses_expose() { + let impl_block: ItemImpl = parse_quote! { + #[contract(expose = [owner, transfer_ownership])] + impl OwnableTrait for MyContract {} + }; + let d = parse_impl(&impl_block).unwrap(); + let expose = d.expose.expect("expose field set"); + assert_eq!( + expose, + vec!["owner".to_string(), "transfer_ownership".to_string()] + ); + } - // Expect "[...]" - let Some(proc_macro2::TokenTree::Group(group)) = iter.next() else { - continue; - }; - if group.delimiter() != proc_macro2::Delimiter::Bracket { - continue; - } + #[test] + fn parses_expose_single_method() { + let impl_block: ItemImpl = parse_quote! { + #[contract(expose = [version])] + impl ISemver for MyContract {} + }; + let d = parse_impl(&impl_block).unwrap(); + assert_eq!(d.expose.unwrap(), vec!["version".to_string()]); + } - // Parse the event tuples from the group - return Some(parse_emits_tuples(group.stream())); - } + #[test] + fn parses_emits_path_topic() { + let method: ImplItemFn = parse_quote! { + #[contract(emits = [(events::OwnershipTransferred::TOPIC, events::OwnershipTransferred)])] + fn transfer(&mut self) {} + }; + let d = parse_method(&method).unwrap(); + let emits = d.emits.expect("emits field set"); + assert_eq!(emits.len(), 1); + assert_eq!(emits[0].topic, "events::OwnershipTransferred::TOPIC"); + assert_eq!( + emits[0].data_type.to_string().replace(' ', ""), + "events::OwnershipTransferred" + ); } - None -} + #[test] + fn parses_emits_string_topic() { + let method: ImplItemFn = parse_quote! { + #[contract(emits = [("custom_topic", MyEvent)])] + fn transfer(&mut self) {} + }; + let d = parse_method(&method).unwrap(); + let emits = d.emits.expect("emits field set"); + assert_eq!(emits.len(), 1); + assert_eq!(emits[0].topic, "custom_topic"); + } -/// Parse the contents of `emits = [...]` into a list of (topic, `data_type`) -/// pairs. -fn parse_emits_tuples(stream: proc_macro2::TokenStream) -> Vec<(String, TokenStream2)> { - let mut events = Vec::new(); + #[test] + fn parses_emits_multiple() { + let method: ImplItemFn = parse_quote! { + #[contract(emits = [ + (events::A::TOPIC, events::A), + ("b_topic", B), + ])] + fn act(&mut self) {} + }; + let d = parse_method(&method).unwrap(); + let emits = d.emits.unwrap(); + assert_eq!(emits.len(), 2); + assert_eq!(emits[0].topic, "events::A::TOPIC"); + assert_eq!(emits[1].topic, "b_topic"); + } - for token in stream { - // Each event is a group: (topic, Type) - let proc_macro2::TokenTree::Group(group) = token else { - continue; + #[test] + fn parses_no_event() { + let method: ImplItemFn = parse_quote! { + #[contract(no_event)] + fn touch(&mut self) {} }; - if group.delimiter() != proc_macro2::Delimiter::Parenthesis { - continue; - } + let d = parse_method(&method).unwrap(); + assert!(d.no_event); + } - if let Some((topic, data_type)) = parse_event_tuple(group.stream()) { - events.push((topic, data_type)); - } + #[test] + fn parses_combined_directives() { + let method: ImplItemFn = parse_quote! { + #[contract(feeds = "Stream", emits = [("t", E)], no_event)] + fn act(&mut self) {} + }; + let d = parse_method(&method).unwrap(); + assert!(d.feeds.is_some()); + assert_eq!(d.emits.unwrap().len(), 1); + assert!(d.no_event); } - events -} + #[test] + fn parses_feeds_in_any_position() { + // `feeds` must be recognised regardless of position within the + // directive list. + let method: ImplItemFn = parse_quote! { + #[contract(no_event, feeds = "T")] + fn act(&mut self) {} + }; + let d = parse_method(&method).unwrap(); + assert!(d.no_event); + assert!(d.feeds.is_some()); + } -/// Parse a single event tuple: (topic, Type). -fn parse_event_tuple(stream: proc_macro2::TokenStream) -> Option<(String, TokenStream2)> { - let mut iter = stream.into_iter().peekable(); + #[test] + fn feeds_value_containing_no_event_does_not_suppress() { + // `no_event` is only set when it appears as a directive keyword, + // never when the literal text "no_event" appears inside another + // directive's value. + let method: ImplItemFn = parse_quote! { + #[contract(feeds = "Foo_no_event")] + fn act(&self) {} + }; + let d = parse_method(&method).unwrap(); + assert!(!d.no_event); + assert!(d.feeds.is_some()); + } - // Extract topic (everything before the comma) - let topic = extract_topic_from_tokens(&mut iter)?; + #[test] + fn err_unknown_directive() { + let method: ImplItemFn = parse_quote! { + #[contract(no_events)] + fn act(&mut self) {} + }; + let err = expect_err(parse_method(&method)); + let msg = err.to_string(); + assert!(msg.contains("unknown contract directive"), "got: {msg}"); + assert!(msg.contains("no_events"), "got: {msg}"); + } - // Skip the comma - while let Some(token) = iter.peek() { - if let proc_macro2::TokenTree::Punct(p) = token - && p.as_char() == ',' - { - iter.next(); - break; - } - iter.next(); + #[test] + fn err_unknown_directive_lists_valid_keywords() { + let method: ImplItemFn = parse_quote! { + #[contract(bogus = 1)] + fn act(&self) {} + }; + let msg = expect_err(parse_method(&method)).to_string(); + assert!(msg.contains("feeds"), "got: {msg}"); + assert!(msg.contains("expose"), "got: {msg}"); + assert!(msg.contains("emits"), "got: {msg}"); + assert!(msg.contains("no_event"), "got: {msg}"); } - // Remaining tokens are the data type - let data_type: TokenStream2 = iter.collect(); - if data_type.is_empty() { - return None; + #[test] + fn err_unknown_directive_suggests_emits_for_emit() { + let method: ImplItemFn = parse_quote! { + #[contract(emit = [("t", E)])] + fn act(&mut self) {} + }; + let msg = expect_err(parse_method(&method)).to_string(); + assert!(msg.contains("did you mean"), "got: {msg}"); + assert!(msg.contains("emits"), "got: {msg}"); } - Some((topic, data_type)) -} + #[test] + fn err_feeds_without_value() { + let method: ImplItemFn = parse_quote! { + #[contract(feeds)] + fn act(&self) {} + }; + let msg = expect_err(parse_method(&method)).to_string(); + assert!(msg.contains("feeds"), "got: {msg}"); + assert!(msg.contains("TypeName"), "got: {msg}"); + } -/// Extract the topic string from the tokens before the comma. -fn extract_topic_from_tokens( - iter: &mut std::iter::Peekable, -) -> Option { - let mut path_segments = Vec::new(); - - while let Some(token) = iter.peek() { - match token { - proc_macro2::TokenTree::Punct(p) if p.as_char() == ',' => { - // End of topic - break; - } - proc_macro2::TokenTree::Punct(p) if p.as_char() == ':' => { - // Part of path separator :: - iter.next(); - } - proc_macro2::TokenTree::Ident(ident) => { - path_segments.push(ident.to_string()); - iter.next(); - } - proc_macro2::TokenTree::Literal(lit) => { - // String literal topic - let s = lit.to_string(); - iter.next(); - // Remove quotes from string literal - if s.starts_with('"') && s.ends_with('"') { - return Some(s[1..s.len() - 1].to_string()); - } - return Some(s); - } - _ => { - iter.next(); - } - } + #[test] + fn err_feeds_non_string_value() { + let method: ImplItemFn = parse_quote! { + #[contract(feeds = SomeType)] + fn act(&self) {} + }; + let msg = expect_err(parse_method(&method)).to_string(); + assert!(msg.contains("feeds"), "got: {msg}"); + assert!(msg.contains("TypeName"), "got: {msg}"); } - if path_segments.is_empty() { - None - } else { - Some(path_segments.join("::")) + #[test] + fn err_feeds_unparseable_type() { + let method: ImplItemFn = parse_quote! { + #[contract(feeds = "fn(")] + fn act(&self) {} + }; + let msg = expect_err(parse_method(&method)).to_string(); + assert!( + msg.contains("not a valid Rust type"), + "expected type-validation error, got: {msg}" + ); } -} -#[cfg(test)] -mod tests { - use syn::ItemImpl; + #[test] + fn err_expose_with_parens() { + let impl_block: ItemImpl = parse_quote! { + #[contract(expose = (owner, version))] + impl Foo for MyContract {} + }; + let msg = expect_err(parse_impl(&impl_block)).to_string(); + assert!(msg.contains("expose"), "got: {msg}"); + assert!(msg.contains("[m1"), "got: {msg}"); + } - use super::*; + #[test] + fn err_emits_malformed_tuple() { + let method: ImplItemFn = parse_quote! { + #[contract(emits = [(BadShape)])] + fn act(&mut self) {} + }; + // Missing comma inside the tuple — parser should error on the + // expected `,` in the tuple. + let err = expect_err(parse_method(&method)); + // We only require some error; the specific message comes from + // syn's tuple parsing. Sanity-check it's not silently dropped. + let msg = err.to_string(); + assert!(!msg.is_empty(), "expected error for malformed tuple"); + } #[test] - fn test_expose_list_simple() { - let impl_block: ItemImpl = syn::parse_quote! { - #[contract(expose = [owner, transfer_ownership])] - impl OwnableTrait for MyContract { - fn owner(&self) -> Address { self.owner } - } + fn err_emits_tuple_extra_tokens() { + // The tuple is `(topic, EventType)` — anything trailing must be + // rejected, not silently dropped. + let method: ImplItemFn = parse_quote! { + #[contract(emits = [("topic", EventType, Extra)])] + fn act(&mut self) {} }; - let expose_list = expose_list(&impl_block.attrs); - assert!(expose_list.is_some()); - let list = expose_list.unwrap(); - assert_eq!(list.len(), 2); - assert!(list.contains(&"owner".to_string())); - assert!(list.contains(&"transfer_ownership".to_string())); + let msg = expect_err(parse_method(&method)).to_string(); + assert!( + msg.contains("topic, EventType"), + "error should describe the expected tuple shape, got: {msg}" + ); } #[test] - fn test_expose_list_single() { - let impl_block: ItemImpl = syn::parse_quote! { - #[contract(expose = [version])] - impl ISemver for MyContract {} + fn err_duplicate_feeds_within_one_attribute() { + let method: ImplItemFn = parse_quote! { + #[contract(feeds = "A", feeds = "B")] + fn act(&self) {} }; - let expose_list = expose_list(&impl_block.attrs); - assert!(expose_list.is_some()); - let list = expose_list.unwrap(); - assert_eq!(list.len(), 1); - assert_eq!(list[0], "version"); + let msg = expect_err(parse_method(&method)).to_string(); + assert!(msg.contains("duplicate"), "got: {msg}"); + assert!(msg.contains("feeds"), "got: {msg}"); } #[test] - fn test_expose_list_none() { - let impl_block: ItemImpl = syn::parse_quote! { - impl OwnableTrait for MyContract { - fn owner(&self) -> Address { self.owner } - } + fn err_duplicate_emits_across_attributes() { + // Duplicates across `#[contract(...)]` attributes must error too — + // otherwise the first list is silently dropped. + let method: ImplItemFn = parse_quote! { + #[contract(emits = [("a", A)])] + #[contract(emits = [("b", B)])] + fn act(&mut self) {} }; - let expose_list = expose_list(&impl_block.attrs); - assert!(expose_list.is_none()); + let msg = expect_err(parse_method(&method)).to_string(); + assert!(msg.contains("duplicate"), "got: {msg}"); + assert!(msg.contains("emits"), "got: {msg}"); } #[test] - fn test_expose_list_other_attribute() { - let impl_block: ItemImpl = syn::parse_quote! { - #[derive(Debug)] - impl OwnableTrait for MyContract { - fn owner(&self) -> Address { self.owner } - } + fn err_duplicate_no_event() { + let method: ImplItemFn = parse_quote! { + #[contract(no_event, no_event)] + fn act(&mut self) {} }; - let expose_list = expose_list(&impl_block.attrs); - assert!(expose_list.is_none()); + let msg = expect_err(parse_method(&method)).to_string(); + assert!(msg.contains("duplicate"), "got: {msg}"); + assert!(msg.contains("no_event"), "got: {msg}"); } } diff --git a/contract-macro/src/parse/events.rs b/contract-macro/src/parse/events.rs index 5e4a629..6c0d1fe 100644 --- a/contract-macro/src/parse/events.rs +++ b/contract-macro/src/parse/events.rs @@ -262,21 +262,16 @@ pub(super) fn method_has_emit_call(method: &ImplItemFn) -> bool { /// Extract events from a method's `#[contract(emits = [...])]` attribute. /// /// Returns the events registered on this specific method, or an empty vec if -/// none. -pub(super) fn method_emits(attrs: &[Attribute]) -> Vec { - directives::emits_list(attrs) - .map(|events| { - events - .into_iter() - .map(|(topic, data_type)| EventInfo { topic, data_type }) - .collect() - }) - .unwrap_or_default() +/// none. Errors if a `#[contract(...)]` attribute on the method is malformed. +pub(super) fn method_emits(attrs: &[Attribute]) -> Result, syn::Error> { + Ok(directives::parse_contract_directives(attrs)? + .emits + .unwrap_or_default()) } /// Collect events from method-level `#[contract(emits = [...])]` attributes /// on the methods of an impl block, restricted to those matching `include`. -fn impl_method_emits(impl_block: &ItemImpl, mut include: F) -> Vec +fn impl_method_emits(impl_block: &ItemImpl, mut include: F) -> Result, syn::Error> where F: FnMut(&ImplItemFn) -> bool, { @@ -285,17 +280,17 @@ where if let ImplItem::Fn(method) = item && include(method) { - events.extend(method_emits(&method.attrs)); + events.extend(method_emits(&method.attrs)?); } } - events + Ok(events) } /// Extract events from method-level `#[contract(emits = [...])]` attributes in /// a trait impl. /// /// Only methods in the `expose_list` are checked for emits attributes. -pub(crate) fn trait_method_emits(trait_impl: &TraitImplInfo) -> Vec { +pub(crate) fn trait_method_emits(trait_impl: &TraitImplInfo) -> Result, syn::Error> { impl_method_emits(trait_impl.impl_block, |method| { trait_impl .expose_list @@ -309,7 +304,7 @@ pub(crate) fn trait_method_emits(trait_impl: &TraitImplInfo) -> Vec { /// Only public methods (excluding `new`) are checked, matching the set of /// methods exposed as contract functions by /// [`super::functions::public_methods`]. -pub(crate) fn inherent_method_emits(impl_block: &ItemImpl) -> Vec { +pub(crate) fn inherent_method_emits(impl_block: &ItemImpl) -> Result, syn::Error> { impl_method_emits(impl_block, |method| { matches!(method.vis, Visibility::Public(_)) && method.sig.ident != "new" }) @@ -556,7 +551,7 @@ mod tests { } }; - let collected = inherent_method_emits(&impl_block); + let collected = inherent_method_emits(&impl_block).unwrap(); assert_eq!( collected.len(), 2, @@ -698,7 +693,7 @@ mod tests { impl_block: &impl_block, expose_list: vec!["transfer_ownership".to_string()], }; - let events = trait_method_emits(&trait_impl); + let events = trait_method_emits(&trait_impl).unwrap(); assert_eq!(events.len(), 1); assert_eq!(events[0].topic, "Transferred::TOPIC"); } @@ -719,7 +714,7 @@ mod tests { pub fn new() -> Self { Self } } }; - let events = inherent_method_emits(&impl_block); + let events = inherent_method_emits(&impl_block).unwrap(); assert_eq!(events.len(), 1); assert_eq!(events[0].topic, "Resolved::TOPIC"); } diff --git a/contract-macro/src/parse/functions.rs b/contract-macro/src/parse/functions.rs index 49f22a9..5212752 100644 --- a/contract-macro/src/parse/functions.rs +++ b/contract-macro/src/parse/functions.rs @@ -141,13 +141,15 @@ pub(crate) fn trait_methods(trait_impl: &TraitImplInfo) -> Result Result Result, let name = method.sig.ident.clone(); let doc = extract_doc_comment(&method.attrs); - let feed_type = directives::extract_feeds_attribute(&method.attrs); + let method_directives = directives::parse_contract_directives(&method.attrs)?; + let feed_type = method_directives.feeds.clone(); + let suppressed = method_directives.no_event; + let has_method_emits = method_directives + .emits + .as_ref() + .is_some_and(|events| !events.is_empty()); let receiver = extract_receiver(method); let has_emit_call = events::method_has_emit_call(method); - let suppressed = directives::event_suppressed(&method.attrs); - let has_method_emits = !events::method_emits(&method.attrs).is_empty(); // Validate feed-related attributes validate_feeds(method, &name, feed_type.as_ref())?; diff --git a/contract-macro/src/parse/mod.rs b/contract-macro/src/parse/mod.rs index f5a633a..e0a8919 100644 --- a/contract-macro/src/parse/mod.rs +++ b/contract-macro/src/parse/mod.rs @@ -59,7 +59,7 @@ pub(crate) fn contract_data<'a>( validate::new_constructor(&name, &impl_blocks, struct_)?; validate::init_method(&name, &impl_blocks)?; - let trait_impls = module::trait_impls(items, &name); + let trait_impls = module::trait_impls(items, &name)?; Ok(ContractData { imports, diff --git a/contract-macro/src/parse/module.rs b/contract-macro/src/parse/module.rs index 79eda45..5bb8d43 100644 --- a/contract-macro/src/parse/module.rs +++ b/contract-macro/src/parse/module.rs @@ -117,33 +117,36 @@ pub(super) fn impl_blocks<'a>(items: &'a [Item], contract_name: &str) -> Vec<&'a /// /// Only trait implementations that have an explicit expose list are returned. /// The expose list specifies which trait methods should have extern wrappers -/// generated. -pub(super) fn trait_impls<'a>(items: &'a [Item], contract_name: &str) -> Vec> { - items - .iter() - .filter_map(|item| { - if let Item::Impl(impl_block) = item - && let Some((_, trait_path, _)) = &impl_block.trait_ - && let Type::Path(type_path) = &*impl_block.self_ty - && type_path.path.is_ident(contract_name) - && let Some(list) = directives::expose_list(&impl_block.attrs) - { +/// generated. Returns an error if a `#[contract(...)]` attribute on a trait +/// impl is malformed. +pub(super) fn trait_impls<'a>( + items: &'a [Item], + contract_name: &str, +) -> Result>, syn::Error> { + let mut result = Vec::new(); + for item in items { + if let Item::Impl(impl_block) = item + && let Some((_, trait_path, _)) = &impl_block.trait_ + && let Type::Path(type_path) = &*impl_block.self_ty + && type_path.path.is_ident(contract_name) + { + let directives = directives::parse_contract_directives(&impl_block.attrs)?; + if let Some(expose_list) = directives.expose { let trait_name = trait_path .segments .iter() .map(|s| s.ident.to_string()) .collect::>() .join("::"); - Some(TraitImplInfo { + result.push(TraitImplInfo { trait_name, impl_block, - expose_list: list, - }) - } else { - None + expose_list, + }); } - }) - .collect() + } + } + Ok(result) } #[cfg(test)] @@ -273,7 +276,7 @@ mod tests { } }]; - let trait_impls = trait_impls(&items, "MyContract"); + let trait_impls = trait_impls(&items, "MyContract").unwrap(); assert_eq!(trait_impls.len(), 1); assert_eq!(trait_impls[0].trait_name, "OwnableTrait"); assert_eq!(trait_impls[0].expose_list, vec!["owner"]); @@ -287,7 +290,7 @@ mod tests { } }]; - let trait_impls = trait_impls(&items, "MyContract"); + let trait_impls = trait_impls(&items, "MyContract").unwrap(); assert_eq!( trait_impls.len(), 0, @@ -312,7 +315,7 @@ mod tests { }, ]; - let trait_impls = trait_impls(&items, "MyContract"); + let trait_impls = trait_impls(&items, "MyContract").unwrap(); assert_eq!(trait_impls.len(), 2); } } From cbbb0d17cb215062ba393cc0ea6b9013d31507c5 Mon Sep 17 00:00:00 2001 From: moana Date: Mon, 4 May 2026 12:43:44 +0200 Subject: [PATCH 2/3] macro: Eliminate double-parse by returning events from method extractors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `functions::public_methods` and `functions::trait_methods` now each return `(Vec, Vec)` — the events vector is the union of `#[contract(emits = [...])]` registrations across the visited methods, harvested from the same `ContractDirectives` parse that the extractor already ran. This drops `events::method_emits`, `events::impl_method_emits`, `events::inherent_method_emits`, and `events::trait_method_emits` (each of which re-parsed the same method attrs to read just the `emits` field). `lib.rs` destructures the new tuple, and the dedup pipeline test in `events.rs` now seeds events through `functions::public_methods`. The migrated event-collection tests (one per visibility filter) live alongside `public_methods` / `trait_methods` in `functions.rs#tests`. --- contract-macro/src/lib.rs | 26 +++--- contract-macro/src/parse/directives.rs | 73 +++++++++++++--- contract-macro/src/parse/events.rs | 112 ++----------------------- contract-macro/src/parse/functions.rs | 106 +++++++++++++++++------ contract-macro/src/parse/mod.rs | 4 +- 5 files changed, 156 insertions(+), 165 deletions(-) diff --git a/contract-macro/src/lib.rs b/contract-macro/src/lib.rs index 50a0e5a..343820e 100644 --- a/contract-macro/src/lib.rs +++ b/contract-macro/src/lib.rs @@ -207,30 +207,24 @@ pub fn contract(_attr: TokenStream, item: TokenStream) -> TokenStream { let mut events = Vec::new(); for impl_block in &impl_blocks { - match parse::public_methods(impl_block) { - Ok(methods) => functions.extend(methods), + let (methods, method_events) = match parse::public_methods(impl_block) { + Ok(result) => result, Err(e) => return e.to_compile_error().into(), - } + }; + functions.extend(methods); events.extend(parse::emit_calls(impl_block)); - // Include events from method-level #[contract(emits = [...])] attributes - match parse::inherent_method_emits(impl_block) { - Ok(method_events) => events.extend(method_events), - Err(e) => return e.to_compile_error().into(), - } + events.extend(method_events); } // Extract functions and events from trait impl blocks with expose lists for trait_impl in &trait_impls { - match parse::trait_methods(trait_impl) { - Ok(trait_functions) => functions.extend(trait_functions), + let (trait_functions, method_events) = match parse::trait_methods(trait_impl) { + Ok(result) => result, Err(e) => return e.to_compile_error().into(), - } + }; + functions.extend(trait_functions); events.extend(parse::emit_calls(trait_impl.impl_block)); - // Include events from method-level #[contract(emits = [...])] attributes - match parse::trait_method_emits(trait_impl) { - Ok(method_events) => events.extend(method_events), - Err(e) => return e.to_compile_error().into(), - } + events.extend(method_events); } // Deduplicate events by topic — first-seen wins. diff --git a/contract-macro/src/parse/directives.rs b/contract-macro/src/parse/directives.rs index 33efc45..ea6ca13 100644 --- a/contract-macro/src/parse/directives.rs +++ b/contract-macro/src/parse/directives.rs @@ -394,17 +394,34 @@ mod tests { } #[test] - fn parses_combined_directives() { - let method: ImplItemFn = parse_quote! { - #[contract(feeds = "Stream", emits = [("t", E)], no_event)] - fn act(&mut self) {} + fn parses_all_four_directives_in_one_attribute() { + // The four supported directives can all coexist inside a single + // `#[contract(...)]` invocation. + let impl_block: ItemImpl = parse_quote! { + #[contract(feeds = "Stream", expose = [m], emits = [("t", E)], no_event)] + impl Trait for MyContract {} }; - let d = parse_method(&method).unwrap(); + let d = parse_impl(&impl_block).unwrap(); assert!(d.feeds.is_some()); + assert_eq!(d.expose.unwrap(), vec!["m".to_string()]); assert_eq!(d.emits.unwrap().len(), 1); assert!(d.no_event); } + #[test] + fn aggregates_directives_across_multiple_attributes() { + // Two `#[contract(...)]` attributes on the same item are folded + // together into a single `ContractDirectives`. + let impl_block: ItemImpl = parse_quote! { + #[contract(expose = [m])] + #[contract(emits = [("t", E)])] + impl Trait for MyContract {} + }; + let d = parse_impl(&impl_block).unwrap(); + assert_eq!(d.expose.unwrap(), vec!["m".to_string()]); + assert_eq!(d.emits.unwrap().len(), 1); + } + #[test] fn parses_feeds_in_any_position() { // `feeds` must be recognised regardless of position within the @@ -515,18 +532,48 @@ mod tests { } #[test] - fn err_emits_malformed_tuple() { + fn err_emits_tuple_missing_comma() { + // Single-element tuple — the parser walks past the topic, then fails + // when expecting `,` before the data type. let method: ImplItemFn = parse_quote! { #[contract(emits = [(BadShape)])] fn act(&mut self) {} }; - // Missing comma inside the tuple — parser should error on the - // expected `,` in the tuple. - let err = expect_err(parse_method(&method)); - // We only require some error; the specific message comes from - // syn's tuple parsing. Sanity-check it's not silently dropped. - let msg = err.to_string(); - assert!(!msg.is_empty(), "expected error for malformed tuple"); + let msg = expect_err(parse_method(&method)).to_string(); + assert!( + msg.contains(','), + "error should mention the missing `,`, got: {msg}" + ); + } + + #[test] + fn err_emits_tuple_not_parenthesized() { + // The list contents must be tuples; a bare ident fails the + // `parenthesized!` step inside `EventTuple::parse`. + let method: ImplItemFn = parse_quote! { + #[contract(emits = [BadShape])] + fn act(&mut self) {} + }; + let msg = expect_err(parse_method(&method)).to_string(); + assert!( + msg.contains("parenthes"), + "error should mention the missing parentheses, got: {msg}" + ); + } + + #[test] + fn err_emits_tuple_missing_data_type() { + // Trailing comma after the topic — the parser walks past the topic + // and the comma, then fails when expecting a type for `data_type`. + let method: ImplItemFn = parse_quote! { + #[contract(emits = [("topic",)])] + fn act(&mut self) {} + }; + let msg = expect_err(parse_method(&method)).to_string(); + assert!( + msg.contains("type") || msg.contains("expected"), + "error should describe the missing type, got: {msg}" + ); } #[test] diff --git a/contract-macro/src/parse/events.rs b/contract-macro/src/parse/events.rs index 6c0d1fe..08fc316 100644 --- a/contract-macro/src/parse/events.rs +++ b/contract-macro/src/parse/events.rs @@ -13,12 +13,9 @@ use std::collections::HashSet; use proc_macro2::TokenStream as TokenStream2; use quote::quote; use syn::visit::Visit; -use syn::{ - Attribute, Expr, ExprCall, ExprLit, ExprPath, ImplItem, ImplItemFn, ItemImpl, Lit, Visibility, -}; +use syn::{Expr, ExprCall, ExprLit, ExprPath, ImplItemFn, ItemImpl, Lit}; -use crate::parse::directives; -use crate::{EventInfo, TraitImplInfo}; +use crate::EventInfo; /// Visitor to find `abi::emit()` calls within function bodies. struct EmitVisitor { @@ -259,57 +256,6 @@ pub(super) fn method_has_emit_call(method: &ImplItemFn) -> bool { !visitor.events.is_empty() } -/// Extract events from a method's `#[contract(emits = [...])]` attribute. -/// -/// Returns the events registered on this specific method, or an empty vec if -/// none. Errors if a `#[contract(...)]` attribute on the method is malformed. -pub(super) fn method_emits(attrs: &[Attribute]) -> Result, syn::Error> { - Ok(directives::parse_contract_directives(attrs)? - .emits - .unwrap_or_default()) -} - -/// Collect events from method-level `#[contract(emits = [...])]` attributes -/// on the methods of an impl block, restricted to those matching `include`. -fn impl_method_emits(impl_block: &ItemImpl, mut include: F) -> Result, syn::Error> -where - F: FnMut(&ImplItemFn) -> bool, -{ - let mut events = Vec::new(); - for item in &impl_block.items { - if let ImplItem::Fn(method) = item - && include(method) - { - events.extend(method_emits(&method.attrs)?); - } - } - Ok(events) -} - -/// Extract events from method-level `#[contract(emits = [...])]` attributes in -/// a trait impl. -/// -/// Only methods in the `expose_list` are checked for emits attributes. -pub(crate) fn trait_method_emits(trait_impl: &TraitImplInfo) -> Result, syn::Error> { - impl_method_emits(trait_impl.impl_block, |method| { - trait_impl - .expose_list - .contains(&method.sig.ident.to_string()) - }) -} - -/// Extract events from method-level `#[contract(emits = [...])]` attributes in -/// an inherent impl block. -/// -/// Only public methods (excluding `new`) are checked, matching the set of -/// methods exposed as contract functions by -/// [`super::functions::public_methods`]. -pub(crate) fn inherent_method_emits(impl_block: &ItemImpl) -> Result, syn::Error> { - impl_method_emits(impl_block, |method| { - matches!(method.vis, Visibility::Public(_)) && method.sig.ident != "new" - }) -} - #[cfg(test)] mod tests { use super::*; @@ -539,8 +485,8 @@ mod tests { // End-to-end through the extract layer: build an impl block where two // public methods carry `#[contract(emits = [...])]` attributes that // share a topic but supply different data types. The macro pipeline - // (inherent_method_emits → dedup_events_by_topic) keeps the first - // occurrence and drops the rest. + // (public_methods → dedup_events_by_topic) keeps the first occurrence + // and drops the rest. let impl_block: ItemImpl = syn::parse_quote! { impl MyContract { #[contract(emits = [(SHARED::TOPIC, FirstEvent)])] @@ -551,7 +497,7 @@ mod tests { } }; - let collected = inherent_method_emits(&impl_block).unwrap(); + let (_methods, collected) = super::super::functions::public_methods(&impl_block).unwrap(); assert_eq!( collected.len(), 2, @@ -670,52 +616,4 @@ mod tests { let topics: Vec<_> = events.iter().map(|e| e.topic.as_str()).collect(); assert_eq!(topics, vec!["topic_a", "topic_b"]); } - - // ======================================================================== - // trait_method_emits / inherent_method_emits tests - // ======================================================================== - - #[test] - fn test_trait_method_emits_collects_events() { - let impl_block: ItemImpl = syn::parse_quote! { - #[contract(expose = [transfer_ownership])] - impl OwnableTrait for MyContract { - #[contract(emits = [(Transferred::TOPIC, Transferred)])] - fn transfer_ownership(&mut self) {} - - // Not in expose list — should be ignored even with emits. - #[contract(emits = [(Hidden::TOPIC, Hidden)])] - fn unexposed(&mut self) {} - } - }; - let trait_impl = TraitImplInfo { - trait_name: "OwnableTrait".to_string(), - impl_block: &impl_block, - expose_list: vec!["transfer_ownership".to_string()], - }; - let events = trait_method_emits(&trait_impl).unwrap(); - assert_eq!(events.len(), 1); - assert_eq!(events[0].topic, "Transferred::TOPIC"); - } - - #[test] - fn test_inherent_method_emits_collects_events() { - let impl_block: ItemImpl = syn::parse_quote! { - impl MyContract { - #[contract(emits = [(Resolved::TOPIC, Resolved)])] - pub fn resolve(&mut self) { self.core.resolve(); } - - // Private method — should be ignored. - #[contract(emits = [(Hidden::TOPIC, Hidden)])] - fn private_helper(&mut self) { self.core.hidden(); } - - // Constructor — should be ignored even if it carries emits. - #[contract(emits = [(New::TOPIC, New)])] - pub fn new() -> Self { Self } - } - }; - let events = inherent_method_emits(&impl_block).unwrap(); - assert_eq!(events.len(), 1); - assert_eq!(events[0].topic, "Resolved::TOPIC"); - } } diff --git a/contract-macro/src/parse/functions.rs b/contract-macro/src/parse/functions.rs index 5212752..591adc1 100644 --- a/contract-macro/src/parse/functions.rs +++ b/contract-macro/src/parse/functions.rs @@ -15,7 +15,7 @@ use syn::{ }; use crate::parse::{directives, events}; -use crate::{FunctionInfo, ParameterInfo, Receiver, TraitImplInfo, validate}; +use crate::{EventInfo, FunctionInfo, ParameterInfo, Receiver, TraitImplInfo, validate}; /// Check if a method body is empty (just `{}`). /// @@ -116,13 +116,18 @@ fn validate_feeds( Ok(()) } -/// Extract methods from a trait impl block based on the expose list. +/// Extract methods and method-level events from a trait impl block. /// -/// Only methods whose names appear in the `expose_list` will be extracted. +/// Only methods whose names appear in the `expose_list` are extracted. /// Methods with empty bodies `{}` are treated as "use default implementation" - -/// the macro will generate wrappers that call the trait method directly. -pub(crate) fn trait_methods(trait_impl: &TraitImplInfo) -> Result, syn::Error> { +/// the macro will generate wrappers that call the trait method directly. The +/// returned event vector is the union of `#[contract(emits = [...])]` +/// registrations across the exposed methods. +pub(crate) fn trait_methods( + trait_impl: &TraitImplInfo, +) -> Result<(Vec, Vec), syn::Error> { let mut functions = Vec::new(); + let mut method_events = Vec::new(); for item in &trait_impl.impl_block.items { if let ImplItem::Fn(method) = item { @@ -144,10 +149,8 @@ pub(crate) fn trait_methods(trait_impl: &TraitImplInfo) -> Result Result Result Result, syn::Error> { +/// `#[contract(feeds = "Type")]` attribute. The returned event vector is the +/// union of `#[contract(emits = [...])]` registrations across the public +/// methods. +pub(crate) fn public_methods( + impl_block: &ItemImpl, +) -> Result<(Vec, Vec), syn::Error> { let mut functions = Vec::new(); + let mut method_events = Vec::new(); for item in &impl_block.items { if let ImplItem::Fn(method) = item { @@ -242,10 +251,8 @@ pub(crate) fn public_methods(impl_block: &ItemImpl) -> Result, let method_directives = directives::parse_contract_directives(&method.attrs)?; let feed_type = method_directives.feeds.clone(); let suppressed = method_directives.no_event; - let has_method_emits = method_directives - .emits - .as_ref() - .is_some_and(|events| !events.is_empty()); + let emits = method_directives.emits.unwrap_or_default(); + let has_method_emits = !emits.is_empty(); let receiver = extract_receiver(method); let has_emit_call = events::method_has_emit_call(method); @@ -275,10 +282,11 @@ pub(crate) fn public_methods(impl_block: &ItemImpl) -> Result, trait_name: None, // Not a trait method feed_type, }); + method_events.extend(emits); } } - Ok(functions) + Ok((functions, method_events)) } /// Extract parameter names and types from a method (excluding self). @@ -446,11 +454,10 @@ mod tests { impl_block: &impl_block, expose_list: vec!["owner".to_string()], }; - let result = trait_methods(&trait_impl); - assert!(result.is_ok()); - let functions = result.unwrap(); + let (functions, events) = trait_methods(&trait_impl).unwrap(); assert_eq!(functions.len(), 1); assert_eq!(functions[0].name.to_string(), "owner"); + assert!(events.is_empty()); } #[test] @@ -470,9 +477,7 @@ mod tests { impl_block: &impl_block, expose_list: vec!["owner".to_string(), "transfer_ownership".to_string()], }; - let result = trait_methods(&trait_impl); - assert!(result.is_ok()); - let functions = result.unwrap(); + let (functions, _events) = trait_methods(&trait_impl).unwrap(); assert_eq!(functions.len(), 2); } @@ -488,8 +493,8 @@ mod tests { } } }; - let functions = match public_methods(&impl_block) { - Ok(functions) => functions, + let (functions, _events) = match public_methods(&impl_block) { + Ok(result) => result, Err(err) => panic!("expected success, got: {err}"), }; assert_eq!(functions.len(), 1); @@ -534,6 +539,55 @@ mod tests { assert!(err.to_string().contains("not found")); } + #[test] + fn test_trait_methods_collects_method_events() { + // Methods in `expose_list` contribute their `#[contract(emits = ...)]` + // entries to the returned events vector; methods outside the list are + // skipped even if they carry emits. + let impl_block: ItemImpl = syn::parse_quote! { + #[contract(expose = [transfer_ownership])] + impl OwnableTrait for MyContract { + #[contract(emits = [(Transferred::TOPIC, Transferred)])] + fn transfer_ownership(&mut self) {} + + // Not in expose list — should be ignored even with emits. + #[contract(emits = [(Hidden::TOPIC, Hidden)])] + fn unexposed(&mut self) {} + } + }; + let trait_impl = TraitImplInfo { + trait_name: "OwnableTrait".to_string(), + impl_block: &impl_block, + expose_list: vec!["transfer_ownership".to_string()], + }; + let (_functions, events) = trait_methods(&trait_impl).unwrap(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].topic, "Transferred::TOPIC"); + } + + #[test] + fn test_public_methods_collects_method_events() { + // Public non-`new` methods contribute their `#[contract(emits = ...)]` + // entries; private methods and `new` are skipped even with emits. + let impl_block: ItemImpl = syn::parse_quote! { + impl MyContract { + #[contract(emits = [(Resolved::TOPIC, Resolved)])] + pub fn resolve(&mut self) { self.core.resolve(); } + + // Private method — should be ignored. + #[contract(emits = [(Hidden::TOPIC, Hidden)])] + fn private_helper(&mut self) { self.core.hidden(); } + + // Constructor — should be ignored even if it carries emits. + #[contract(emits = [(New::TOPIC, New)])] + pub fn new() -> Self { Self } + } + }; + let (_functions, events) = public_methods(&impl_block).unwrap(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].topic, "Resolved::TOPIC"); + } + // ======================================================================== // Feed validation tests // ======================================================================== diff --git a/contract-macro/src/parse/mod.rs b/contract-macro/src/parse/mod.rs index e0a8919..19466ac 100644 --- a/contract-macro/src/parse/mod.rs +++ b/contract-macro/src/parse/mod.rs @@ -25,9 +25,7 @@ mod functions; mod imports; mod module; -pub(crate) use events::{ - dedup_events_by_topic, emit_calls, inherent_method_emits, trait_method_emits, -}; +pub(crate) use events::{dedup_events_by_topic, emit_calls}; pub(crate) use functions::{public_methods, trait_methods}; use syn::{Item, ItemMod}; From 4759c5c09f6385e36c2ce5a50f5c39eccf192136 Mon Sep 17 00:00:00 2001 From: moana Date: Mon, 4 May 2026 12:21:08 +0200 Subject: [PATCH 3/3] chore: Add changelog entry for unified directive parser --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a11119..4686dc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Replace the four hand-rolled `#[contract(...)]` directive parsers with a single typed pass returning a `ContractDirectives` struct. Malformed inputs (typo'd keyword, wrong shape, unparseable feeds type, `expose = (...)` with parens, extra tokens after a `(topic, EventType)` tuple) now produce a `compile_error!` instead of being silently dropped. Duplicate directives (within one attribute or across multiple `#[contract(...)]` attributes on the same item) also error [#24]. - Move workspace to Rust edition 2024 on the stable toolchain (MSRV 1.85). Generated contract wrappers now use `#[unsafe(no_mangle)]`. - Remove `-Z build-std=core,alloc` from contract builds (no longer needed on stable). - Replace EVM-flavored test-bridge with a general-purpose test contract that exercises every `#[contract]` macro code path without domain-specific types. @@ -83,6 +84,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#3]: https://github.com/dusk-network/forge/issues/3 [#6]: https://github.com/dusk-network/forge/issues/6 +[#24]: https://github.com/dusk-network/forge/issues/24 [Unreleased]: https://github.com/dusk-network/forge/compare/v0.2.1...HEAD