[make:entity] add php8 attribute support #920

SimonMarx · 2021-07-04T10:58:52Z

Hey everyone,

i saw the open issue #892 and i had the same problem.
Since my application is running on php8 and i do not want to create all my entities manually, i've added attribute support for the make:entity command.
The Annotation support is still available and (i think) untouched.

Things I was / am unsure about:

Before i made changes, the process checked if the class is annotated. I was unsure if i should add a new check isAttributed, or decide which kind of manipulation strategy the ClassSourceManipulator should use (annotation or attribute) depending on the given MappingDriver. As you can see in the code, i selected option number 2.
The method DoctrineHelper::isClassAnnotated is theoretically not needed anymore. But removing public methods is forbidden, do i have to flag/annotate them somehow in the doc blocks?
Commands like make:user does not support Attributes now, should that logic be implemented in this Pull Request?

To be honest, this is my first public pull request and im a bit nervous 😰 . I tried to follow all rules from the contributing guidelines, but maybe i missed some rules, so I am grateful for any feedback, tips, dos and don'ts.

Thanks in advance for your feedbacks and here is a screenshot of an generated entity with attributes 😃

This patch accounts for the negative return value of substr_compare when the two strings are totally different.

Required for addAttribute(). See: https://github.com/nikic/PHP-Parser/blob/master/CHANGELOG.md#version-4110-2021-07-03

…upport

davelima · 2021-07-28T22:32:26Z

Working fine to me! There is something i can do to help getting it merged? 🙌

dunglas

lgtm

src/Doctrine/DoctrineHelper.php

jrushlow · 2021-08-05T12:00:48Z

@SimonMarx can you rebase this against the main branch so we can see what the tests look like? thanks in advance!

adlpz · 2021-08-12T15:46:26Z

This change contains a bug in the Mapping Driver resolution that will return the wrong driver when multiple drivers are in place. I've submitted a pull request to @SimonMarx 's branch here: SimonMarx#2

I'd suggest getting that one through before rebasing and merging this one.

…-support Added attribute mapping detection

…upport Fix Mapping Driver guesser for new entities

weaverryan

Wow, tests are green and this is close! I've left some comments for things we should fix before merge - I think @jrushlow is going to keep pushing this across the finish line, but @SimonMarx you've been wonderful on this PR, so if you have some time, just coordinate with jrushlow so you don't both do the work. And thanks again!

src/DependencyInjection/CompilerPass/DoctrineAttributesCheckPass.php

src/Doctrine/DoctrineHelper.php

src/Maker/MakeEntity.php

src/Maker/MakeResetPassword.php

src/Maker/MakeUser.php

weaverryan · 2021-09-21T17:22:34Z

src/Maker/MakeUser.php

+            true,
+            !$useAttributesForDoctrineMapping,
+            true,
+            $useAttributesForDoctrineMapping


This constructor is complex, in large part because that nasty, confusing 3rd argument (which is my fault).

Anyways, I've seen this twice so far and it gave me an idea: add a DoctrineHelper::createClassSourceManipulator(string $className) method that handles all of this ugliness for us.

src/Resources/skeleton/doctrine/Entity.tpl.php

src/Test/MakerTestDetails.php

src/Maker/MakeEntity.php

weaverryan · 2021-09-24T13:50:41Z

@SimonMarx I noticed there are 2 of other committers on this PR (besides @jrushlow and I). Can you explain who they are?

~~@Geekimo~~ Ah, I see this comes from feat: make:entity php 8 attributes support #926 !
~~@adlpz~~ And this comes from Fix Mapping Driver guesser for new entities SimonMarx/maker-bundle#2

~~Were you collaborating with these users?~~

I figured it out 😄

weaverryan · 2021-09-24T14:09:55Z

@SimonMarx - Also, see #978 for a cleaned up version of this PR, I need your "ok" on that PR (~~as well as determining where the other contributors came from~~).

Thanks!

This PR was merged into the 1.0-dev branch. Discussion ---------- Adding Entity attribute support Hi! This is #920 cleaned up. Somehow, in that PR, the commits got tanged. For example, some commits were done twice, like SimonMarx@d12c2f9 and SimonMarx@3848f4f - but the bigger problem is that some commits (like the 2nd listed) have one date on GitHub (e.g. Aug 12th) but another date when pulled down locally (July 4th). Whatever is causing this seems to also cause, during a rebase, for these commits to be rebased on reverse. In short, it was an odd mess. To fix this, I've created a single commit with co-authors. I need to verify that this is ok with `@SimonMarx` and determine where these other two contributors came from :) (see #920 (comment)) Cheers! ``` Co-authored-by: Simon Marx <[email protected]> Co-authored-by: Morgan ABRAHAM <[email protected]> Co-authored-by: Adria Lopez <[email protected]> Co-authored-by: Jesse Rushlow <[email protected]> ``` Commits ------- 9ecc684 Adding Entity attribute support

