feat: harden serde-shape contracts and ergonomics - #12
Merged
Conversation
Container tagging duplicated enum repr, while has_flatten duplicated field wire positions. Manual implementations could therefore construct graphs whose public fields contradicted one another. Why: downstream walkers should not need precedence rules for impossible states. Enum repr and FieldWireShape now remain the single authoritative locations.
Migrate serde-shape-derive to serde_derive_internals 0.30 and syn 3, including the updated name, function-pointer, and parser APIs. Why: Serde 1.0.229 already uses this parser stack. Sharing it removes a second syn major version from downstream builds and keeps serde-shape attribute parsing and diagnostics on the same generation as Serde.
Recognize the internal deserializers that Serde injects for borrowed Cow<str> and Cow<[u8]> fields, and emit their known String and Bytes wire shapes. Why: these helpers are generated by Serde rather than user-defined custom behavior. Treating them as opaque prevented documentation and configuration tooling from inspecting otherwise ordinary borrowed fields.
Require Ord for tree collections and Eq, Hash, BuildHasher, and Default where Serde needs them to construct hash collections during deserialization. Why: DeserializeShape should describe an input contract that the corresponding Serde type can actually accept. The previous under-constrained impls exposed shapes for collection instantiations with no Deserialize implementation.
Replace the embedded TOML and environment-variable editor with one focused traversal test covering flattened fields, internal tags, newtype payloads, renamed fields, and graph resolution. Remove the dependencies used only by the editor. Why: serde-shape must expose enough metadata for configuration consumers without maintaining ScopeQL-like naming and mutation policy. The previous 337-line test duplicated downstream application logic and made unrelated policy changes look like library regressions.
Add FieldWireShape::shape() to borrow the contributed ShapeRef from regular, flattened, or inline fields while returning None for omitted fields. Why: graph consumers commonly need to continue traversal without branching on wire position. A single accessor removes repeated non-exhaustive matches while preserving the enum for callers that do care about placement.
Document that arrays and pointer-width integers retain useful Rust semantics even when Serde dispatches them through different low-level serializer methods. Why: callers should not mistake a shape graph for a recording serializer or a format-independent byte-level wire contract. The previous wording promised exact data-model calls that the public model intentionally normalizes.
Keep blanket shape forwarding for serialized references, but restrict deserialization references to Serde-supported &str, &[u8], and &Path inputs. Why: Serde has no blanket Deserialize implementation for &T or &mut T. The old impl made invalid types such as &u32 appear deserializable and also prevented downstream crates from implementing precise shapes for their own borrowed local types.
Remove the workspace-wide allowances for too_many_arguments and type_complexity; the current codebase passes both lints without local exceptions. Why: blanket suppression lets future oversized APIs and unreadable types enter unnoticed. Exceptions should be justified where they are needed, not pre-authorized for the whole project.
Remove the container-level transparent boolean while retaining transparent behavior through FieldWireShape::Inline on the affected field. Why: transparency was fully duplicated by wire placement, so manual shapes could disagree about whether a container was transparent. Keeping one observable representation makes walkers and hand-written implementations unambiguous.
Package and verify the derive archive first, then unpack the main archive and check it with crates.io patched to that packaged derive implementation. Why: both crates currently share an already-published version. Cargo verification fetched the old registry derive source, so the release workflow was not checking the pair it was about to publish.
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
Each commit covers one concern and explains its rationale in a
Why:paragraph. The PR description is intentionally brief so the commit sequence remains the review path.Validation
cargo x build --lockedcargo x testcargo x lintcargo x package --lockedno_stdchecks forthumbv7em-none-eabihfon Rust 1.85.0