-
Notifications
You must be signed in to change notification settings - Fork 2
fix(sdk): M1 SDK sweep fixes - macro hardening, doc fidelity #246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,8 +16,10 @@ use quote::quote; | |||||||||||||||||||
| use syn::{ImplItem, ItemImpl, Type}; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// The handler names recognised on a `#[module]` impl. Any method not in | ||||||||||||||||||||
| /// this set is left untouched on the type; any handler in the set that is | ||||||||||||||||||||
| /// absent is treated as a no-op in the generated `on-event` dispatch. | ||||||||||||||||||||
| /// this set is left untouched on the type, except that names starting | ||||||||||||||||||||
| /// with `on_` are rejected at compile time (a typo'd handler would | ||||||||||||||||||||
| /// otherwise silently never fire); any handler in the set that is absent | ||||||||||||||||||||
| /// is treated as a no-op in the generated `on-event` dispatch. | ||||||||||||||||||||
| const HANDLERS: [&str; 5] = ["init", "on_block", "on_chain_logs", "on_tick", "on_message"]; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// Generate the per-cdylib glue for a nexum module. | ||||||||||||||||||||
|
|
@@ -34,7 +36,12 @@ const HANDLERS: [&str; 5] = ["init", "on_block", "on_chain_logs", "on_tick", "on | |||||||||||||||||||
| /// (`Guest`, `Fault`, the `nexum::host::*` modules) lands at the module | ||||||||||||||||||||
| /// crate root, so the emitted glue and the handler bodies resolve those | ||||||||||||||||||||
| /// names there; the WIT directory is located by walking up from | ||||||||||||||||||||
| /// `CARGO_MANIFEST_DIR`. | ||||||||||||||||||||
| /// `CARGO_MANIFEST_DIR`. Two corollaries: the consuming crate must | ||||||||||||||||||||
| /// declare `wit-bindgen` as a direct dependency (the emitted | ||||||||||||||||||||
| /// `wit_bindgen::generate!` call resolves against the consumer's | ||||||||||||||||||||
| /// 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] | ||||||||||||||||||||
| pub fn module(attr: TokenStream, item: TokenStream) -> TokenStream { | ||||||||||||||||||||
| if !attr.is_empty() { | ||||||||||||||||||||
|
|
@@ -57,6 +64,42 @@ pub fn module(attr: TokenStream, item: TokenStream) -> TokenStream { | |||||||||||||||||||
| .to_compile_error() | ||||||||||||||||||||
| .into(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if let Some((_, trait_path, _)) = &input.trait_ { | ||||||||||||||||||||
| return syn::Error::new_spanned( | ||||||||||||||||||||
| trait_path, | ||||||||||||||||||||
| "#[nexum_sdk::module] must be applied to an inherent impl, not a trait impl", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| .to_compile_error() | ||||||||||||||||||||
| .into(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if !input.generics.params.is_empty() { | ||||||||||||||||||||
| return syn::Error::new_spanned( | ||||||||||||||||||||
| &input.generics, | ||||||||||||||||||||
| "#[nexum_sdk::module] must be applied to a non-generic impl", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| .to_compile_error() | ||||||||||||||||||||
| .into(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // A typo'd handler (`on_blocks`, `on_chainlogs`, ...) would otherwise | ||||||||||||||||||||
| // compile as an ordinary helper while its event silently no-ops, so | ||||||||||||||||||||
| // reserve the `on_` prefix for the recognised handler set. | ||||||||||||||||||||
| for item in &input.items { | ||||||||||||||||||||
| if let ImplItem::Fn(f) = item { | ||||||||||||||||||||
| let name = f.sig.ident.to_string(); | ||||||||||||||||||||
| if name.starts_with("on_") && !HANDLERS.contains(&name.as_str()) { | ||||||||||||||||||||
| return syn::Error::new_spanned( | ||||||||||||||||||||
| &f.sig.ident, | ||||||||||||||||||||
| format!( | ||||||||||||||||||||
| "`{name}` is not a recognised #[nexum_sdk::module] handler; expected one \ | ||||||||||||||||||||
| of {HANDLERS:?} (rename helpers so they do not start with `on_`)" | ||||||||||||||||||||
| ), | ||||||||||||||||||||
|
Comment on lines
+93
to
+96
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error tells the user to rename, but the lower-friction fix is to move the helper to a separate
Suggested change
|
||||||||||||||||||||
| ) | ||||||||||||||||||||
| .to_compile_error() | ||||||||||||||||||||
| .into(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| let present: Vec<&str> = input | ||||||||||||||||||||
| .items | ||||||||||||||||||||
|
|
@@ -69,6 +112,15 @@ pub fn module(attr: TokenStream, item: TokenStream) -> TokenStream { | |||||||||||||||||||
| _ => None, | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| .collect(); | ||||||||||||||||||||
| if present.is_empty() { | ||||||||||||||||||||
| return syn::Error::new_spanned( | ||||||||||||||||||||
| self_ty, | ||||||||||||||||||||
| "#[nexum_sdk::module] found no recognised handlers on this impl; define at least one \ | ||||||||||||||||||||
| of `init`, `on_block`, `on_chain_logs`, `on_tick`, `on_message`", | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| .to_compile_error() | ||||||||||||||||||||
| .into(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| let has = |name: &str| present.contains(&name); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| let (nexum_wit, shepherd_wit) = match locate_wit() { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -404,23 +404,35 @@ The Rust API surface is otherwise unchanged in 0.2. The C ABI and `nexum-host` e | |||||
|
|
||||||
| ### Rust SDK | ||||||
|
|
||||||
| ```diff | ||||||
| - use nexum_sdk::{provider, Identity, MsgClient, RemoteStore}; | ||||||
| + use nexum_sdk::{provider, Signer, Messaging, RemoteStore}; | ||||||
| ``` | ||||||
|
|
||||||
| | 0.1 type | 0.2 type | Notes | | ||||||
| |---|---|---| | ||||||
| | `IdentityClient` | `Signer` | Trait renamed to reflect what it does | | ||||||
| | `MsgClient` | `Messaging` | Drops the meaningless `Client` suffix | | ||||||
| | `CowClient` | `Cow` | Same | | ||||||
| | `HostTransport` | (internal) | Now `pub(crate)`; you access it through `provider()` | | ||||||
| | `block_on` (re-export) | (removed from public API) | Hidden behind the `#[nexum::module]` macro | | ||||||
| | `Error` (multiple variants per domain) | `Fault` + `HostFault` / `ChainError` | Per-interface typed errors over the shared vocabulary; see §2 | | ||||||
|
|
||||||
| ### Proc macro | ||||||
|
|
||||||
| `#[nexum::module]` and `#[shepherd::module]` are unchanged in shape. They now generate against `event-module` / `shepherd` worlds. If you targeted `headless-module` explicitly anywhere, rename to `event-module`. | ||||||
| 0.1 had no Rust SDK crate: modules called the wit-bindgen-generated | ||||||
| imports directly and hand-wrote the per-cdylib `Guest` / `export!` | ||||||
| glue. 0.2 ships `nexum-sdk` (host-neutral helpers) with | ||||||
| `shepherd-sdk` layering the CoW Protocol surface on top. What that | ||||||
| means for a module you are porting: | ||||||
|
|
||||||
| - **Host traits.** Strategy code is written against the | ||||||
| `ChainHost` / `LocalStoreHost` / `LoggingHost` traits (plus | ||||||
| `CowApiHost` in `shepherd-sdk`) instead of the raw wit-bindgen | ||||||
| import shims; the `bind_host_via_wit_bindgen!` / | ||||||
| `bind_cow_host_via_wit_bindgen!` macros generate the adapter at | ||||||
| the cdylib boundary, and the `nexum-sdk-test` / | ||||||
| `shepherd-sdk-test` mocks slot in for native unit tests. | ||||||
| - **Typed errors.** Handlers and host traits use the `Fault` / | ||||||
| `ChainError` / `CowApiError` vocabulary from §2 in place of 0.1's | ||||||
| bare strings. | ||||||
| - **One attribute macro.** `#[nexum_sdk::module]` (a re-export of | ||||||
| `nexum_macros::module`) replaces the hand-written glue: applied to | ||||||
| an inherent `impl` block of named handlers (`init`, `on_block`, | ||||||
| `on_chain_logs`, `on_tick`, `on_message`), it emits the | ||||||
| `wit_bindgen::generate!` call, the host adapter, the `Guest` impl | ||||||
| dispatching to the handlers present, and `export!`. Handlers are | ||||||
| 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 | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "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.
Suggested change
|
||||||
| (`provider()`, `Signer`, `Messaging`, `RemoteStore`, a hidden | ||||||
| `HostTransport`); none of that shipped. See | ||||||
| [doc 05](../05-sdk-design.md) for the SDK that exists. | ||||||
|
|
||||||
| ### Non-Rust SDKs | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 thewit_bindgen::generate!expansion itself, not from "wit-bindgen's generatedGuesttrait" broadly. Worth verifying which namesgenerate!actually emits unqualified before documenting this as a constraint; if it turns out the expansion is also hygienic, the corollary can be dropped.