Skip to content

[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

Closed
wants to merge 1 commit into from

Conversation

malarzm
Copy link

@malarzm malarzm commented Dec 13, 2017

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:

includes:
    - vendor/phpstan/phpstan-doctrine/extension.neon
parameters:
    doctrine:
        metadata:
            CmsBundle\Entity:
                type: annotation
                paths: [%rootDir%/../../../src/CmsBundle/Entity]

@ondrejmirtes
Copy link
Member

Hi,
please fix the build and also write some tests.

@Majkl578
Copy link
Contributor

Majkl578 commented Jan 23, 2018

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.

@malarzm
Copy link
Author

malarzm commented Jan 23, 2018

@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
Copy link

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?

@ondrejmirtes
Copy link
Member

@ossinkine Definitely not 😊

@ondrejmirtes
Copy link
Member

Superseded by #49.

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

Successfully merging this pull request may close these issues.

4 participants