✨ zlink-macros: Support rename_all_arguments on the proxy macro - #299
Conversation
rename_all attribute for method parameter renaming
2425123 to
811d084
Compare
811d084 to
35a629d
Compare
35a629d to
ccda498
Compare
rename_all attribute for method parameter renamingrename_all on the proxy macro
There was a problem hiding this comment.
Pull request overview
This PR extends the #[proxy] macro in zlink-macros to support a rename_all = "...“ attribute, allowing proxy method parameter names to be case-converted automatically (with per-parameter #[zlink(rename = "...")] still taking precedence). This fits the macros crate’s role of generating client-side Varlink proxy code with correct wire-format naming.
Changes:
- Parse
rename_allfrom#[proxy(...)]attributes and thread it through proxy method / chain method code generation. - Apply
RenameAll::apply_to_fieldto parameters that do not have an explicit#[zlink(rename = "...")]. - Add/extend tests and documentation to cover
rename_allusage and precedence.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| zlink-macros/src/naming.rs | Exposes RenameAll::parse to allow proxy macro parsing. |
| zlink-macros/src/proxy.rs | Parses rename_all from proxy attributes and passes it into generators. |
| zlink-macros/src/proxy/method_impl.rs | Applies rename_all when generating serde parameter structs (method impl path). |
| zlink-macros/src/proxy/chain_method.rs | Applies rename_all for chain methods’ parameter serialization. |
| zlink-macros/src/proxy/chain_extension.rs | Applies rename_all for chain extension methods’ parameter serialization. |
| zlink-macros/src/lib.rs | Documents rename_all as a supported #[proxy] attribute and lists accepted values. |
| zlink-macros/tests/proxy/rename.rs | Adds tests for rename_all (PascalCase/camelCase), explicit override precedence, and chain methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zeenix
left a comment
There was a problem hiding this comment.
Reviewed on top of the existing comments (not repeating them). The raw-identifier issue already flagged in three files is real; two additional findings inline: (1) rename_all-produced names are never validated against the Varlink field grammar, so documented values like kebab-case silently emit wire names Varlink cannot express — the opposite of what #298 established for the derives; (2) the resolution logic is triplicated across the three codegen files, which is why the raw-ident bug appears three times — it should be a single shared helper. Fixes for all comments follow on claude/zlink-pr-299-review-r18v2c.
Generated by Claude Code
bcff004 to
4d60ddb
Compare
rename_all on the proxy macrorename_all_arguments on the proxy macro
systemd's Varlink APIs use camelCase for method arguments per their Varlink API style guide, with PascalCase where a name mirrors a documented option name (the Varlink spec itself mandates no case convention; its own examples are snake_case). Rust arguments are snake_case, so today each argument of a proxy for such a service must be individually annotated with `#[zlink(rename = "...")]`. This adds `rename_all_arguments` to the `#[proxy]` macro so a single attribute converts all argument names automatically. Per-argument `#[zlink(rename = "...")]` takes precedence over `rename_all_arguments`, which irregular names (e.g. machined's `OSRelease`) still need. The attribute is named `rename_all_arguments`, not `rename_all`, so it is obvious it renames the method arguments and not the methods. The resolution lives in a single helper shared by the three code generators (regular, chain and chain-extension methods), so the precedence rule cannot drift between them. The convention is applied to the unraw'd ident -- `r#` is Rust syntax, never part of the name, so `r#type` under `PascalCase` maps to `Type`, not `R#type`. Every name the convention produces is validated against the Varlink field grammar, matching what the derives and the service macro already guarantee since the fix for #296: `kebab-case` turning `dry_run` into `dry-run` is a compile error on the argument, not a wire name the peer rejects. Tests cover PascalCase and camelCase conversion, explicit-override precedence, chain methods, raw identifiers (plain and chained), and the grammar rejection (as a compile_fail doctest and helper unit tests). Fixes #170. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2pHFDxsBxrrYAVKeCosGy
4d60ddb to
798aa8f
Compare
systemd's Varlink APIs use camelCase for method arguments per their Varlink API style guide, with PascalCase where a name mirrors a documented option name (the Varlink spec itself mandates no case convention; its own examples are snake_case). Rust arguments are snake_case, so today each argument of a proxy for such a service must be individually annotated with
#[zlink(rename = "...")]— see the machined mock inzlink/tests/mock_machined_service.rs, which hand-renames every argument. This addsrename_all_argumentsto the#[proxy]macro so a single attribute converts all argument names automatically. The attribute is deliberately not calledrename_all: the explicit name makes it obvious it renames the method arguments, not the methods.Per-argument
#[zlink(rename = "...")]takes precedence overrename_all_arguments, which irregular names (e.g. machined'sOSRelease,UIDShift) still need. The convention is applied to the unraw'd ident (r#typeunderPascalCasemaps toType, notR#type), and every produced name is validated against the Varlink field grammar — matching what the derives and the service macro already guarantee since #296 — so e.g.kebab-caseon a multi-word argument is a compile error, not an invalid wire name.Fixes #170.
Changes
naming.rs: WidenRenameAll::parsetopub(crate), make its unknown-value error attribute-neutralproxy.rs: Parserename_all_argumentsfrom proxy attributes, thread it to all three code generatorsproxy/utils.rs: Newresolve_serialized_namehelper holding the precedence rule, the unrawing, and the grammar validation in one placemethod_impl.rs/chain_extension.rs/chain_method.rs: Resolve argument wire names through the shared helperlib.rs: Documentrename_all_argumentsin the supported-attributes list, including the systemd camelCase guidance and the grammar constraint (with acompile_faildoctest)tests/proxy/rename.rs: Tests for PascalCase, camelCase, explicit-override precedence, chain methods, and raw identifiers (plain and chained)