Support ORM's embeddables #108

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

Jump to bottom

Open

malarzm wants to merge 2 commits into phpstan:1.3.x

from malarzm:embeddables

Open

Support ORM's embeddables #108

malarzm wants to merge 2 commits into phpstan:1.3.x from malarzm:embeddables

Conversation

malarzm

Copy link

@malarzm malarzm commented Jan 27, 2020 •

edited

Loading

Today at work I've realized there's currently no rules for ORM's embeddables so here's an attempt to change that :) I'll remove WIP once I'm done with my goals, for now I'm making a PR for early feedback (or to learn that somebody else is already working on this).

Goals:

Check if property's type agrees with ORM's knowledge
Check columns specified in embeddables just like for entities

malarzm

malarzm commented

Jan 27, 2020

View reviewed changes

src/Rules/Doctrine/ORM/EntityColumnRule.php Outdated Show resolved Hide resolved

malarzm

malarzm commented

Jan 28, 2020

View reviewed changes

src/Rules/Doctrine/ORM/EntityColumnRule.php

@@ -64,9 +68,6 @@ public function processNode(Node $node, Scope $scope): array

}

$className = $class->getName();

if ($objectManager->getMetadataFactory()->isTransient($className)) {

Copy link

Author

@malarzm malarzm Jan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to figure out internally why the embeddables are considered transient, but removal of this check causes no test failures

Copy link

Author

@malarzm malarzm Jan 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also classes not managed by Doctrine will have an exception thrown a bit later while trying to get their metadata and will return early from the rule with no errors, so I believe the intent is preserved

Copy link

Author

@malarzm malarzm Jan 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created an issue in ORM but I think we can continue without waiting for its resolution doctrine/orm#8006

@malarzm malarzm force-pushed the embeddables branch from 4a4149f to b99ae3d Compare

January 28, 2020 21:41

@malarzm malarzm changed the title ~~(削除) [WIP] Support ORM's embeddables (削除ここまで)~~ (追記) Support ORM's embeddables (追記ここまで)

Jan 28, 2020

@malarzm malarzm force-pushed the embeddables branch from b99ae3d to c7775c3 Compare

January 30, 2020 19:12

@malarzm malarzm mentioned this pull request

Jan 30, 2020

Make Embeddable not transient doctrine/orm#8006

Merged

@malarzm malarzm force-pushed the embeddables branch from c7775c3 to 121e460 Compare

January 30, 2020 19:27

malarzm added 2 commits

January 30, 2020 20:34

@malarzm


 Check if embedded class matches property's type

@malarzm


 Inspect columns in embeddables

5a8e7c3

@malarzm malarzm force-pushed the embeddables branch from 121e460 to 5a8e7c3 Compare

January 30, 2020 19:35

@malarzm

Copy link

Author

malarzm commented Jan 30, 2020

PR is now ready :)

@malarzm malarzm requested a review from lookyman

February 7, 2020 12:30

@malarzm

Copy link

Author

malarzm commented Feb 24, 2020

Anybody? @lookyman @ondrejmirtes ?

@ondrejmirtes

Copy link

Member

ondrejmirtes commented Feb 24, 2020

Hi, sorry for keeping you waiting, I'll look into this when I have the time.

@ondrejmirtes ondrejmirtes force-pushed the master branch 4 times, most recently from b2b2414 to 1fe21ab Compare

December 13, 2020 11:54

@ondrejmirtes ondrejmirtes force-pushed the master branch from bca5b04 to 24f7283 Compare

June 9, 2021 14:38

@enumag

Copy link

Contributor

enumag commented Nov 17, 2021

Any news on this one? It would be helpful imo. :-)

@kissifrot

Copy link

kissifrot commented Jan 19, 2022

Looks like this PR has conflicts, but it would be helpful to have it merged :)

@ondrejmirtes ondrejmirtes force-pushed the 1.3.x branch 2 times, most recently from aa4e98e to 8b28264 Compare

May 25, 2023 13:40

Labels

None yet

4 participants

@malarzm @ondrejmirtes @enumag @kissifrot

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support ORM's embeddables #108

Are you sure you want to change the base?

Support ORM's embeddables #108

Uh oh!

Conversation

@malarzm malarzm commented Jan 27, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

@malarzm malarzm Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

@malarzm malarzm Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

@malarzm malarzm Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

malarzm commented Jan 30, 2020

Uh oh!

malarzm commented Feb 24, 2020

Uh oh!

ondrejmirtes commented Feb 24, 2020

Uh oh!

enumag commented Nov 17, 2021

Uh oh!

kissifrot commented Jan 19, 2022

Uh oh!

Uh oh!

Support ORM's embeddables #108

Are you sure you want to change the base?

Support ORM's embeddables #108

Uh oh!

Conversation

@malarzm malarzm commented Jan 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

@malarzm malarzm Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

@malarzm malarzm Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

@malarzm malarzm Jan 30, 2020

Choose a reason for hiding this comment

Uh oh!

malarzm commented Jan 30, 2020

Uh oh!

malarzm commented Feb 24, 2020

Uh oh!

ondrejmirtes commented Feb 24, 2020

Uh oh!

enumag commented Nov 17, 2021

Uh oh!

kissifrot commented Jan 19, 2022

Uh oh!

Uh oh!

@malarzm malarzm commented Jan 27, 2020 •

edited

Loading