[WIP] Feat: Type properties with PHP >= 7.4 #689

Superkooka · 2020-10-22T21:58:24Z

Edit: now this flag is global and you can defined if you want to use typing in the configuraiton file 🥳 Just one test dosn't pass.
When it's fix, this PR is ready to be merge.

Hi 👋
With PHP 7.4 and the arrival of PHP 8.0 typing is starting to gain importance. So it's why I have adding the --typed flag. This flag type the properties of your entity.

For exemple, with PHP 7.4 or more, if I create this entity, I obtain the PHP below:

PS D:\symfony\sf-maker> php .\bin\console make:entity --typed

 Class name of the entity to create or update (e.g. AgreeableKangaroo):
 > TypedPizza
TypedPizza

 created: src/Entity/TypedPizza.php
 created: src/Repository/TypedPizzaRepository.php

 Entity generated! Now let's add some fields!
 You can always add more fields later manually or by re-running this command.

 New property name (press <return> to stop adding fields):
 > isCooked

 Field type (enter ? to see all types) [boolean]:
 >


 Can this field be null in the database (nullable) (yes/no) [no]:
 >

 updated: src/Entity/TypedPizza.php

 Add another property? Enter the property name (or press <return> to stop adding fields):
 >



  Success!


 Next: When you're ready, create a migration with php bin/console make:migration

<?php

namespace App\Entity;

use App\Repository\TypedPizzaRepository;
use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity(repositoryClass=TypedPizzaRepository::class)
 */
class TypedPizza
{
    /**
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="boolean")
     */
    private ?bool $isCooked;

    public function getId(): ?int
    {
        return $this->id;
    }

    public function getIsCooked(): ?bool
    {
        return $this->isCooked;
    }

    public function setIsCooked(bool $isCooked): self
    {
        $this->isCooked = $isCooked;

        return $this;
    }
}

If I execute this command with a lower PHP version I have a confirmation warning to ask if I really want typed properties.
To run the test, this pr need PHP 7.4 inot the CI, see #690

weaverryan · 2020-10-23T00:45:57Z

Hi @Superkooka!

Ah, thank you for this! It's been on my mind lately!!! ❤️

2 quick things come to mind:

A) If someone is using PHP 7.4, when would they not want to use typed properties? I'm wondering about the option: do we need it? Or, do you "envision" the question as a "sanity" check to make sure they aren't accidentally using php 7.4 on a code base that will actually use 7.3 (for example) on production?

B) There are various other spots where we should also use typed properties. A simple - but probably not exhaustive - thing to check for is to search this bundle for addEntityField(). That method is also called in EntityRegenerator and UserClassBuilder. This is part of the reason why having an option on a command feels odd to me. We would need to add the option on other commands too. But really, you probably either want (or don't want) typed properties globally. So a global option (or just detecting PHP 7.4) might be better.

Thanks!

duboiss · 2020-10-23T08:07:22Z

Hey @weaverryan

Regarding behavior, what do you think of this? For all the commands that may need to generate class properties.

By checking the PHP required version in composer.json, if >= 7.4, we type automatically.
If this PHP required version is lower than 7.4 but the PHP interpreter version (in the path) is >= 7.4, asking for type.
If user says yes, warn him to update his composer.json with a minimum of 7.4.
Else, is that both versions are lower than 7.4, we don't type.

Second, for editing, what to base it on? Same behavior? Make:entity is the only command for "update" a php class ?

Next, yes it should concern the whole bundle and not just make:entity. What would you recommend for this at the files level ? A new "Util" file ?

Thanks ;)

innocenzi

Here are a few suggestions about the code style, and a few English fixes as well 👍

src/Maker/MakeEntity.php

src/Resources/help/MakeEntity.txt

src/Util/ClassSourceManipulator.php

tests/Maker/MakeEntityTest.php

src/Util/ClassSourceManipulator.php

tests/Maker/MakeEntityTest.php

Superkooka · 2020-11-01T13:03:36Z

To avoid all the problems with the different versions of php (<7.4) I decided to use a global flag (--typed) as well as the possibility of activating this flag by default in a configuration file as long as the version minimum PHP supported by the maker bundle will not be >= 7.4. In the following day(s) I will rebase and force push on my branch to keep a clean history of commits

feat: adding a configuration option to set the default typing

…mode.

Superkooka · 2020-11-19T18:36:05Z

Hi 👋 ! I don't have the time to fix for passing the CI, so if someone can fix it, I just have to squash the commit and this PR is ready to merge.

aleksblendwerk · 2020-12-13T12:47:49Z

Seems the tests pass so far but the whole job was just cancelled after 10 minutes before they all finished.

Could the tests be triggered again so we can see if there's still anything actually wrong?

Superkooka · 2020-12-13T18:11:18Z

Just look to the "CI / PHP 7.1.3 + symfony/skeleton@stable" job, the other are automatically cancel after.

aleksblendwerk · 2020-12-13T18:31:59Z

Just look to the "CI / PHP 7.1.3 + symfony/skeleton@stable" job, the other are automatically cancel after.

That 7.1.3 run has tests skipped that are OK on 7.4 though. I doubt 7.1 is the top priority here...

Superkooka · 2020-12-13T19:46:54Z

I think if the CI crash in 7.1, it's also crash on 7.4, the 7.1 is always supported and I don't know how to run only one test 😄

jrushlow · 2020-12-14T14:22:38Z

Hi @Superkooka great work so far! Just a heads up in PR #736 I've created a canUseTypedProperties(): bool method, https://github.com/symfony/maker-bundle/blob/44278a695902c491086ecc94e5d9a2bf1f1ccd80/src/Util/PhpCompatUtil.php

This is not merged yet but should be shortly. Once it's merged, I think rather than setting a flag - you'll be able to check the newly created $use_typed_properties global variable that will be available when creating templates. e.g.

duboiss mentioned this pull request Oct 23, 2020

make:entity ability to auto-add php 7.4 typing #669

Closed

Superkooka changed the title ~~Feat: adding the possibility to type properties on entity with --typed~~ Feat: Type properties with PHP >= 7.4 Oct 23, 2020

innocenzi suggested changes Oct 23, 2020

View reviewed changes

Superkooka changed the title ~~Feat: Type properties with PHP >= 7.4~~ [WIP] Feat: Type properties with PHP >= 7.4 Oct 23, 2020

jrushlow reviewed Oct 27, 2020

View reviewed changes

src/Util/ClassSourceManipulator.php Show resolved Hide resolved

jrushlow added the Status: Needs Work Additional work is needed label Oct 27, 2020

OskarStark reviewed Oct 28, 2020

View reviewed changes

tests/Maker/MakeEntityTest.php Outdated Show resolved Hide resolved

Superkooka added 7 commits November 2, 2020 22:05

feat: adding a global flag --typing to type properties 🤯

48d7077

feat: adding a configuration option to set the default typing

fix: bug, can't set default value when using InputOption::VALUE_NONE …

680400b

…mode.

feat: used yes/no question if php version < 7.4

27c2920

fix: typo

48036b7

fix: istyped ==> is_typed

7fc40eb

fix: typo (yes again :/)

15f69cd

fix: I hate me, typo again

d7e430e

weaverryan changed the base branch from master to main November 16, 2020 20:00

Superkooka closed this Dec 28, 2020

Superkooka mentioned this pull request Dec 31, 2020

feat: adding typing PHP 7.4 #773

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[WIP] Feat: Type properties with PHP >= 7.4 #689

[WIP] Feat: Type properties with PHP >= 7.4 #689

Uh oh!

Superkooka commented Oct 22, 2020 •

edited

Loading

Uh oh!

weaverryan commented Oct 23, 2020

Uh oh!

duboiss commented Oct 23, 2020 •

edited

Loading

Uh oh!

innocenzi left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Superkooka commented Nov 1, 2020

Uh oh!

Superkooka commented Nov 19, 2020 •

edited

Loading

Uh oh!

aleksblendwerk commented Dec 13, 2020

Uh oh!

Superkooka commented Dec 13, 2020

Uh oh!

aleksblendwerk commented Dec 13, 2020 •

edited

Loading

Uh oh!

Superkooka commented Dec 13, 2020

Uh oh!

jrushlow commented Dec 14, 2020

Uh oh!

Uh oh!

Uh oh!

[WIP] Feat: Type properties with PHP >= 7.4 #689

[WIP] Feat: Type properties with PHP >= 7.4 #689

Uh oh!

Conversation

Superkooka commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weaverryan commented Oct 23, 2020

Uh oh!

duboiss commented Oct 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

innocenzi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Superkooka commented Nov 1, 2020

Uh oh!

Superkooka commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aleksblendwerk commented Dec 13, 2020

Uh oh!

Superkooka commented Dec 13, 2020

Uh oh!

aleksblendwerk commented Dec 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Superkooka commented Dec 13, 2020

Uh oh!

jrushlow commented Dec 14, 2020

Uh oh!

Uh oh!

Superkooka commented Oct 22, 2020 •

edited

Loading

duboiss commented Oct 23, 2020 •

edited

Loading

Superkooka commented Nov 19, 2020 •

edited

Loading

aleksblendwerk commented Dec 13, 2020 •

edited

Loading