Hey!

Thanks for your PR. You are targeting branch "7.3" but it seems your PR description refers to branch "TBH I am not sure about this one 😅".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

I already saw that this causes some issues... But instead of fixing all of that now and occupying github action pipelines I'll wait for your feedback to see if you would accept this PR.

Looking at the normalizers shipped in Symfony, they all verify the passed data tonormalize()/denormalize() is indeed what is expected (and throw a serializer exception otherwise)

So to me, it sounds like it is a good practice to verify the type in the method itself rather than relying on the return value ofsupportsNormalization()/getSupportedTypes()?

Looking at the normalizers shipped in Symfony, they all verify the passed data tonormalize()/denormalize() is indeed what is expected (and throw a serializer exception otherwise)

@wouterj I just checked two normalizers randomly and that doesn't seem to be the case (unless I misunderstood your point):

• symfony/src/Symfony/Component/Serializer/Normalizer/ConstraintViolationListNormalizer.php

  Line 46 in23fd2eb

  publicfunctionnormalize(mixed$object, ?string$format =null,array$context = []):array

  (line 62 assumesmixed $object is a list of violations - but we know it should beConstraintViolationListInterface)

• symfony/src/Symfony/Component/Messenger/Transport/Serialization/Normalizer/FlattenExceptionNormalizer.php

  Line 29 in36a2bb1

  publicfunctionnormalize(mixed$object, ?string$format =null,array$context = []):array

  - same as previous case,mixed $object is assumed to be aFlattenException instance.

I don't know how/if psalm/phpstan is set up, but in either case I would have expected a warning that the usage of$object was not expected (first case not iterable, second case undefined symbol getMessage() etc).

With this PR (and some small adjustments), static analysis tools should figure out$object's type, and modern IDEs should be able to provide autocompletion withnormalize() (and possiblydenormalize() - but that's not covered in this PR - yet) - (without having to redeclare the PHPDoc for(de)normalise()).

People should still be able to opt-out by using@implements NormalizerInterface<mixed>.

Interesting, so we are inconsistent about this in Symfony. See e.g.

I have to admit that this is kind of an edge case. It is not absolutely safe to just assume the type passed tonormalize is the one we check for insupportsNormalization, since somebody might as well instantiate aValueObjectNormalizer on their own and callnormalize without callingsupportsNormalization before.

I.e. this is probably how Symfony is calling this under the hood:

$valueObjectNormalizer =newValueObjectNormalizer();if ($valueObjectNormalizer->supportsNormalization($value)) {$valueObjectNormalizer->normalize($value);}

But nothing stops a developer from instantiating that manually and call it in the "wrong" way:

$valueObjectNormalizer =newValueObjectNormalizer();$valueObjectNormalizer->normalize($value);

Nevertheless, I would say this is an edge case, and at least for me the current PR would still be a nice addition, since it is closer to how this is actually working under the hood.