feat: tighten serde-shape's public contract - #13
Open
tisonkun wants to merge 9 commits into
Open
Conversation
Replace the identical serialization and deserialization name records with TypeName, and add TypeName::of<T> for the common type_name plus Serde-name construction path. Why: direction is already expressed by the graph containing the value. Two structurally identical public types made manual implementations and consumers duplicate imports and conversion code without preventing any invalid state.
Add from_fn constructors to both graph directions and route for_type through them. The builder receives the normal graph context, so named and recursive definitions remain available. Why: custom shape functions already work at derive boundaries, but a foreign root still required a throwaway local wrapper. The graph API should accept the same customization mechanism directly.
Add definition_for to both graph directions, use it for root_definition, and remove the downstream helper that repeated ShapeRef matching plus id lookup. Why: every graph walker reaches definitions through ShapeRef values. Requiring each consumer to reproduce this plumbing adds noise and creates inconsistent handling of non-definition or foreign ids.
Remove DeserializeShape from unsized str, byte slices, and Path while keeping explicit implementations for Serde-supported borrowed and owned forms. Why: the trait describes a type’s deserialization contract, but those unsized types do not implement Serde Deserialize themselves. Advertising them made compile-time reflection claim capabilities that callers could not use.
Move atomic SerializeShape and DeserializeShape implementations, their tests, and their support documentation behind the crate’s std feature. Why: Serde only implements its atomic traits with std enabled. Exposing atomic shapes in a no_std graph claimed a contract that the matching Serde configuration cannot provide.
Why: field_identifier and variant_identifier enums deserialize through Serde's identifier visitor rather than the externally tagged enum protocol. Calling them externally tagged misleads graph consumers, while allowing a serialization shape contradicts Serde's own derive rejection.
Why: matching only the helper function name caused unrelated custom deserializers such as a user-defined borrow_cow_str path to be reported as String. The recovery is valid only for the exact private path synthesized by serde_derive_internals.
Why: Serde serializes core::fmt::Arguments through its string representation, but downstream crates cannot add SerializeShape for this foreign standard-library type themselves.
Why: Serde supports OsStr and OsString on Unix and Windows, but downstream crates cannot implement shape traits for these foreign standard-library types. Their wire form is platform-specific, so a plain String shape would hide the tagged Unix byte or Windows wide-unit representation.
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.
Summary
Review
The nine commits are intentionally independent. Each commit body contains a
Why:paragraph explaining the concrete user problem and why the change belongs in this foundational crate; the commit sequence is the intended review path.Breaking changes and new support are recorded in
CHANGELOG.md.Validation
cargo x build --lockedcargo x testcargo x lintcargo x package --lockedno_stdchecks forthumbv7em-none-eabihfon Rust 1.85.0