Skip to content

fix(json-abi): handle globals and propagate attrs to globals#1047

Open
voith wants to merge 8 commits intoalloy-rs:mainfrom
voith:fix/global-variables
Open

fix(json-abi): handle globals and propagate attrs to globals#1047
voith wants to merge 8 commits intoalloy-rs:mainfrom
voith:fix/global-variables

Conversation

@voith
Copy link

@voith voith commented Dec 12, 2025

Motivation

fixes #966

JSON to Solidity generation didn’t account for globals, so globals were mistakenly emitted inside the interface. That broke name resolution during expansion. For example:

Original:

struct Foo { uint256 number; }
interface IHandler {
    struct FooBar { Foo foo; }
}
contract Handler is IHandler {
    function handle(FooBar calldata foobar) external {}
}

json-to-sol produced:

interface IHandler {
    struct FooBar { Foo foo; }
}
contract Handler is IHandler {
    struct Foo { uint256 number; } // wrongly moved inside
    function handle(FooBar calldata foobar) external {}
}

Solution

  • ABI internalTypes without a namespace are now treated as globals (e.g., "internalType": "struct GlobalStruct" goes global), while namespaced types (e.g., struct Interface.InterfaceStruct) stay under their interface/library. Only structs and UDVTs become globals; enums are flattened to UDVTs and also global. Functions/errors/events can’t be reliably identified as global from the ABI, so they remain under the interface as before. This behavior matches what I verified against gnidan/abi-to-sol.

  • Printer indentation now respects globals at the root, using an indentation_level setting instead of flags to control formatting.

  • Attribute propagation now parses the tokenstream into an ast and updates attrs there (globals included), rather than re-splitting token streams. This keeps the parsing logic in syn-solidity and avoids duplicating it. Basic benchmarks showed no significant slowdown; happy to benchmark further if maintainers have concerns.

  • I've split the PR into 3 commits for ease of reviewing:
    b52de13 — handle globals separately when emitting JSON ABI to Solidity.
    1ff6a56 — improve JsonAbi printer indentation handling for globals and nested items.
    9e1c6c6 — propagate sol! derive attrs to non-interface JSON ABI items.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@voith voith force-pushed the fix/global-variables branch from 9e1c6c6 to 00b15c3 Compare December 12, 2025 18:05
@voith voith changed the title Fix Json-To-Sol: emit globals at root and propagate attrs to globals fix(json-abi): handle globals and propagate attrs to globals Dec 12, 2025
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

makes sense, but it is a breaking change. i haven't looked in a while but I believe the reason we emitted 'global' types inside of the main contract is because of legacy / creating a single module with everything inside

@voith voith force-pushed the fix/global-variables branch from d5db4d2 to 02a9723 Compare December 16, 2025 11:53
@voith
Copy link
Author

voith commented Dec 16, 2025

makes sense, but it is a breaking change. i haven't looked in a while but I believe the reason we emitted 'global' types inside of the main contract is because of legacy / creating a single module with everything inside

Thanks, I agree this is a breaking change. I think the previous output was semantically incorrect for globals, but I get the legacy single-module goal.

How would you like to proceed?
Options:

  • Keep the fix (globals at top level) and treat it as a breaking change for the next release.
  • if we need compatibility in the current major, we could add #[sol(emit_globals = false)]. I’d prefer keeping the fixed behavior(emit_globals = true) as default and documenting it as a breaking change for the next release.

Which direction do you prefer?

@DaniPopes
Copy link
Member

yeah let's go with the attribute so we can keep the default behavior the same

@voith
Copy link
Author

voith commented Dec 17, 2025

let's go with the attribute so we can keep the default behavior the same

Done. Went with the name #[sol(standalone_globals)] as it makes the intent (“put globals at the top level”) explicit.

By default standalone_globals=false, so this is no longer a breaking change.

/// attributes cloned onto them.
/// - The single interface gets the outer attributes, a generated doc (including the original
/// Solidity/JSON ABI), and the `#[sol(bytecode = ..., deployed_bytecode = ...)]` attribute.
fn apply_attrs_to_items(items: &mut [ast::Item], ctx: &ApplyAttrsCtx<'_>) {
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Author

@voith voith Dec 18, 2025

Choose a reason for hiding this comment

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

The previous quote!-based attr propagation worked when to_sol only emitted a library + interface. Once globals were added, splitting the token stream into the right pieces got messy and brittle. Here’s a diff of what the token‑splitting approach would look like:

diff --git a/crates/sol-macro-input/src/json.rs b/crates/sol-macro-input/src/json.rs
index 13906b0..2ea0ff2 100644
--- a/crates/sol-macro-input/src/json.rs
+++ b/crates/sol-macro-input/src/json.rs
@@
-use crate::{SolInput, SolInputKind};
+use crate::{SolAttrs, SolInput, SolInputKind};
 use alloy_json_abi::{ContractObject, JsonAbi, ToSolConfig};
-use proc_macro2::{Ident, TokenStream, TokenTree};
+use proc_macro2::{Delimiter, Ident, TokenStream, TokenTree};
 use quote::quote;
 use syn::{AttrStyle, Result};
@@
-        let mut all_tokens = tokens_for_sol(&name, &sol)?.into_iter();
+        let tokens: Vec<_> = tokens_for_sol(&name, &sol)?.into_iter().collect();
@@
-        let mut library_tokens_iter = all_tokens
-            .by_ref()
-            .take_while(|tt| !matches!(tt, TokenTree::Ident(id) if id == "interface"))
-            .skip_while(|tt| matches!(tt, TokenTree::Ident(id) if id == "library"))
-            .peekable();
-
-        let library_tokens = library_tokens_iter.by_ref();
-
-        let mut libraries = Vec::new();
-
-        while library_tokens.peek().is_some() {
-            let sol_library_tokens: TokenStream = std::iter::once(TokenTree::Ident(id("library")))
-                .chain(
-                    library_tokens
-                        .take_while(|tt| !matches!(tt, TokenTree::Ident(id) if id == "library")),
-                )
-                .collect();
-
-            let tokens = quote! {
-                #(#derives)*
-                #(#sol_derives)*
-                #sol_library_tokens
-            };
-
-            libraries.push(tokens);
-        }
-        let sol_interface_tokens: TokenStream =
-            std::iter::once(TokenTree::Ident(id("interface"))).chain(all_tokens).collect();
+        let (libraries, sol_interface_tokens, globals) =
+            split_libraries_interface_globals(tokens, &derives, &sol_derives);
@@
             #[doc = #doc_str]
             #[sol(#bytecode #deployed_bytecode)]
             #sol_interface_tokens
+
+            #globals
         };
@@
+fn split_libraries_interface_globals(
+    tokens: Vec<TokenTree>,
+    derives: &[&syn::Attribute],
+    sol_derives: &[&syn::Attribute],
+) -> (Vec<TokenStream>, TokenStream, TokenStream) {
+    let mut libraries = Vec::new();
+    let mut idx = 0;
+    let mut interface_start = 0;
+
+    while idx < tokens.len() {
+        match &tokens[idx] {
+            TokenTree::Ident(id) if id == "library" => {
+                let start = idx;
+                idx += 1;
+                while idx < tokens.len()
+                    && !matches!(&tokens[idx], TokenTree::Ident(id) if id == "library" || id == "interface")
+                {
+                    idx += 1;
+                }
+                let lib_ts: TokenStream = tokens[start..idx].iter().cloned().collect();
+                let ts = quote! {
+                    #(#derives)*
+                    #(#sol_derives)*
+                    #lib_ts
+                };
+                libraries.push(ts);
+            }
+            TokenTree::Ident(id) if id == "interface" => {
+                interface_start = idx;
+                break;
+            }
+            _ => idx += 1,
+        }
+    }
+
+    let mut interface = TokenStream::new();
+    let mut globals = TokenStream::new();
+    let mut j = interface_start;
+    while j < tokens.len() {
+        let tt = tokens[j].clone();
+        interface.extend(Some(tt.clone()));
+        if matches!(&tt, TokenTree::Group(g) if g.delimiter() == Delimiter::Brace) {
+            j += 1;
+            break;
+        }
+        j += 1;
+    }
+    globals.extend(tokens[j..].iter().cloned());
+
+    let globals = prepend_attrs_to_globals(globals, derives, sol_derives);
+    (libraries, interface, globals)
+}
+
+fn prepend_attrs_to_globals(
+    globals: TokenStream,
+    derives: &[&syn::Attribute],
+    sol_derives: &[&syn::Attribute],
+) -> TokenStream {
+    let attrs = quote! { #(#derives)* #(#sol_derives)* };
+    let mut out = TokenStream::new();
+    for tt in globals {
+        if matches!(&tt, TokenTree::Ident(id) if id == "struct" || id == "type") {
+            out.extend(attrs.clone());
+        }
+        out.extend(Some(tt));
+    }
+    out
+}

split_libraries_interface_globals works, but it’s brittle:

  • It assumes one interface after any libraries and that to_sol orders things exactly that way; if to_sol changes (multiple interfaces, different ordering, different tokens), this splitter needs to be rewritten.
  • It matches bare library/interface idents at the top level. If those strings appear in other contexts, we’ll mis-split.
  • Globals are assumed to be whatever trails after the interface’s first brace group, and we only prepend derives to struct/type items, other global forms would be missed.

Approach taken in the PR:
Parse into ast::File and delegate all of that structure to syn-solidity, so we can reliably identify interface vs. non-interface items and clone derives/sol attrs onto the right nodes without ad‑hoc token surgery. It’s clearer, less error-prone, and won’t need constant updates if to_sol’s output shape evolves.

If you have a preferred direction or a cleaner way to do this, let me know.

@voith
Copy link
Author

voith commented Feb 16, 2026

@DaniPopes Did I answer your questions or do you need any other clarifications?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[Bug] sol! type not found with free-standing struct

2 participants