Simon Marx and others added 7 commits July 4, 2021 10:33

add attribute support

d12c2f9

add exception when expr node build failed

dd4cf98

add accidantily removed addDriver check

ede1cd2

add doc blocks

f8b3faa

remove unused private method, remove unecessary doc blocks

b9f701b

cs fixer run

32bec7f

Added attribute mapping detection

44cf99d

Geekimo mentioned this pull request Jul 13, 2021

feat: make:entity php 8 attributes support #926

Closed

Geekimo and others added 5 commits July 14, 2021 00:22

CS fix

1f30ddc

Mapping type check condition operator fix

8cd167c

Fix Mapping Driver guesser for new entities

1a8e1a8

This patch accounts for the negative return value of substr_compare when the two strings are totally different.

Bumped php-parser requirement to 4.11

aa97440

Required for addAttribute(). See: https://github.com/nikic/PHP-Parser/blob/master/CHANGELOG.md#version-4110-2021-07-03

Merge branch 'symfony:main' into feature/php8-make-entity-attribute-s…

f2aab49

…upport

dunglas approved these changes Aug 3, 2021

View reviewed changes

src/Doctrine/DoctrineHelper.php Outdated Show resolved Hide resolved

jrushlow added the Status: Waiting Feedback Needs feedback from the author label Aug 5, 2021

jrushlow linked an issue Aug 5, 2021 that may be closed by this pull request

Add support for PHP 8 Attributes to make:entity #892

Closed

SimonMarx and others added 11 commits August 12, 2021 21:03

Merge pull request #1 from Geekimo/feature/php8-make-entity-attribute…

a96ac57

…-support Added attribute mapping detection

Merge pull request #2 from adlpz/feature/php8-make-entity-attribute-s…

19d63d4

…upport Fix Mapping Driver guesser for new entities

add attribute support

3848f4f

add exception when expr node build failed

7fb08c0

add accidantily removed addDriver check

9b1e19b

add doc blocks

1908b36

remove unused private method, remove unecessary doc blocks

5c74cf8

cs fixer run

d3c1979

Added attribute mapping detection

a3560d0

CS fix

8d57702

Mapping type check condition operator fix

a11837f

weaverryan and others added 13 commits September 20, 2021 15:25

fixing phpcs

a96660c

use phpcompat util in helper

e2b2111

add attribute tests for csm

f164c8a

add many to one attribute tests

9139ad3

add one to many attribute tests

e0119c1

add many to many attribute tests

0c1cbe6

add one to one attribute tests

40e8f53

handle legacy user empty

1fecf75

handle user w/ relation

0c01e40

handle user profile

b82efe2

handle user with use statements

c186f22

handle legacy cases

d82cc32

cleanup

c7e3900

jrushlow added Status: Needs Review Needs to be reviewed and removed Status: Waiting Feedback Needs feedback from the author labels Sep 21, 2021

weaverryan reviewed Sep 21, 2021

View reviewed changes

jrushlow added 3 commits September 22, 2021 14:29

additional cleanup

bd7ed43

php-cs-fixer it up

8ac83e8

WIP - refactor createCSM logic

74bd5bc

weaverryan reviewed Sep 22, 2021

View reviewed changes

src/Maker/MakeEntity.php Outdated Show resolved Hide resolved

jrushlow added 2 commits September 22, 2021 15:47

fix whooops

6559669

cleanup

88b8954

jrushlow mentioned this pull request Sep 22, 2021

Rely on defaults for doctrine mapping types symfony/recipes#971

Merged

refactor exception msg

bb2baba

weaverryan mentioned this pull request Sep 24, 2021

Adding Entity attribute support #978

Merged

weaverryan closed this Sep 27, 2021

jrushlow added the Feature New Feature label May 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[make:entity] add php8 attribute support #920

[make:entity] add php8 attribute support #920

SimonMarx commented Jul 4, 2021

davelima commented Jul 28, 2021

dunglas left a comment

jrushlow commented Aug 5, 2021

adlpz commented Aug 12, 2021

weaverryan left a comment

weaverryan Sep 21, 2021

weaverryan commented Sep 24, 2021 •

edited

Loading

weaverryan commented Sep 24, 2021 •

edited

Loading

[make:entity] add php8 attribute support #920

[make:entity] add php8 attribute support #920

Conversation

SimonMarx commented Jul 4, 2021

davelima commented Jul 28, 2021

dunglas left a comment

Choose a reason for hiding this comment

jrushlow commented Aug 5, 2021

adlpz commented Aug 12, 2021

weaverryan left a comment

Choose a reason for hiding this comment

weaverryan Sep 21, 2021

Choose a reason for hiding this comment

weaverryan commented Sep 24, 2021 • edited Loading

weaverryan commented Sep 24, 2021 • edited Loading

weaverryan commented Sep 24, 2021 •

edited

Loading

weaverryan commented Sep 24, 2021 •

edited

Loading