Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symfony (de)normalizer and supportsNormalization return type #410

Open
jdeniau opened this issue Oct 3, 2024 · 0 comments
Open

Symfony (de)normalizer and supportsNormalization return type #410

jdeniau opened this issue Oct 3, 2024 · 0 comments

Comments

@jdeniau
Copy link
Contributor

jdeniau commented Oct 3, 2024

When using a custom symfony normalizer as defined in the documentation, the first parameter of the normalize method is the same as the return type of the supportsNormalization method.

Let's take the documentation example

namespace App\Serializer;

use App\Entity\Topic;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;

class TopicNormalizer implements NormalizerInterface
{
    public function __construct(
        #[Autowire(service: 'serializer.normalizer.object')]
        private readonly NormalizerInterface $normalizer,

        private UrlGeneratorInterface $router,
    ) {
    }

    public function normalize($topic, ?string $format = null, array $context = []): array
    {
        $data = $this->normalizer->normalize($topic, $format, $context);

        // Here, add, edit, or delete some data:
        $data['href']['self'] = $this->router->generate('topic_show', [
            'id' => $topic->getId(),
        ], UrlGeneratorInterface::ABSOLUTE_URL);

        return $data;
    }

    public function supportsNormalization($data, ?string $format = null, array $context = []): bool
    {
        return $data instanceof Topic;
    }

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

$topic is an instance of Topic here. In order to help phpstan report issue, we can add the annotation like this:

+     /**
+      * @param Topic @topic
+      */
      public function normalize($topic, ?string $format = null, array $context = []): array
      {

By doing this, we can call methods of Topic on $topic.
But if we apply phpstan's strict rule, and in particular if we activate the reportMaybesInMethodSignatures: true parameter, then phpstan reports this error:

Parameter #1 $object (Topic) of method App\Serializer\TopicNormalizer::normalize() should be contravariant with parameter $object (mixed) of method Symfony\Component\Serializer\Normalizer\NormalizerInterface::normalize() 

And it is totally right, we should not guess the type of the child method variance.
But in that case, Symfony does garanty the type due to the supportsNormalization method.

A possible solution without the symfony extension would be to have "something" like this (inspired by Typescript notation):

    /**
     * @var Parameters<typeof self::supportsNormalization>[0] $topic
     */
   public function normalize($topic, ?string $format = null, array $context = []): array
    {
        $data = $this->normalizer->normalize($topic, $format, $context);

        // Here, add, edit, or delete some data:
        $data['href']['self'] = $this->router->generate('topic_show', [
            'id' => $topic->getId(),
        ], UrlGeneratorInterface::ABSOLUTE_URL);

        return $data;
    }

    /**
     * @phpstan-assert-if-true Topic $data
     */
    public function supportsNormalization($data, ?string $format = null, array $context = []): bool
    {
        return $data instanceof Topic;
    }

But:
I do not know if it's possible to do than with phpstan. I know that you can infer the type of a prop with $this->xxx : https://phpstan.org/writing-php-code/narrowing-types#equality-assertions, but I don't think that you can set the parameter type of another function, maybe with Dynamic Return Type extension ?
If phpstan-symfony does handle that, it should override the covariance rule. It's probably 99% the case in any Symfony normalizer, but nothing forbid you to call TopicNormalizer::normalize from anywhere in your code without calling supportsNormalization (but the DX for the 99% would be really better !)

If type can be inferred, the perfect way of writing this normalizer would be to change nothing to the example:

    // @topic type is infered from the assertion on `supportsNormalization
    public function normalize($topic, ?string $format = null, array $context = []): array
    {
		// …
    }

    public function supportsNormalization($data, ?string $format = null, array $context = []): bool
    {
		// we have here a case of "basic" type inference, so `supportsNormalization` should know that $data is a Topic
        return $data instanceof Topic;

		// if we have weirder case, it might be recognize as a Union type, for example:
		return $data instanceof Topic || is_string($data); // type may be `Topic|string`
    }

If you think that it is doable, I can spent some time implementing this. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant