Find a way for PHP attribute generator to match Symfony coding standards

0 votes
asked Sep 5, 2022 in Feature Request by micamou (170 points)
edited Sep 5, 2022 by micamou

In our quality tools, we enforce respecting Symfony's coding standards by runnig PHP CS fixer with one rule set : https://cs.symfony.com/doc/ruleSets/Symfony.html

When running CS fixer on Skipper generated Entities with attributes, the fixer alters the attributes on 4 different rules (see bellow).

When running Skipper generation again, the attributes are regenerated with Skipper's rules.

example of the "issue"

Skipper generated :

#[ORM\Table(name: "table_1",
        options: [
            "comment"=>"Long comment multiline",
            "row_format"=>"XY"
        ])]

#[ORM\Table(name: "table_2",
        options: ["comment"=>"Short comment","row_format"=>"XY"])]

PHP CS fixer "fixes" :

#[ORM\Table(name: 'table_1',
        options: [
            'comment'   => "Long comment multiline",
            'row_format'=> 'XY',
        ])]


#[ORM\Table(name: 'table_2',
        options: ['comment'=>'Short comment', 'row_format'=>'XY'])]

Propositions

A. Add a configuration to Skipper to run a post-generation script per Entity passing the Class as argument, like this

$ php vendor/bin/php-cs-fixer fix src/Entity/Person.php

B. Add a configuration to setup Skipper to respect the 4 rules bellow

Rules for reference

single_quote
https://cs.symfony.com/doc/rules/string_notation/single_quote.html

whitespaceaftercommainarray
https://cs.symfony.com/doc/rules/array_notation/whitespace_after_comma_in_array.html

trailingcommain_multiline
https://cs.symfony.com/doc/rules/control_structure/trailing_comma_in_multiline.html

binaryoperatorspaces
https://cs.symfony.com/doc/rules/operator/binary_operator_spaces.html

Side quest A

targetEntity could probably use ::class constant instead of explicit FQDN

#[ORM\ManyToMany(targetEntity: "App\Entity\Stakeholder\Organization", mappedBy: 'activities')]

could probably be replaced by

#[ORM\ManyToMany(targetEntity: Organization:class, mappedBy: 'activities')]

given the namespaces matches or the related entity is imported
but I am not sure about it and haven't tested it for now.

*To be honest, it is PHP Rector who proposed this fix when it ran it with this config https://getrector.org/blog/how-to-upgrade-annotations-to-attributes

Side quest B

named arguments order should match parameters order

Example:

#[ORM\ManyToMany(targetEntity: Organization:class, mappedBy: 'activities')]

should be

#[ORM\ManyToMany(mappedBy: 'activities', targetEntity: Organization:class)]

according to \Doctrine\ORM\Mapping\OneToMany::__construct

new OneToMany($mappedBy, $targetEntity, $cascade, $fetch, $orphanRemoval, $indexBy)
commented Sep 5, 2022 by ludek.vodicka Skipper developer (140,450 points)

Thanks for the detailed report. I believe we will be able to update exported to meet all requirements.

I will let you know when the update will be ready.

commented Sep 5, 2022 by micamou (170 points)

Sorry, I answered the question to add detail instead of editing the original post.
I have added 2 side quests A, and B which I discovered later and have far less impact.

Thank you again

commented Sep 5, 2022 by ludek.vodicka Skipper developer (140,450 points)

Thanks for adding. I can't promise that everything will be possible but we will do our best ;-)

commented Sep 6, 2022 by ludek.vodicka Skipper developer (140,450 points)

After the discussion with our team and our external framework mentors, here the possible ways and our thoughts:

Main issue

This is something we discussed a lot. We also already implemented the quotation rule but after that, we decided/realized that we can't push such change to production. The reason is that such a change would cause a lot of changes in all our customer's projects which means unnecessary confirmation/validation from their side.

Because of that we decided to rollback these changes and stick with the current export format and go with the second way (A), and implement a way how to execute automatic scripts after the export.

After a discussion we believe we can implement it even more universal and introduce automatic after-export steps which would allow to execute scripts when:

  • any file is exported
  • a whole module is exported
  • a whole project is exported

This would bring a lot of new possibilities to the skipper and also solve the main issue with the formatting.

Side quest A

This is doable, but the question is if it's necessary. The full namespace path is more explicit and we didn't find any recommendation/rule to use a short path if necessary. And because Skipper exports are trying to go with the rule "as much explicit as possible", I believe full path here is a better choice.

Side quest B

We're still considering this, but because even the Doctrine2 documentation doesn't meet these rules, we're not sure if this is really necessary (https://www.doctrine-project.org/projects/doctrine-orm/en/2.13/reference/attributes-reference.html#onetomany).

The issue with such change is the same like with the main issue. This change would mean a lot of changes in our customers projects which could mean a lot of manual work for the change confirmations because changes couldn't be visible/obvious at the first sight when comparing old/new versions.

commented Sep 6, 2022 by ludek.vodicka Skipper developer (140,450 points)

Based on our first prototype, we're extending existing external tool support, but these "external tools" will be stored together with project and module (because of sharing modules).

Till now, we identified these new substitution marks for file export:

%OUTPUT_FILE%
%OUTPUT_FILE_PATH%
%OUTPUT_FILE_NAME%
%OUTPUT_FILE_EXT%
%OUTPUT_FILE_NAME_WITHOUT_EXT% 

And these for project export:

%OUTPUT_DIRECTORY%"

And both exports will have also access to these common marks:

%PROJECT_NAME%
%PROJECT_DIRECTORY%
%PROJECT_MAIN_FILE%
%ORM_NAME%
%MVC_NAME%

Do you have any idea what other marks should be necessary?

commented Sep 7, 2022 by ludek.vodicka Skipper developer (140,450 points)

Almost ready

Skipper external tools

commented Sep 7, 2022 by ludek.vodicka Skipper developer (140,450 points)

Skipper external tasks

1 Answer

0 votes
answered Sep 7, 2022 by ludek.vodicka Skipper developer (140,450 points)

Hi,

A new beta with implemented after-export custom tasks is available for download:

https://www.skipper18.com/support/402/downloads-skipper-beta

Please try it and let me know if you are able to configure everything you need.

Ludek

commented Sep 13, 2022 by micamou (170 points)

Hi Ludek

I can't find the mac version, perhaps it was not built ?

https://github.com/Atlantic18/Skipper/releases/tag/3.3.2.1790

Thank you very much

commented Sep 14, 2022 by ludek.vodicka Skipper developer (140,450 points)

Sorry, thanks for the info. It's fixed now.

commented Sep 15, 2022 by micamou (170 points)

Hello Ludek,

It works fine for me, thank you very much

...