Php-cs-fixer: [Meta] PHP 7.2 features

[SpacePossum]

PHP 7.2.0 Alpha 3 is released , lets list the things we need to take care of or could support.

• [ ] "object" (in any case) can no longer be used as a class name. RFC
• [ ] It is now possible to remove argument type annotations when overriding an inherited method. RFC
• [ ] It is now allowed to override an abstract method with another abstract method in a child class. RFC
• [x] A trailing comma in group use statements is now allowed. RFC PR
• [ ] The "object" type annotation is now supported. RFC
• [ ] The (unset) cast has been deprecated. This does not affect the unset($var) language construct.
• [ ] Deprecations for PHP 7.2 RFC

Sources:
https://github.com/php/php-src/blob/php-7.2.0RC2/NEWS
https://github.com/php/php-src/blob/php-7.2.0RC2/UPGRADING

Note: I've only listed the first bunch of items I spotted that might effect us, i.e. because we made assumptions on PHP behavor and/or syntax, and items we could write fixers for.

Please comment if I forgot anything (@ maintainers, feel free to update this issue with more details when needed)

Ps. Jun 18 should be the feature freeze; https://wiki.php.net/todo/php72 so in theory new items might be in Beta 1 (that are not listed here yet)

[glensc]

the freeze date is actually Jul 18 2017 | PHP 7.2 branched and feature freeze on the linked page

[SpacePossum]

thanks :) I either copy-pasted wrongly or the doc linked was updated, anyway I'll update the ticket :)

[glensc]

PHP 7.2.0 Beta 1 Released :)

PHP 7.2.0 Beta 1 builds on previous releases with:

• The much anticipated Sodium extension
• Opcache improvements
• Countable support for DOMNodeList and DOMNamedNodeMap
• Improved handling for invalid UTF8 in json_decode()

[keradus]

nothing related to this project...

[ghost]

iiuc, there's _not_ yet a current/working branch that's php 7.2-friendly?

        - friendsofphp/php-cs-fixer v2.3.2 requires php ^5.6 || >=7.0 <7.2 -> your PHP version (7.2.0-dev) does not satisfy that requirement.
        - Installation request for friendsofphp/php-cs-fixer (locked at v2.3.2, required as ^2.3) -> satisfiable by friendsofphp/php-cs-fixer[v2.3.2].

just a bit confused between this, and other posts ...

[keradus]

there won't be fully compatible release with 7.2 until all 7.2 syntax changes (thus, for us, incompatibilities) will be handled.
you could execute app on 7.2 nevertheless by setting PHP_CS_FIXER_IGNORE_ENV env var

[shouze]

as @pgnd said,

- friendsofphp/php-cs-fixer v2.3.2 requires php ^5.6 || >=7.0 <7.2 -> your PHP version (7.2.0-dev) does not satisfy that requirement.

So, is it planned at any time to update composer.json requirements to allow to install with composer under php 7.2 beta?

[keradus]

as @keradus said,

there won't be fully compatible release with 7.2 until all 7.2 syntax changes (thus, for us, incompatibilities) will be handled.

---

So, is it planned at any time to update composer.json requirements to allow to install with composer under php 7.2 beta?

it's not about changing one composer.json file. requirement in composer states which syntax we understand and support. if software will be executed on 7.2 code, it could break it, and behaviour is unexpectable.
If you want to have PHP CS Fixer run on 7.2, please help with supporting 7.2 syntax changes.

[Jean85]

Can we please discuss in detail what specific impact each new feature has on this project? I would like to help on this, but as a newcomer here I don't know enough stuff to start coding, I would love some pointers!

3007 has demonstrated that the tests suite is currently green under 7.2, so no issues in the current state. As for what quoted in the opening message, I would like to summarize my conclusions. Please correct me if I'm wrong:

1. "object" (in any case) can no longer be used as a class name:
   this has no impact on us, since no fixer exist for this, and no Object class is present in the project
2. It is now possible to remove argument type annotations when overriding an inherited method:
   No impact here too, no fixer or no known issue in the current codebase
3. It is now allowed to override an abstract method with another abstract method in a child class:
   No impact here too, no fixer and it's only a new possibility that works between two different (parent-child) classes
4. A trailing comma in group use statements is now allowed:
   Already done in #2941
5. The "object" type annotation is now supported
   This may need some testing? Do we have some test that needs a new fixture with an object $a annotation?
6. The (unset) cast has been deprecated. This does not affect the unset($var) language construct:
   No test failure (due to deprecation) so no usage of this as far as I can tell

As I can see, only 5 may need some (small) work to allow PHP 7.2. Am I missing something?

[keradus]

Hi. not able to read whole of it now, will do it later (and I'm pinging @SpacePossum for time being), but one thing I could say now:

3007 has demonstrated that the tests suite is currently green under 7.2, so no issues in the current state.

We need to add all new/changed syntax of PHP 7.2 to:
https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/2.5/tests/Fixtures/Integration/misc/PHP7_2.test

to claim we do support 7.2
It's not about "we change 7.2 syntax", but it's about "we understand and not break 7.2 syntax"
so first thing is to gather syntax changes in 7.2 integration test and fix whatever is needed, and then, if we want to, we could make new rules targeting 7.2 only (but that part is not obligatory to provide official 7.2 support)

[SpacePossum]

First off, the tests prove the code runs fine on PHP7.2. However we need to prove we understand PHP7.2 code (as @keradus stated).

Looking at the items I've listed there are still a few TODO's. My view on things;

It is now possible to remove argument type annotations when overriding an inherited method. RFC

For this we should check the fixers that insert and/or remove type hints, either in code or in PHPDocs. Even if there are no issues, we still need tests to prove it.

"object" (in any case) can no longer be used as a class name. RFC
The "object" type annotation is now supported. RFC

Same as above. I expect additional work for this after that.

It is now allowed to override an abstract method with another abstract method in a child class. RFC

We should check all fixers dealing with abstract methods and add tests.

A trailing comma in group use statements is now allowed. RFC PR

One issue was fixed, however we should add tests to all fixers that deal with imports.
Include the ones that reorders, checks for unused imports etc., just to see those work as expected.

The (unset) cast has been deprecated. This does not affect the unset($var) language construct.

I looked into this, but I see no work to be done here for us.

[Jean85]

For this we should check the fixers that insert and/or remove type hints, either in code or in PHPDocs. Even if there are no issues, we still need tests to prove it.

The only fixer that add/removes an argument that I could find is phpdoc_add_missing_param_annotation, but it doesn't care about parent/child classes, so it shouldn't be impacted by that change.

One issue was fixed, however we should add tests to all fixers that deal with imports.
Include the ones that reorders, checks for unused imports etc., just to see those work as expected.

Do you mean those in combination with the one already done? Or the tests should start with an input with the trailing comma?

[SpacePossum]

For example the tests of OrderedImportsFixer should be extended to with PHP 7.2 test code with the trailing , in the imports to see if still works as expected. Same for other fixers that deal with imports.

[Jean85]

You seem to have already done it! See b3b58f501a11d26e3e9b86fdecfdf06efa440d5f

That's funny :smile:

[hkdobrev]

Nice!

That PR #2941 description stated:

This doesn't solve all issues for PHP 7.2 support, but it is a good first round I think :)

Wondering whether there's something more.

[SpacePossum]

I'm getting old :)

[Jean85]

Ahah :smile:
So, do we consider this issue done? :+1:

[keradus]

this meta-issue ? no
even if group-imports is tested, we need to at least add other things to integration tests

[SpacePossum]

Closing as https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/3038 provides the last of tests.
Enhancements and other implementation details can be discussed in dedicated issues and PR's.