0 votes

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)
in Feature Request by (170 points)
edited by

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.

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

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

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.

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?

Almost ready

Skipper external tools

Skipper external tasks

1 Answer

0 votes

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

by Skipper developer (141k 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

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

Hello Ludek,

It works fine for me, thank you very much