Unraw raw identifiers in the service and proxy macros - #295
Merged
Conversation
derive(ReplyError) generates a Deserialize impl that clones the user's enum
variants into a helper enum, __ZlinkDeserHelper, nested inside the
deserialize function body so serde can derive on it directly. Being a
separate item in that scope, it doesn't inherit the user's own
#[allow(non_camel_case_types)] on their enum, so a lowercase variant name
(e.g. `lowercase { .. }`) fails `-D warnings` unconditionally. Worse, the
diagnostic points at the user's source and tells them to add the very
attribute they already have, with no way to silence it.
Add the allow directly to the generated helper, since it's the one place
that can actually see it.
Checked the other generated helper items in zlink-macros for the same
nested-in-a-function-body pattern (proxy chain/method wrapper structs and
enums, the Serialize impl's ParametersSerializer). None of them clone
user idents into a position that can trip non_camel_case_types: their
generated type names are either fixed or already normalized to
PascalCase, and the only cloned user idents end up as struct fields,
which that lint doesn't cover.
This only lifts the lint. A lowercase name is still not a valid Varlink error
name -- `error_def` parses it with `type_name`, which requires an uppercase
first letter -- so the realistic raw variants that motivated this (`r#fn`,
`r#match`) stay unusable, now for that reason instead. Making the derives
reject names our own parser cannot read is a separate concern.
Fixes #293.
Assisted-by: Claude Sonnet 4.5
Assisted-by: Claude Opus 4.8 (1M context)
The service macro builds the IDL parameter name by hand but leaves the wire name to serde, and serde unraws idents. The two therefore disagreed for a raw parameter: the IDL advertised `r#type` while the method only ever deserialized `type`, so a client trusting our own interface description got `missing field type`. It compiled silently and nothing caught it. The IDL is the wrong half here. Varlink cannot express `r#type` at all -- `#` starts a comment -- so the description we emitted was not even parseable, as the new test shows by round-tripping it through our own parser. A raw method name had a second failure: the name reached `format_ident!` as `R#type` and panicked with no span and no hint. Both now resolve through `naming::unraw`, keeping naming decisions in the one module that owns them so the IDL and the wire cannot drift apart. The `_`-prefix check guarding those names had the same gap: it judged the raw string, so `r#_foo` slipped past it and then reached the IDL unraw'd as the `_foo` the check exists to reject. It now judges the string `wire_name` does. Fixes #292. Assisted-by: Claude Opus 4.8 (1M context)
A raw method name (`async fn r#type`) made the proxy macro panic with `"R#typeParams" is not a valid identifier`: the name reached `format_ident!` still carrying its `r#`. Same class as the service macro fix, but with a milder consequence -- it never compiled, so no wrong bytes could reach the wire. Parameters were already fine here: the proxy has no IDL side and inherits serde's unrawing, so `r#move` has always serialized as `move`. Only the string-derived names change: the wire method name, the generated `TypeParams`/`TypeWrapper` idents, and the `chain_type` ident. The generated fn deliberately keeps the raw ident, since it still has to match the trait it implements. Assisted-by: Claude Opus 4.8 (1M context)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Raw identifiers, continued from #294 — plus one lint fix found alongside them.
r#is Rust syntax for using a keyword as an identifier; it is never part of the name. #294 taughtthe
Type/CustomType/introspect::ReplyErrorderives that. These three commits finish thejob for the two macros #294 didn't touch, and fix an unrelated-but-adjacent lint bug found while
writing its tests.
zlink-macros: Allow non-camel-case variants in ReplyError's helper(derive(ReplyError): a lowercase variant fails-D warningsand the user's own#[allow]can't silence it #293) — the wireReplyErrorderive nests a helper enum insideDeserialize::deserializeto clone the variantsacross for serde. Nested items don't inherit attributes, so the user's own
#[allow(non_camel_case_types)]could never reach it: a lowercase variant failed-D warningswith the diagnostic telling the user to add the attribute they already had. Nothing to do with
raw idents.
zlink-macros: Unraw idents in the service macro(#[zlink::service]mishandles raw identifiers: the IDL disagrees with the wire #292) — the macro built the IDL parametername by hand while leaving the wire name to serde, and serde unraws. The two disagreed, and the
IDL was the wrong half: Varlink can't express
r#typeat all (#starts a comment), so what weemitted wasn't parseable by our own parser. A raw method name additionally panicked in
format_ident!asR#type.zlink-macros: Unraw idents in the proxy macro— no issue of its own; found while verifying#[zlink::service]mishandles raw identifiers: the IDL disagrees with the wire #292, whose text wrongly claimed proxy was unaffected. Parameters were indeed fine (serde unrawsthem), but a raw method name panicked with
"R#typeParams" is not a valid identifieracross 8sites, one of which would have built the ident
chain_r#type.Both halves of #292 had to land together: unrawing only the static's ident would have made
r#typecompile while the IDL still described the field asr#type.An explicit
#[zlink(rename = "...")]is still used verbatim throughout — that's a user-suppliedstring, not an ident.
Each commit passes standalone (469 → 471 → 473), so the branch bisects. Every fix is
mutation-verified: reverting it fails a test with the error the issue reports.
Fixes #292.
Fixes #293.