Bump better-generics to ^1.0 and add #[JsonMapper] class attribute#9
Merged
thiagocordeiro merged 8 commits intomainfrom May 3, 2026
Merged
Bump better-generics to ^1.0 and add #[JsonMapper] class attribute#9thiagocordeiro merged 8 commits intomainfrom
thiagocordeiro merged 8 commits intomainfrom
Conversation
The 1.0 release renamed Tcds\Io\Generic\Reflection\Type\Parser\TypeParser
out of existence and exposes the equivalent string-shaped helpers via
DocBlockTypeResolver instead.
Migrate the three call sites:
- TypeParser::getGenericTypes(string) → DocBlockTypeResolver::instance()
->genericTypeParts(string) (same array{string, list<string>} return).
- TypeParser::getParamMapFromShape(string) → DocBlockTypeResolver::instance()
->shapeMemberStrings(string) (same array{string, array<string,string>}
return).
PHPStan level=max stays clean and the unit suite passes (54 tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the existing $typeMappers constructor argument but pinned at
the class definition site, matching how Jackson lets you decorate the
class itself.
Usage:
#[JsonMapper(reader: MoneyReader::class, writer: MoneyWriter::class)]
readonly class Money { ... }
Resolution order on every read/write:
1. typeMappers constructor arg (most specific, wins)
2. #[JsonMapper] attribute on the target class
3. default reader/writer
The attribute carries class-strings rather than instances, so it works
for either Reader/Writer or StaticReader/StaticWriter implementations
that have a no-arg constructor. Closures still go through the
constructor argument because attributes can't carry them.
Includes Money/MoneyReader/MoneyWriter fixtures and a JsonMapperTest
covering: read via attribute, write via attribute, and the override
path where an explicit typeMappers entry beats the attribute.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
new ($attribute->reader)() instantiates a StaticReader, but a static reader has only static methods — there's nothing meaningful to call on the instance, and the existing $reader instanceof StaticReader check would be false on a class-string anyway. Refactor the reader/writer resolution into resolveReader/resolveWriter helpers that always return a Closure: - typeMappers explicit instance: same instanceof StaticReader split as before, but spell __invoke(...) instead of relying on $reader(...) so PHPStan stops second-guessing the union. - class attribute: pick StaticReader::read(...) / StaticWriter::write(...) via is_subclass_of without instantiating; otherwise instantiate the Reader/Writer and return ->__invoke(...). - default reader/writer: typed as Reader/Writer (never static), so ->__invoke(...) is direct. Inline assert(... instanceof Reader/Writer) on the attribute path narrows for PHPStan since is_subclass_of doesn't refine class-strings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
assert() is a no-op when zend.assertions=-1 (production), so the narrowing was only happening to satisfy PHPStan, not actually guarding against a misconfigured class-string at runtime. Replace with a real instanceof + throw JacksonException when the class implements neither Reader/Writer nor the static counterpart. PHPStan still narrows through the early-exit branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The attribute previously required the class-string to implement Reader/StaticReader (or Writer/StaticWriter). Relax that: any class with an __invoke matching MapperClosure (the closure shape declared on ObjectMapper) is now also accepted, which is the natural way to express a "closure as a class" since attributes can't carry literal closures. Resolution becomes: 1. is_subclass_of(.., StaticReader::class) → Class::read(...) 2. instantiate; is_callable($instance) → Closure::fromCallable($instance) 3. otherwise throw JacksonException with a clear message Adds Slug/SlugReader/SlugWriter fixtures (plain invokables, no Reader/Writer implements) and two JsonMapperTest cases covering both the invokable read and write paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The constructor now takes Closure|string|null on both reader and writer, so a JsonMapper instance built programmatically can carry a MapperClosure (the closure shape declared on ObjectMapper) rather than only a class-string. PHP attribute literals still can't carry closures, but library users who construct JsonMapper themselves — for tests, dynamic registration, etc. — now have the same expressive power as the typeMappers constructor argument. resolveReader/resolveWriter short-circuit on Closure before falling through the class-string branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The typeMappers entry can be a MapperClosure too, not just Reader/StaticReader. Previously the line collapsed it onto the $reader->__invoke(...) path — which works at runtime since Closure also has __invoke, but is misleading and PHPStan doesn't always narrow correctly through it. Treat Closure explicitly: if the registered value already is a Closure, return it directly; otherwise apply the StaticReader::read(...) / Reader::__invoke(...) split as before. Same fix for the writer side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The class-level attribute is the canonical declaration-site source for how the class should be (de)serialized; the typeMappers constructor argument is mapper-instance config that should not be able to silently override what the class itself declares. Reorder resolveReader/resolveWriter to check the attribute first, then typeMappers, then the default reader/writer. Update the docblock on JsonMapper accordingly, and rename/repurpose the test that previously asserted the opposite precedence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thiagocordeiro
force-pushed
the
chore/class-mapper-annotation
branch
from
3c355fa to
f7d7402
Compare
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
Two pre-1.0 changes stacked together.
Bump
tcds-io/php-better-genericsto^1.0— the 1.0 release replacesTcds\Io\Generic\Reflection\Type\Parser\TypeParserwithDocBlockTypeResolver. Migrate the three call sites:TypeParser::getGenericTypes(string)→DocBlockTypeResolver::instance()->genericTypeParts(string)TypeParser::getParamMapFromShape(string)→DocBlockTypeResolver::instance()->shapeMemberStrings(string)Add
#[JsonMapper]class attribute for pinning a custom reader/writer at the class definition site:Resolution order on every read/write:
typeMappersconstructor argument (most specific, wins)#[JsonMapper]attribute on the target classAttribute carries class-strings (PHP attributes can't hold closures), so it works for either
Reader/WriterorStaticReader/StaticWriterimplementations with a no-arg constructor. Closures still go throughtypeMappers.Adds
Money/MoneyReader/MoneyWriterfixtures and aJsonMapperTestcovering read via attribute, write via attribute, and the override path wheretypeMappersbeats the attribute.Test plan
composer cs:checkcleancomposer test:stan(level=max) cleancomposer test:unit— 57 tests / 93 assertions🤖 Generated with Claude Code