Skip to content
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.

Sign up for GitHub

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

feat: @Symfony - keep Annotation,NamedArgumentConstructor,Target annotations as single group #7399

Merged
merged 5 commits into from
Oct 31, 2023
Merged

feat: @Symfony - keep Annotation,NamedArgumentConstructor,Target annotations as single group #7399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c937dbf
feat: `@Symfony` - keep Annotation,NamedArgumentConstructor,Target an…
keradus Oct 31, 2023
8f4cf6e
docs
keradus Oct 31, 2023
fe2e3ce
Merge branch 'master' into xx
keradus Oct 31, 2023
0fc5468
kick CI
keradus Oct 31, 2023
a83d808
sort docs
keradus Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/list.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2394,7 +2394,7 @@ List of Available Rules
- | ``groups``
| Sets of annotation types to be grouped together. Use ``*`` to match any tag character.
| Allowed types: ``string[][]``
| Default value: ``[['deprecated', 'link', 'see', 'since'], ['author', 'copyright', 'license'], ['category', 'package', 'subpackage'], ['property', 'property-read', 'property-write']]``
| Default value: ``[['author', 'copyright', 'license'], ['category', 'package', 'subpackage'], ['property', 'property-read', 'property-write'], ['deprecated', 'link', 'see', 'since']]``
- | ``skip_unlisted_annotations``
| Whether to skip annotations that are not listed in any group.
| Allowed types: ``bool``
Expand Down
5 changes: 4 additions & 1 deletion doc/ruleSets/Symfony.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ Rules

- `phpdoc_return_self_reference <./../rules/phpdoc/phpdoc_return_self_reference.rst>`_
- `phpdoc_scalar <./../rules/phpdoc/phpdoc_scalar.rst>`_
- `phpdoc_separation <./../rules/phpdoc/phpdoc_separation.rst>`_
- `phpdoc_separation <./../rules/phpdoc/phpdoc_separation.rst>`_ with config:

``['groups' => [['Annotation', 'NamedArgumentConstructor', 'Target'], ['author', 'copyright', 'license'], ['category', 'package', 'subpackage'], ['property', 'property-read', 'property-write'], ['deprecated', 'link', 'see', 'since']]]``

- `phpdoc_single_line_var_spacing <./../rules/phpdoc/phpdoc_single_line_var_spacing.rst>`_
- `phpdoc_summary <./../rules/phpdoc/phpdoc_summary.rst>`_
- `phpdoc_tag_type <./../rules/phpdoc/phpdoc_tag_type.rst>`_ with config:
Expand Down
12 changes: 9 additions & 3 deletions doc/rules/phpdoc/phpdoc_separation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ character.

Allowed types: ``string[][]``

Default value: ``[['deprecated', 'link', 'see', 'since'], ['author', 'copyright', 'license'], ['category', 'package', 'subpackage'], ['property', 'property-read', 'property-write']]``
Default value: ``[['author', 'copyright', 'license'], ['category', 'package', 'subpackage'], ['property', 'property-read', 'property-write'], ['deprecated', 'link', 'see', 'since']]``

``skip_unlisted_annotations``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -155,6 +155,12 @@ Rule sets

The rule is part of the following rule sets:

- `@PhpCsFixer <./../../ruleSets/PhpCsFixer.rst>`_
- `@Symfony <./../../ruleSets/Symfony.rst>`_
- `@PhpCsFixer <./../../ruleSets/PhpCsFixer.rst>`_ with config:

``['groups' => [['Annotation', 'NamedArgumentConstructor', 'Target'], ['author', 'copyright', 'license'], ['category', 'package', 'subpackage'], ['property', 'property-read', 'property-write'], ['deprecated', 'link', 'see', 'since']]]``

- `@Symfony <./../../ruleSets/Symfony.rst>`_ with config:

``['groups' => [['Annotation', 'NamedArgumentConstructor', 'Target'], ['author', 'copyright', 'license'], ['category', 'package', 'subpackage'], ['property', 'property-read', 'property-write'], ['deprecated', 'link', 'see', 'since']]]``


19 changes: 13 additions & 6 deletions src/Fixer/Phpdoc/PhpdocSeparationFixer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@
*/
final class PhpdocSeparationFixer extends AbstractFixer implements ConfigurableFixerInterface
{
/**
* @internal
*
* @var string[][]
*/
public const OPTION_GROUPS_DEFAULT = [
['author', 'copyright', 'license'],
['category', 'package', 'subpackage'],
['property', 'property-read', 'property-write'],
['deprecated', 'link', 'see', 'since'],
];

/**
* @var string[][]
*/
Expand Down Expand Up @@ -161,12 +173,7 @@ protected function createConfigurationDefinition(): FixerConfigurationResolverIn
return new FixerConfigurationResolver([
(new FixerOptionBuilder('groups', 'Sets of annotation types to be grouped together. Use `*` to match any tag character.'))
->setAllowedTypes(['string[][]'])
->setDefault([
['deprecated', 'link', 'see', 'since'],
['author', 'copyright', 'license'],
['category', 'package', 'subpackage'],
['property', 'property-read', 'property-write'],
])
->setDefault(self::OPTION_GROUPS_DEFAULT)
->setAllowedValues([$allowTagToBelongToOnlyOneGroup])
->getOption(),
(new FixerOptionBuilder('skip_unlisted_annotations', 'Whether to skip annotations that are not listed in any group.'))
Expand Down
8 changes: 7 additions & 1 deletion src/RuleSet/Sets/SymfonySet.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

namespace PhpCsFixer\RuleSet\Sets;

use PhpCsFixer\Fixer\Phpdoc\PhpdocSeparationFixer;
use PhpCsFixer\RuleSet\AbstractRuleSetDescription;

/**
Expand Down Expand Up @@ -166,7 +167,12 @@ public function getRules(): array
],
'phpdoc_return_self_reference' => true,
'phpdoc_scalar' => true,
'phpdoc_separation' => true,
'phpdoc_separation' => [
'groups' => [
['Annotation', 'NamedArgumentConstructor', 'Target'],
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say that grouping those would also belong to a doctrine annotations ruleset, as they relate to the usage of doctrine annotations.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think its valid point,
yet phpdoc_separation is not currently in use by @DoctrineAnnotation and I'm not sure if we want to start using it there.

There is tricky point of rule getting override if multiple rulesets are using it, and maybe it's not a top priority to solve it now. Doctrine ruleset could benefit from even bigger cleanup - especially about annotations -> attributes.
I'm not user of Doctrine and won't be able to judge there. Maybe we improve ruleset of Sf in first step, and if anyone would like to also take closer look to improve ruleset of Doctrine, we can have that as follow-up PRs (also very open if you are up for implementing something for Doctrine settings/rules)

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, the @DoctrineAnnotation is about doctrine/annotations IMO, so it should not care about attributes.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, it's probably like who effort to cleanup current rules, create rules that migrate anno to attrib, make sure rules works for both (or have sibling rule), ....
big thing I don't wan to mess myself, as not knowing that ecosystem.

...PhpdocSeparationFixer::OPTION_GROUPS_DEFAULT,
],
],
'phpdoc_single_line_var_spacing' => true,
'phpdoc_summary' => true,
'phpdoc_tag_type' => [
Expand Down