Skip to content

bindings-macro: reject non-ident reducer/procedure/view argument patterns (#2369) - #5847

Open
Manojseetaram wants to merge 3 commits into
clockworklabs:masterfrom
Manojseetaram:bindings_macro
Open

bindings-macro: reject non-ident reducer/procedure/view argument patterns (#2369)#5847
Manojseetaram wants to merge 3 commits into
clockworklabs:masterfrom
Manojseetaram:bindings_macro

Conversation

@Manojseetaram

Copy link
Copy Markdown

Description of Changes

API and ABI breaking changes

Expected complexity level and risk

Testing

Fixes intermittend SSL error when fetching the iai-calllgrind fit dependcy in ci , caused by cargo's default libgit2-based git client. Shellling out to the system git binary avoids this ,
Fixes clockworklabs#5186 problem
@Manojseetaram

Copy link
Copy Markdown
Author

The bug

crates/bindings-macro's reducer, procedure, and view macros extract each
argument's name for the ARG_NAMES const, which feeds into the schema
(ProductType) sent to SDK/client codegen. When an argument's pattern
wasn't a plain identifier (e.g. a destructuring pattern like
SomeProduct { a, b }: SomeProduct), the macro silently emitted None
instead of erroring — producing an unnamed field in the generated schema
instead of any indication something was wrong.

The fix

Reject non-identifier argument patterns at macro-expand time with a clear
error, rather than the alternative (auto-generating synthetic names) — this
seemed like the safer, simpler option to land first. Bare _ wildcards are
still allowed, since _ intentionally means "no name" and doesn't lose any
named sub-field information the way destructuring does.

This logic was duplicated identically across reducer.rs, procedure.rs,
and view.rs, so I extracted it into a shared extract_arg_names helper in
util.rs.
x
Screenshot 2026-08-31 at 12 04 32 AM
image

​```rust
#[derive(SpacetimeType)]
struct SomeProduct {
a: i32,
b: i32,
}

#[spacetimedb::reducer]
fn foo(_: &ReducerContext, _: i32, SomeProduct { a, b }: SomeProduct) {
let _ = a + b;
}
​```

cargo check -p sdk-test-module now fails with:

error: arguments must be simple identifiers (e.g. `x: MyType`) or `_`, not patterns like destructuring; bind to a name and destructure in the function body instead --> modules/sdk-test/src/lib.rs:842:36 | 842 | fn foo(_: &ReducerContext, _: i32, SomeProduct { a, b }: SomeProduct) { | ^^^^^^^^^^^^^^^^^^^^ ​

Confirmed all other existing reducers/procedures/views in the repo still
compile (searched for any _ wildcard usage as an argument — none exist
currently, but the fix explicitly allows it going forward).

@bfops

bfops commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Hi @Manojseetaram, if you're intending for us to take a look at this PR, please fill out the description.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants