Let Serde hand back the scalars it accepts - #730
Merged
Conversation
serialize() takes a string or an int — its own test passes both through it — but deserialize() was declared object|array, so reading either back raised a TypeError on the way out. Anything that cached a bare scalar could store it and never read it again; the file cache is one call away from that, and today only gets away with it because every caller happens to cache an array or an object. Widen the native return type to what the other half accepts. The phpDoc is left exactly as it was: the parenthesised form that would satisfy the parser reintroduces template-resolution errors at every call site, which is why it is baselined with that note. The one place the wider type rippled to is the public link view, where the deserialized vault is annotated at the call site instead — so its baseline entry goes, one fewer than before.
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.
The bug
Serde::serialize()acceptsarray|object|string|int— and its own test passes a string and an int through it — whileSerde::deserialize()was declared to returnobject|array. So reading either scalar back raised:Anything that cached a bare scalar could store it and never read it again.
FileCache::load()is one call away from that; it only gets away with it today because every caller happens to cache an array or an object.The fix
Widen the native return type to what the other half accepts.
The phpDoc is left exactly as it is. The parenthesised form that would satisfy the parser reintroduces template-resolution errors at every call site — which is why it carries a baseline entry with that note already. Changing it here would be re-treading a decision that was made and recorded.
The one place the wider type rippled to is the public link view, where the deserialized vault is annotated at the call site instead. Its baseline entry goes with it — the baseline shrinks by one, from 350 to 349.
Testing
The existing serialize provider — a string and an int among them — now runs through
deserialize()as well. It fails against the old signature with exactly theTypeErrorabove, for both scalars.Unit suite green: 3085. PHPStan and PHPCS clean.