Extend DenormalizerInterface.stub gate to cover Symfony 7.4+ (LSP errors on per-type denormalizer implementations)

[oksidisko]

Summary

Implementing DenormalizerInterface with a single concrete return type now triggers method.childReturnType (and, if narrowing @param to compensate, method.childParameterType) errors that have no clean PHPDoc resolution. The errors are caused by the conditional return type that has been part of Symfony's DenormalizerInterface since 7.4. phpstan-symfony already ships a stub that resolves this — it just isn't loaded for Symfony ≥ 7.4.

Environment

• phpstan/phpstan: 2.1.56
• phpstan/phpstan-symfony: 2.0.18
• symfony/serializer: 8.0.10
• PHP: 8.5

Reproduction

A typical per-type denormalizer:

use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;

class DateRangeNormalizer implements DenormalizerInterface
{
    public function denormalize(mixed $data, string $type, ?string $format = null, array $context = []): DateRange
    {
        return new DateRange(/* ... */);
    }

    public function supportsDenormalization(mixed $data, string $type, ?string $format = null, array $context = []): bool
    {
        return $type === DateRange::class;
    }

    public function getSupportedTypes(?string $format): array
    {
        return [DateRange::class => true];
    }
}

PHPStan (level 8) reports:

Return type (DateRange) of method ...DateRangeNormalizer::denormalize() should be covariant
with return type (($type is class-string<object> ? object : mixed)) of method
Symfony\Component\Serializer\Normalizer\DenormalizerInterface::denormalize()
  identifier: method.childReturnType

The natural attempt to fix it — narrowing the parameter to bind TObject:

/** @param class-string<DateRange> $type */
public function denormalize(mixed $data, string $type, ...): DateRange

produces a second error:

Parameter #2 $type (class-string<DateRange>) of method ...::denormalize() should be
contravariant with parameter $type (string) of method DenormalizerInterface::denormalize()
  identifier: method.childParameterType

The two requirements (bind TObject to satisfy covariance, keep param as wide as string for contravariance) are mutually exclusive in PHPStan's LSP model. There is no PHPDoc that satisfies both.

Root cause

Since Symfony 7.4, DenormalizerInterface::denormalize() is annotated:

/**
 * @template TObject of object
 * @param class-string<TObject>|string $type
 * @return ($type is class-string<TObject> ? TObject : mixed)
 */

The conditional return is great for callers of Serializer::denormalize($data, Foo::class) (return resolves to Foo), but for implementations of the interface — which are per-type and rely on supportsDenormalization() for runtime dispatch — the conditional cannot be honored in general by a single concrete return type.

phpstan-symfony already has the right workaround: a stub at stubs/Symfony/Component/Serializer/Normalizer/DenormalizerInterface.stub that flattens @return to mixed. That stub is loaded at src/Stubs/Symfony/StubFilesExtensionLoader.php:125:

if ($this->isInstalledVersionBelow('symfony/serializer', '7.4.0.0')) {
    $files[] = $stubsDir . '/Symfony/Component/Serializer/Normalizer/DenormalizerInterface.stub';
}

The conditional return type has been present in the source since 7.4, so the gate excludes exactly the versions where the stub is needed.

Proposed fix

Either remove the version gate entirely, or bump it to cover currently-supported Symfony versions (e.g. '9.0.0.0'):

- if ($this->isInstalledVersionBelow('symfony/serializer', '7.4.0.0')) {
+ if ($this->isInstalledVersionBelow('symfony/serializer', '9.0.0.0')) {
      $files[] = $stubsDir . '/Symfony/Component/Serializer/Normalizer/DenormalizerInterface.stub';
  }

The existing stub content already does the right thing; it just isn't being applied.

Workaround in the meantime

Users can suppress the diagnostic per-method:

/** @phpstan-ignore method.childReturnType */
public function denormalize(...): DateRange

but that scales poorly across codebases with many normalizers.

[VincentLanglet]

cc @stof which added the isInstalledVersionBelow check

[stof]

Which stub is it that solves an issue with the upstream Symfony types ? And why wasn't this issue reported to Symfony to actually fix the upstream types instead of silently patching them (which then ignores any improvement done by the Symfony team until someone copies them to the stubs) ?

[stof]

Btw, I don't even think a stub was added here to fix the upstream type. This mixed type was copying the upstream type of older Symfony version at the time the stub was created. And the presence of this type in the stub has hidden the issue caused by the new upstream type because phpstan never used the new type when using the extension.

[oksidisko]

@stof

Fair on (2) -- the stub is a stale mirror, not a fix, and extending the gate would freeze an obsolete annotation over Symfony's current one. Wrong remedy; I will close this here.

On (1), I agree this is probably upstream rather than phpstan-symfony. The issue I am seeing is that the current DenormalizerInterface annotation works as a caller-facing promise for the aggregate Serializer, but is too strong as an implementer-facing contract for per-type denormalizers.

Minimal example:

use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;

final class Number {}

final class NumberDenormalizer implements DenormalizerInterface
{
    public function denormalize(mixed $data, string $type, ?string $format = null, array $context = []): Number
    {
        return new Number();
    }
}

With strict method-signature checks enabled, PHPStan reports:

Return type (Number) of method NumberDenormalizer::denormalize() should be covariant with
return type (($type is class-string<object> ? object : mixed)) of method
Symfony\Component\Serializer\Normalizer\DenormalizerInterface::denormalize()
method.childReturnType

So before I raise this on symfony/symfony: what do you think the upstream ask should be?

1. Should DenormalizerInterface have a looser return annotation than Serializer, because the interface is implemented by per-type denormalizers?
2. Or is the current annotation intentional, and strict static analysis users should suppress method.childReturnType on precise per-type implementations?

[stof]

I don't know (yet) what should be the proper way to define types for that, to handle both the Serializer usage and the per-type denormalizer implementations. But this discussion should happen in the Symfony issue tracker IMO.

The fact that the current annotation causes issues for implementors of the interface was a shortcoming IMO, not taken into account in the PR adding the new type (and hidden in many projects by the stale stub in phpstan-symfony)