Ignore serde attributes specta does not model (fixes remote = "Self")#2
Merged
Merged
Conversation
The serde attribute parsers returned Ok for unknown keys without consuming their value, so syn's parse_nested_meta failed the whole derive with "expected `,`" on any name-value or list attribute specta does not model - container remote/bound/expecting, variant bound, field getter - whenever the serde feature is enabled. serde validates its own attribute namespace, so specta's job is to skip what it does not understand: consume the value (or parenthesized list) and move on.
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
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.
When the
serdefeature is enabled,parse_container_meta/parse_variant_meta/parse_field_metareturnOk(())for serde attribute keys they do not recognize without consuming the attribute's value. syn'sparse_nested_metathen hits the unconsumed= valuetokens and fails the whole derive withexpected,``, so the type silently loses itsTypeimpl and downstream crates fail with "the trait `specta::Type` is not implemented".Any legal serde name-value or list attribute outside the subset specta models triggers this: container
remote/bound/expecting, variantbound, fieldgetter. The one that bit us is#[serde(remote = "Self")]onFlightDatain the fltsci repo - the pattern that makes serde emit its derived serialize/deserialize functions as inherent items so the trait impls can be hand-written (migrate-then-parse for the durable JSON schema versioning design). Buildingflights_apialone worked, because nothing there enablesspecta-macros/serde; any build includingspecta-serde(DX bindings, payload-compatibility checker, rust-analyzer workspace checks) failed.Fix: an
elsebranch in each of the three parsers that consumes and discards the value (= <expr>or a parenthesized list). serde validates its own attribute namespace - a typo still fails the serde derive itself - so specta's job is only to skip what it does not model, including attributes serde adds in the future.Tests:
tests/tests/serde_unknown_attrs.rscovers all five attributes above and asserts the exported shape is unaffected. Before the fix, that file fails to compile with fiveexpected,`` errors. Also verified end-to-end with a scratch crate mirroring the fltsci setup (serde_with::apply+ `Type` derive + `remote = "Self"` + hand-written trait impls delegating to the inherent functions, with `specta-serde` in the graph): round-trip works and `PhasesFormat` exports the correct phase-specific shapes.