Skip to content

[PropertyInfo][Serializer] Add limited generics support to PhpStanExtractor #47556

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

Closed
wants to merge 1 commit into from

Conversation

tigitz
Copy link
Contributor

@tigitz tigitz commented Sep 12, 2022

Q A
Branch? 6.2
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #45071
License MIT
Doc PR

This PR aims to provides a limited support of generics serialization with the help of PhpStanExtractor.

Basically achieving:

class Foo {
    /**
     * @param Generic<\stdClass> $bar
     */
    public function __construct(public $bar){}
}

/**
 * @template T
 */
class Generic {
    /**
     * @param T $variableType
     */
    public function __construct(public $variableType){}
}

$dummy = new \stdClass();
$dummy->prop = 'foo';

$normalizers->normalize(new Foo(bar:new Generic($dummy)));
// ['bar' => ['variableType' => ['prop' => 'foo']]]

$normalizers->denormalize(['bar' => ['variableType' => ['prop' => 'foo']]]);
// new Foo(bar:new Generic($dummy))

It introduces an internal normalization_outer_class_property key in the normalization context to provide the property extractor the type found in the generic.

Its keeping BC with iterables types documented as generics (e.g. array<>, Collection<>)

It's an early stage feature as generics can be very flexible and I encourage everyone to please test it on their own projects and help me catch all the remaining use cases.

It's limited in the sense that it doesn't support:

  • Nested generics
  • Doesn't support @implements Collection<T>

Debatable topics:

  • normalization_outer_class_property key naming
  • Should it requires an opt-in / opt-out mechanism ? As it stands, for each OuterObject->InnerObject normalizations there's an added key/value in the context. Hopefully it won't be a huge performance impact but maybe we could let users opting-out.
  • I've refactored a bit the handling of promoted property type extraction so that, if both present, @var type will take precedence over @param type.

To Do

  • Update the ContextBuilder ?

@tigitz tigitz requested a review from dunglas as a code owner September 12, 2022 13:36
@carsonbot carsonbot added this to the 6.2 milestone Sep 12, 2022
@carsonbot carsonbot changed the title [Serializer][PropertyInfo] Add limited generics support to PhpStanExtractor [PropertyInfo][Serializer] Add limited generics support to PhpStanExtractor Sep 12, 2022
@tigitz
Copy link
Contributor Author

tigitz commented Sep 12, 2022

@nicolas-grekas Thanks again for giving me the push I needed to complete and polish my POC for this PR. OSS dynamics FTW as you say :)

It now seems much less insurmountable to contribute 🚀.

@carsonbot
Copy link

Hey!

I think @Korbeil has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@nicolas-grekas nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@norkunas
Copy link
Contributor

Could someone review this? :)

@chalasr
Copy link
Member

chalasr commented Mar 29, 2025

@mtarld is this needed now that we have TypeInfo?

@mtarld
Copy link
Contributor

mtarld commented Mar 31, 2025

Indeed, we don't need it anymore, as the PhpStanExtractor now relies on TypeInfo's StringTypeResolver, which reads generics properly.

@mtarld
Copy link
Contributor

mtarld commented Mar 31, 2025

But, if we want to make it work with the Serializer, there is some other work to do. Which can be done in another PR IMO

Thanks for the work done here! 🙂

@mtarld mtarld closed this Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PropertyInfo] Add support for generics
8 participants