fix(sdk): M1 SDK sweep fixes - macro hardening, doc fidelity#246
Conversation
edd46ba to
af4a126
Compare
af4a126 to
ef2e37a
Compare
ef2e37a to
f33abe9
Compare
f33abe9 to
41018ab
Compare
41018ab to
1554feb
Compare
1554feb to
9285d37
Compare
9285d37 to
ceab8bd
Compare
ceab8bd to
eaab2ce
Compare
eaab2ce to
6168e79
Compare
6168e79 to
319f48e
Compare
319f48e to
93b1a7d
Compare
93b1a7d to
bfd97b4
Compare
bfd97b4 to
6c22e84
Compare
6c22e84 to
489bea6
Compare
489bea6 to
078faf1
Compare
7044e1f to
127ccf3
Compare
127ccf3 to
7f54893
Compare
7f54893 to
aafc6bb
Compare
aafc6bb to
e73d859
Compare
e73d859 to
23b9b9b
Compare
23b9b9b to
cc9d464
Compare
cc9d464 to
f21768f
Compare
f21768f to
ac21033
Compare
lgahdl
left a comment
There was a problem hiding this comment.
Good hardening. Three items below.
| /// namespace), and the crate root must not shadow std prelude names | ||
| /// such as `Result`, `Vec`, or `Ok` (wit-bindgen's generated `Guest` | ||
| /// trait refers to them unqualified). | ||
| #[proc_macro_attribute] |
There was a problem hiding this comment.
The second corollary may be inaccurate: the macro's own quote! output already uses ::core::result::Result, ::std::vec::Vec, and ::core::result::Result::Ok — fully qualified paths — so shadowing those names in the crate root would not break the macro-emitted code. The constraint, if real, comes from the wit_bindgen::generate! expansion itself, not from "wit-bindgen's generated Guest trait" broadly. Worth verifying which names generate! actually emits unqualified before documenting this as a constraint; if it turns out the expansion is also hygienic, the corollary can be dropped.
| /// namespace), and the crate root must not shadow std prelude names | |
| /// such as `Result`, `Vec`, or `Ok` (wit-bindgen's generated `Guest` | |
| /// trait refers to them unqualified). | |
| #[proc_macro_attribute] | |
| /// `CARGO_MANIFEST_DIR`. One corollary: the consuming crate must | |
| /// declare `wit-bindgen` as a direct dependency (the emitted | |
| /// `wit_bindgen::generate!` call resolves against the consumer's | |
| /// namespace, not `nexum-macros`'s). |
| format!( | ||
| "`{name}` is not a recognised #[nexum_sdk::module] handler; expected one \ | ||
| of {HANDLERS:?} (rename helpers so they do not start with `on_`)" | ||
| ), |
There was a problem hiding this comment.
The error tells the user to rename, but the lower-friction fix is to move the helper to a separate impl block — the macro only inspects the single attributed impl. A user with a well-named on_retry_cooldown_expired helper would prefer that over renaming.
| format!( | |
| "`{name}` is not a recognised #[nexum_sdk::module] handler; expected one \ | |
| of {HANDLERS:?} (rename helpers so they do not start with `on_`)" | |
| ), | |
| format!( | |
| "`{name}` is not a recognised #[nexum_sdk::module] handler; \ | |
| expected one of {HANDLERS:?}. Move helpers that start with \ | |
| `on_` to a separate `impl` block, or rename them." | |
| ), |
| synchronous `fn`s; there is no `async` support, no `block_on`, and | ||
| no separate `#[shepherd::module]`. | ||
|
|
||
| Earlier drafts of this section described a richer 0.2 surface |
There was a problem hiding this comment.
"Earlier drafts" is ambiguous — it reads as earlier commits of this file (stale docs) rather than features that were planned and cut. A reader who wrote code against those APIs gets a different message from each reading.
| Earlier drafts of this section described a richer 0.2 surface | |
| This section previously described a richer 0.2 surface |
ac21033 to
5281ffa
Compare
1795182 to
f7f2f3a
Compare
5281ffa to
eeb9deb
Compare
428db31 to
e205b8b
Compare
e205b8b to
ef44329
Compare
A method named on_blocks (or any other on_-prefixed typo) previously compiled as an ordinary helper while its event silently no-opped; the only signal was a dead_code warning that vanishes on pub items. Reserve the on_ prefix for the recognised handler set and error on anything else. Also reject trait impls, generic impls, and impls with no recognised handlers - all previously slipped past the self-type guard and failed later with incidental, unhygienic errors (or compiled into a do-nothing module). Document the wit-bindgen direct-dependency and prelude- shadowing invariants inherited from the generated glue.
Section 7 still described the superseded Signer/Messaging/RemoteStore/ provider() vision - APIs that never shipped - and a #[shepherd::module] macro that does not exist, while doc 05 endorsed it as the authoritative 0.1 -> 0.2 SDK table. Replace it with the real story: 0.1 had no SDK crate, 0.2 ships nexum-sdk/shepherd-sdk host traits, typed errors, and the single #[nexum_sdk::module] attribute. Point doc 05's cross-link text at what the section now says, and fix doc 05 details flagged by the sweep: the CowApiHost parenthetical (submit_order, cow_api_request), the crate trees (proptests.rs in both SDK crates), the testing-story claim (nexum-runtime's feature-gated test_utils::TestRuntime is an in-tree component-level harness), and the first #[nexum::module] mention now names the real nexum_sdk path.
sdk.md is doc 05's designated day-to-day reference but still presented the pre-macro bind_host_via_wit_bindgen! flow as the authoring pattern. Add an authoring section covering the attribute and its handler set, mention it in the intro, and include nexum-macros in the rustdoc command.
What
Rewrites
docs/migration/0.1-to-0.2.mdsection 7 to match the SDK surface that actually shipped (0.2 ships host traits, typed errors, and the single#[nexum_sdk::module]attribute; 0.1 never had an SDK crate at all), and adds an explicit note thatSigner/Messaging/RemoteStore/provider()/#[shepherd::module]never shipped. Retargets doc 05's cross-link text from "rename table" to "what changed".Hardens the
#[nexum_sdk::module]macro (crates/nexum-macros/src/lib.rs) to reject, each with a targeted compile error, anyon_-prefixed method not in theHANDLERSset (previously a silent no-op for typo'd handlers), trait impls, generic impls, and impls with zero recognised handlers.Qualifies doc 05's testing-story claim to the SDK side of things, documenting
nexum-runtime's feature-gatedtest_utils::TestRuntimeas the separate in-tree component-level harness that sits outside the module-author SDK contract. Also corrects doc 05'sCowApiHostparenthetical (submit_order,cow_api_request), the crate trees, and the first#[nexum::module]mention to name the realnexum_sdkpath.Adds an "Authoring a module" section to
docs/sdk.mddocumenting the#[nexum_sdk::module]attribute and its handler set, and includesnexum-macrosin the rustdoc command.Why
Stacked QA fixes for the M1 SDK sweep (part of #136): the migration doc and doc 05 had drifted from what actually shipped, and the module macro silently swallowed typo'd or malformed handler methods instead of failing at compile time. Fixing both keeps the docs trustworthy for module authors and closes a real correctness gap in the macro. This PR is stacked on docs/m1-sdk-design-rewrite (its base branch).
Testing
All run inside
nix develop --command, with--lockedand--config 'build.target-dir="/code/nxm/runtime/target"':cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningsRUSTDOCFLAGS=-D warnings cargo doc --workspace --no-depscargo build --release --target wasm32-wasip2for example, twap-monitor, ethflow-watcher, price-alert, balance-tracker, stop-loss, flaky-bomb, fuel-bomb, memory-bombcargo test --workspace --all-features --no-fail-fastAI Assistance
Claude (Opus) used for the implementation.