Skip to content

Commit a7efce5

Browse files
Fix custom attrs incorrectly applied to testutils fns (#1699)
### What Move attribute filtering before `testutils_only_code` generation in `derive_fn.rs` so that custom attribute macros on methods in `#[contractimpl]` blocks are not incorrectly applied to generated wrapper functions. ### Why When a method in a `#[contractimpl]` block has a custom attribute macro, the attribute was being: - Correctly filtered out of wasm-generated code - Incorrectly passed to testutils wrapper functions (like `__invoke_raw_slice`) This caused `cargo test` to fail while `cargo build --target wasm32v1-none` succeeded. For example, if an attribute macro injects code that references `Self`, it would fail when applied to the generated free functions where `Self` is not valid: ``` error[E0411]: cannot find type Self in this scope ``` This bug affects test-only code, and did not impact the code generation of code that builds to wasm. It looks like I introduced this bug in bf73a94 (#1691), which restructured the generated wrapper functions to be emitted as free functions instead of inside a nested module. As part of adding `#[doc(hidden)]` and `#[allow(non_snake_case)]` to the `testutils_only_code`, `#(#attrs)*` was also added to match the pattern used for the other generated functions. However, this was done without noticing that attribute filtering happened *later* in the function and so `testutils_only_code` received unfiltered attributes while the main code received filtered attributes: ```rust // testutils_only_code used UNFILTERED attrs let testutils_only_code = if cfg!(feature = "testutils") { Some(quote! { #(#attrs)* // <-- Added in bf73a94, but unfiltered! pub fn #invoke_raw_slice(...) { ... } }) }; // Filtering happened AFTER let attrs = attrs.iter() .filter(|attr| pass_through_attr_to_gen_code(attr)) .collect(); ``` The fix moves attribute filtering before testutils_only_code generation so both use consistently filtered attributes. This bug highlights a gap in the tests for macro composability, and so a test has been added to close that gap. Close #1698 ### Known limitations N/A
1 parent a3cefc1 commit a7efce5

File tree

4 files changed

+104
-7
lines changed

4 files changed

+104
-7
lines changed

soroban-sdk-macros/src/derive_fn.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,11 @@ pub fn derive_pub_fn(
169169
return Err(quote! { #(#compile_errors)* });
170170
}
171171

172+
let attrs: Vec<_> = attrs
173+
.iter()
174+
.filter(|attr| pass_through_attr_to_gen_code(attr))
175+
.collect();
176+
172177
let testutils_only_code = if cfg!(feature = "testutils") {
173178
Some(quote! {
174179
#[doc(hidden)]
@@ -190,12 +195,6 @@ pub fn derive_pub_fn(
190195
None
191196
};
192197

193-
// Filter attributes to those that should be passed through to the generated code.
194-
let attrs = attrs
195-
.iter()
196-
.filter(|attr| pass_through_attr_to_gen_code(attr))
197-
.collect::<Vec<_>>();
198-
199198
// Generated code.
200199
Ok(quote! {
201200
#[doc(hidden)]

tests/macros/proc_macros/src/lib.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,16 @@ pub fn parse_item_fn(_metadata: TokenStream, input: TokenStream) -> TokenStream
1717
let item = parse_macro_input!(input as ItemFn);
1818
quote! { #item }.into()
1919
}
20+
21+
/// An attribute macro that checks it is being used on a method of a type, not a free function.
22+
/// It does this by injecting code that references `Self`, which is only valid inside an impl block.
23+
#[proc_macro_attribute]
24+
pub fn check_fn_is_item_fn(_metadata: TokenStream, input: TokenStream) -> TokenStream {
25+
let mut item = parse_macro_input!(input as ItemFn);
26+
// Insert a statement that uses `Self` at the beginning of the function
27+
let check_stmt: syn::Stmt = syn::parse_quote! {
28+
let _ = core::any::type_name::<Self>();
29+
};
30+
item.block.stmts.insert(0, check_stmt);
31+
quote! { #item }.into()
32+
}

tests/macros/src/lib.rs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// validating that they are composable and compatible.
33

44
#![no_std]
5-
use proc_macros::{parse_item_fn, parse_item_impl};
5+
use proc_macros::{check_fn_is_item_fn, parse_item_fn, parse_item_impl};
66
use soroban_sdk::{contract, contractimpl};
77

88
#[contract]
@@ -14,6 +14,10 @@ impl Contract {
1414
// Test that attribute macros that expect to be used on fns are composable with contractimpl.
1515
#[parse_item_fn]
1616
pub fn empty() {}
17+
18+
// Test that attribute macros are not copied to the generated export functions. See test below.
19+
#[check_fn_is_item_fn]
20+
pub fn empty2() {}
1721
}
1822

1923
#[cfg(test)]
@@ -30,4 +34,24 @@ mod test {
3034

3135
client.empty();
3236
}
37+
38+
// Test that custom attribute macros on methods in #[contractimpl] are not
39+
// incorrectly applied to generated wrapper functions.
40+
//
41+
// The #[check_fn_is_item_fn] macro injects code that references `Self`,
42+
// which is only valid inside an impl block. If #[contractimpl] incorrectly
43+
// passes the attribute to generated free functions (like __invoke_raw_slice),
44+
// the injected `Self` reference would cause a compile error:
45+
// "error[E0411]: cannot find type `Self` in this scope"
46+
//
47+
// This test passing means the attribute is correctly filtered out of
48+
// generated wrapper functions and only applied to the original method.
49+
#[test]
50+
fn test_custom_attrs_are_not_copied_onto_generated_fns() {
51+
let e = Env::default();
52+
let contract_id = e.register(Contract, ());
53+
let client = ContractClient::new(&e, &contract_id);
54+
55+
client.empty2();
56+
}
3357
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
{
2+
"generators": {
3+
"address": 1,
4+
"nonce": 0,
5+
"mux_id": 0
6+
},
7+
"auth": [
8+
[],
9+
[]
10+
],
11+
"ledger": {
12+
"protocol_version": 25,
13+
"sequence_number": 0,
14+
"timestamp": 0,
15+
"network_id": "0000000000000000000000000000000000000000000000000000000000000000",
16+
"base_reserve": 0,
17+
"min_persistent_entry_ttl": 4096,
18+
"min_temp_entry_ttl": 16,
19+
"max_entry_ttl": 6312000,
20+
"ledger_entries": [
21+
{
22+
"entry": {
23+
"last_modified_ledger_seq": 0,
24+
"data": {
25+
"contract_data": {
26+
"ext": "v0",
27+
"contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM",
28+
"key": "ledger_key_contract_instance",
29+
"durability": "persistent",
30+
"val": {
31+
"contract_instance": {
32+
"executable": {
33+
"wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
34+
},
35+
"storage": null
36+
}
37+
}
38+
}
39+
},
40+
"ext": "v0"
41+
},
42+
"live_until": 4095
43+
},
44+
{
45+
"entry": {
46+
"last_modified_ledger_seq": 0,
47+
"data": {
48+
"contract_code": {
49+
"ext": "v0",
50+
"hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
51+
"code": ""
52+
}
53+
},
54+
"ext": "v0"
55+
},
56+
"live_until": 4095
57+
}
58+
]
59+
},
60+
"events": []
61+
}

0 commit comments

Comments
 (0)