-
Notifications
You must be signed in to change notification settings - Fork 112
[PoC] Resolve custom repository classes #18
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
Conversation
Hi,
please fix the build and also write some tests.
Thinking ahead of time: Since this repository is generically for PHPStan Doctrine extension, how would ORM/ODM support be handled here? Or other Doctrine packages like Collections? Because i.e.:
parameters:
doctrine:
metadata:
and the code in this PR look quite ORM-specific right now.
Also how multiple versions would be handled? In not too distant future there will be need for ORM 2 & ORM 3 + ODM 1 & ODM 2 parallel support, but each of them are going to have incompatibilities.
Also there will be no Common 3.0.
@Majkl578 very valid points! That's why it was proof of concept only ;)
how would ORM/ODM support be handled here?
I assumed that phpstan-doctrine is ORM oriented but indeed, basic stuff would work with ODM as well. Theoretically the metadata could look like this:
metadata:
CmsBundle\Entity:
doctrine: orm # possibly would be the default?
type: annotation
paths: [%rootDir%/../../../src/CmsBundle/Entity]
another approach could be to infer whether classes are designed for ORM or ODM by looking at provided mapping/annotation in classes.
Also how multiple versions would be handled?
Easiest would be versioning - v1 of the plugin requiring ORM 2 and next version for ORM 3. Also depending on the number of breaks the plugin itself could provide a compatibility layer for metadata it needs and avoid major version bump. With ODM it'll be a bit easier as we do not aim to break mapping stuff just yet.
ossinkine
commented
Sep 11, 2018
I'm not well acquainted with the PHPStan code, but is not it enough to replace new MixedType with regular type detection (from docblocks, typehints and so on, in any case, custom repositories should contain such information) in EntityRepositoryDynamicReturnTypeExtension.php#L40?
@ossinkine Definitely not 😊
Superseded by #49.
Uh oh!
There was an error while loading. Please reload this page.
This is only a proof of concept, has some rough edges (including CS) and obviously needs tests. FWIW it does work :) I'm opening this PR to see whether such approach would be accepted to not waste time.
Motivation behind creating dummy entity manager is that we get support for all mapping formats out of the box and don't need to resolve mappings on our own. With this we'll be able to get same mappings as Doctrine has so it opens possibilities to more checks.
Example of configuration: