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.

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

PhpdocOrderByValueFixer - Add additional annotations to sort #5391

Merged

Conversation

localheinz
Copy link
Member

@localheinz localheinz commented Dec 24, 2020

This PR

  • adjusts the phpdoc_order_by_value fixer to allow sorting of additional annotations by value
    • @method
    • @property
    • @property-read
    • @property-write

Sorry, something went wrong.

@localheinz localheinz changed the title [PhpDocOrderByValue] Allow sorting of property, property-read, and property-write annotations by value [PhpdocOrderByValue] Allow sorting additional annotations by value Dec 25, 2020
@localheinz localheinz force-pushed the feature/phpdoc-order-by-value branch 2 times, most recently from a8241a4 to b9df4f2 Compare December 25, 2020 00:29
@localheinz localheinz marked this pull request as draft December 25, 2020 01:02
@localheinz localheinz force-pushed the feature/phpdoc-order-by-value branch 2 times, most recently from 6ad2139 to 42d55a8 Compare December 25, 2020 10:08
@localheinz localheinz marked this pull request as ready for review December 25, 2020 10:09
@coveralls
Copy link

coveralls commented Dec 25, 2020

Coverage Status

Coverage increased (+0.007%) to 91.375% when pulling f98de85 on localheinz:feature/phpdoc-order-by-value into bb6ee85 on FriendsOfPHP:2.17.

@SpacePossum
Copy link
Contributor

Thanks for the PR!
Sounds like a good idea :)
Been testing a bit and wanted to share so thoughts.

Starting with the added @method support.
Taking an example from the test:
@method int foo(array $b)
will get the "comparableContent" fooarray $b).
Maybe it can improved to make it the function name (here foo) only?
Test:

[
    '<?php
    /**
     * @method int foo(Z $b)
     * @method int fooA( $b)
     */
    class Foo {}
    ',
    '<?php
    /**
     * @method int fooA( $b)
     * @method int foo(Z $b)
     */
    class Foo {}
    ',
],

If there is no return type, for example @method fooA( $b), the "comparableContent" becomes * @method fooa( $b).
I think here the @method part should be omitted to ensure correct sorting multiple statements, for example the test:

[
    '<?php
    /**
     * @method int a($b)
     * @method z($b)
     */
    class Foo {}
    ',
    '<?php
    /**
     * @method z($b)
     * @method int a($b)
     */
    class Foo {}
    ',
],

The properties fixing have a similar issue. For example @property-read $bar gets "comparableContent" * @property-read $bar while the type ones, for example, * @property-read int|\stdClass $foo get "comparableContent" $foo, meaning that sorting a list property-reads with and without types might not go as expected.
Test:

[
                '<?php
                /**
                 * @property-read $a
                 * @property-read int $z
                 */
                class Foo {}
                ',
                '<?php
                /**
                 * @property-read int $z
                 * @property-read $a
                 */
                class Foo {}
                ',
            ]

Of course this is me expecting the sorting is on the names of the method/properties and being agnostic to the return type, however to might not what others expect.

@localheinz
Copy link
Member Author

@SpacePossum

Thank you for taking the time to review and experiment!

Will look into it!

@localheinz
Copy link
Member Author

@SpacePossum

How about now?

@localheinz localheinz changed the title [PhpdocOrderByValue] Allow sorting additional annotations by value [PhpdocOrderByValueFixer] Allow sorting additional annotations by value Dec 28, 2020
@SpacePossum SpacePossum changed the title [PhpdocOrderByValueFixer] Allow sorting additional annotations by value PhpdocOrderByValueFixer - Add additional annotations to sort Dec 29, 2020
@SpacePossum SpacePossum added this to the 2.17.4 milestone Dec 29, 2020
@SpacePossum SpacePossum added the RTM Ready To Merge label Dec 29, 2020
@SpacePossum
Copy link
Contributor

looking great, thanks!

@SpacePossum SpacePossum force-pushed the feature/phpdoc-order-by-value branch from b3be523 to 604a112 Compare December 31, 2020 14:01

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…rite annotations by value

Enhancement: Allow sorting of method annotations by value
@SpacePossum SpacePossum force-pushed the feature/phpdoc-order-by-value branch from 604a112 to f98de85 Compare December 31, 2020 14:02
@SpacePossum
Copy link
Contributor

Thank you @localheinz.

@SpacePossum SpacePossum removed the RTM Ready To Merge label Dec 31, 2020
@SpacePossum SpacePossum merged commit f6c598f into PHP-CS-Fixer:2.17 Dec 31, 2020
@localheinz localheinz deleted the feature/phpdoc-order-by-value branch December 31, 2020 14:06
@localheinz
Copy link
Member Author

Thank you, @drupol and @SpacePossum!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants