Symfony: Add support for #[MapRequestPayload] and #[MapQueryString] DTOs - #339
Merged
Conversation
janedbal
changed the title
Contributor
|
@janedbal Can this be merged? |
Co-Authored-By: Claude Code
…meters The vendor RequestPayloadValueResolver explicitly supports nullable and defaulted controller arguments (it resolves to null when the query string or payload is absent), so `#[MapQueryString] ?FilterDto $filter = null` is the canonical usage pattern. The provider guarded the parameter type with isObject()->yes(), which is only "maybe" for a nullable type, so such DTOs were skipped entirely and their constructor/properties reported as dead. Strip null from the type before extracting object class names instead. Co-Authored-By: Claude Code
The vendor RequestPayloadValueResolver matches these attributes with ArgumentMetadata::IS_INSTANCEOF, so custom attributes extending MapRequestPayload or MapQueryString (e.g. to preset a format or validation groups) are resolved the same way. The provider compared attribute class names strictly, missing such subclasses and reporting their DTOs as dead. Resolve the attribute class and check descendance instead. Co-Authored-By: Claude Code
The vendor resolver supports mapping a JSON array into a list of DTOs: an `array`-typed controller argument combined with the attribute's $type argument is denormalized to `X[]`, instantiating X for every item. The provider only inspected the parameter type (which yields no object class for `array`), so DTOs used exclusively this way were reported dead. Read the attribute's `type` argument as an additional DTO class source. Co-Authored-By: Claude Code
PropertyAccessor (via ReflectionExtractor::getWriteInfo) resolves a mutator purely by name and accessibility: a public set* method accepting one argument is invoked for a matching payload key, whether or not a backing property with the same name exists (e.g. a setter storing into an internal array). The provider required a same-named property to exist, leaving such setters reported as dead. Drop the property-existence requirement and instead mirror the vendor's isMethodAccessible() criteria: the setter must be public and callable with exactly one argument. This also stops marking zero-argument or non-public set* methods, which the vendor never calls. Co-Authored-By: Claude Code
For pluralizable payload keys, ReflectionExtractor::getWriteInfo() resolves an adder/remover pair (addTag()/removeTag()) before falling back to a setter, so collection-style DTOs are populated through those methods at runtime. The provider only recognized set* mutators, leaving adders and removers reported as dead. Matching the vendor, a lone adder without its remover counterpart is not used (the pair is required), and both methods must satisfy the same accessibility rules as setters. Co-Authored-By: Claude Code
PropertyAccessor reflects on the concrete DTO class, so public properties and mutators inherited from a parent class are populated the same way as own ones. The provider skipped members not declared directly on the DTO, which left members of project-level parent classes (not covered by the vendor usage provider) reported as dead when the parent was never itself used as a controller argument. Anchor the emitted usages to the declaring class so they resolve to the parent's members. Co-Authored-By: Claude Code
The provider marked every declared property of a payload DTO as written, but PropertyAccessor can only write public non-static non-readonly properties directly. Readonly and promoted properties are populated through the constructor and private ones only through mutators - both paths already produce visible or provider-emitted usages of their own. Over-marking suppressed real dead code: a private property with no setter and no constructor assignment is never populated by the serializer, yet its "never written" report was silenced. Co-Authored-By: Claude Code
The serializer denormalizes object-typed constructor parameters, writable properties and mutator arguments recursively, instantiating and populating nested DTOs exactly like the root one. The provider only processed the class of the controller argument itself, so members of nested DTOs were reported as dead. Collect usages recursively over natively-typed constructor parameters, writable properties and mutator parameters, guarding against type cycles with a visited set (same approach as #[MapInput] handling). Collections of nested DTOs declared only via PhpDoc (e.g. list<ItemDto>) are not traversed, as native reflection carries no element type. Co-Authored-By: Claude Code
janedbal
force-pushed
the
feature/symfony-map-request-payload
branch
from
July 13, 2026 20:51
27476ae to
cb9061c
Compare
The payload mapping feature grew enough test classes to warrant a dedicated fixture file, gated on symfony/http-kernel >= 6.3 where these attributes were introduced (the shared symfony.php fixture has no version guard). Co-Authored-By: Claude Code
The $type argument of #[MapRequestPayload] only exists since symfony/http-kernel 7.1. On older versions the attribute has no such constructor parameter, so no usage is emitted (correctly matching the runtime, where array payload mapping does not exist) and the shared fixture failed on PHP 8.1 (http-kernel downgraded to 6.4) and prefer-lowest jobs. Move the test case into a version-gated fixture. Co-Authored-By: Claude Code
testNoFatalError require_onces every fixture regardless of version gates, and the map-payload fixture declares an attribute extending MapRequestPayload, which is a load-time fatal when the parent class does not exist (prefer-lowest installs http-kernel 5.4). Skip the declarations via an early return when the class is missing; the corresponding testDead data set is already version-skipped there. Co-Authored-By: Claude Code
janedbal
marked this pull request as ready for review
July 14, 2026 07:56
The vendor's AbstractObjectNormalizer resolves attribute types through PropertyInfo, whose PhpDocExtractor reads @var/@param annotations - so an array constructor parameter or property annotated list<ItemDto> is denormalized element-by-element, instantiating and populating ItemDto. The nested-DTO recursion only followed native reflection types and stopped at `array`, leaving such element DTOs reported as dead. Use PHPStan's reflection (which merges PhpDoc and native types) for constructor parameters, writable properties and mutator arguments, and unwrap iterable value types recursively. Co-Authored-By: Claude Code
Member
Author
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
2 participants
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.
Problem
When a Symfony controller parameter uses
#[MapRequestPayload]or#[MapQueryString], the framework'sRequestPayloadValueResolveruses the serializer to create the DTO object. The DTO's constructor is called and its properties are populated — all invisible in user source code. This causes false positives where the DTO constructor, properties, and setter methods are reported as dead code.The serializer behavior depends on the normalizer used. With the default
ObjectNormalizer(registered at priority -1000 in Symfony's framework-bundle):setFoo(),addFoo()/removeFoo()pairs) are called via PropertyAccessorChanges
#[MapRequestPayload]and#[MapQueryString]attributes (including their subclasses, matching the vendor'sIS_INSTANCEOFlookup) on controller method parameters#[MapRequestPayload(type: X::class)]array payloads (attribute argument available since symfony/http-kernel 7.1) resolve toXReflectionExtractor::getWriteInfo()resolves them: publicset*methods callable with one argument (no backing property required) andadd*/remove*pairslist<ItemDto>), matching the vendor's PropertyInfo-based type resolutionKnown limitations
ObjectNormalizerpopulation strategy; custom denormalizers orPropertyNormalizer(which writes private properties via reflection) and#[DiscriminatorMap]subclass instantiation may still produce false positivesCo-Authored-By: Claude Code