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

PHP 8.0 support #4702

Closed
47 of 62 tasks
julienfalque opened this issue Dec 18, 2019 · 46 comments
Closed
47 of 62 tasks

PHP 8.0 support #4702

julienfalque opened this issue Dec 18, 2019 · 46 comments
Labels
kind/meta Set of actions to be broken down into single issues/PR's topic/PHP8.0 Related to features available in PHP 8.0+

Comments

@julienfalque
Copy link
Member

julienfalque commented Dec 18, 2019

Syntax changes that require new tests to make sure the tool is compatible with PHP 8.0:

- GMP:
  . gmp_random() has been removed. One of gmp_random_range() or
    gmp_random_bits() should be used instead.
- OCI8:
  . The OCI-Lob class is now called OCILob, and the OCI-Collection class is now
    called OCICollection for name compliance enforced by PHP 8 arginfo
    type annotation tooling.
  . Several alias functions have been marked as deprecated.
- Reflection:
  . The method signatures
        ReflectionClass::newInstance($args)
        ReflectionFunction::invoke($args)
        ReflectionMethod::invoke($object, $args)

    have been changed to:
        ReflectionClass::newInstance(...$args)
        ReflectionFunction::invoke(...$args)
        ReflectionMethod::invoke($object, ...$args)

    Code that must be compatible with both PHP 7 and PHP 8 can use the following
    signatures to be compatible with both versions:
        ReflectionClass::newInstance($arg = null, ...$args)
        ReflectionFunction::invoke($arg = null, ...$args)
        ReflectionMethod::invoke($object, $arg = null, ...$args)

Other changes that might require fixing existing tests and/or adding new ones:

Additional source: https://github.com/php/php-src/blob/php-8.0.0rc1/UPGRADING

Note

  • From now we only accept PR's that are compatible with PHP8
  • Updating composer.json of the project will be done after all current functionality has been made compatible (new feature support can wait)
  • We will keep the build step nightly as allowed to fail. When there is a dedicated Travis target for PHP8 we will add it to the steps
@julienfalque julienfalque added the kind/meta Set of actions to be broken down into single issues/PR's label Dec 18, 2019
@julienfalque julienfalque pinned this issue Dec 18, 2019
@mvorisek
Copy link
Contributor

mvorisek commented Jan 8, 2020

Union Types rule should definitely support sorting to prevent !n possible orders. Probably based on phpdoc rules where there is alrady option for it.

@julienfalque
Copy link
Member Author

Note that this issue is not about implementing new rules but making sure that existing rules do not break with new PHP 8.0 syntax changes. Nice suggestion though 👍

@GrahamCampbell
Copy link
Contributor

GrahamCampbell commented Jul 6, 2020

More changes to the lexer: sebastianbergmann/php-token-stream#95. (php/php-src#5182)

@GrahamCampbell
Copy link
Contributor

@SpacePossum that RFC has been rejected. Voting closed yesterday, and they failed to get a 2/3 majority (only managed 58.9%).

@hamdrew
Copy link

hamdrew commented Oct 1, 2020

@GrahamCampbell can you link the RFC? php/php-src#5182 appears to be accepted and merged (despite the closed PR status).

@hamdrew
Copy link

hamdrew commented Oct 1, 2020

Would a dedicated Travis CI job for PHP 8.0 be a part of the definition of done for this issue? I know there is a nightly builds job, but that differs from pointing to a release of PHP 8.0 (even if it is beta).

@julienfalque
Copy link
Member Author

Would a dedicated Travis CI job for PHP 8.0 be a part of the definition of done for this issue?

If Travis CI supports dedicated PHP 8.0 jobs, I would say yes.

@GrahamCampbell
Copy link
Contributor

#5114 gets us started with some stuff. If anyone could work out why that\s not working, that'd be a help.

@hamdrew
Copy link

hamdrew commented Oct 2, 2020

PHP 8.0 Wiki Page and Timetable - GA Release 11/26/2020

PHP 8.0.0RC1 Upgrade Notes - information on new features and backwards-incompatible changes.

Implemented RFCs for 8.0 - a good place to learn of language changes that would most affect this project

@hamdrew
Copy link

hamdrew commented Oct 2, 2020

I took some time to review all the core changes in the upgrade notes and their respective RFCs. I picked out the ones that had syntax changes, or I otherwise felt might break a rule.

I created a Google Sheet to track these potentially rule-breaking changes. Anyone can comment on this sheet. Please request edit permissions if desired:

removed link to avoid confusion

This is just a quick-and-dirty organization technique for myself that I thought would be useful; if there is a better place to organize this (like in this GitHub issue description?) let me know.

Edit: PHPCompatibility has a nice checklist of changes as well. I also like how they organized the tasks.

@julienfalque
Copy link
Member Author

@hamdrew To be honest I prefer tracking PHP changes here, I updated the description. Hopefully I didn't forget anything important.

@hamdrew
Copy link

hamdrew commented Oct 3, 2020

@julienfalque I agree! I removed the link to avoid confusion.

Last night I set up a PHP 8 docker environment to develop from. I share it here if anyone finds it useful: https://github.com/hamdrew/PHP-CS-Fixer-Docker

I didn't want to contribute a docker development environment directly to this project because that is a large discussion that doesn't need to happen right now.

@SpacePossum SpacePossum added the topic/PHP8.0 Related to features available in PHP 8.0+ label Oct 9, 2020
hamdrew added a commit to hamdrew/PHP-CS-Fixer that referenced this issue Oct 11, 2020
This commit addresses the following PHP8 backwards-incompatible change:

> SplFixedArray is now an IteratorAggregate and not an Iterator. SplFixedArray::rewind(), ::current(), ::key(), ::next(), and ::valid() have been removed. In their place, SplFixedArray::getIterator() has been added. Any code which uses explicit iteration over SplFixedArray must now obtain an Iterator through SplFixedArray::getIterator(). This means that SplFixedArray is now safe to use in nested loops.

Addresses PHP-CS-Fixer#4702
@hamdrew
Copy link

hamdrew commented Oct 11, 2020

I addressed the SplFixedArray issue first in #5167, since it was prohibiting a lot of tests from running.

@SpacePossum To what branch would you like php8-related PRs to be targeted? I chose master for now.

@SpacePossum
Copy link
Contributor

Thanks @hamdrew for the work in the PR!
Currently I'm working in PHP8 support as well in #5166 , for now I picked 2.15 to see what we can do for as many as branches we support.

@hamdrew
Copy link

hamdrew commented Oct 12, 2020

@SpacePossum Cool, I'll target the same branch in my PRs to be consistent.

hamdrew added a commit to hamdrew/PHP-CS-Fixer that referenced this issue Oct 12, 2020
This commit addresses the following PHP8 backwards-incompatible change:

> SplFixedArray is now an IteratorAggregate and not an Iterator. SplFixedArray::rewind(), ::current(), ::key(), ::next(), and ::valid() have been removed. In their place, SplFixedArray::getIterator() has been added. Any code which uses explicit iteration over SplFixedArray must now obtain an Iterator through SplFixedArray::getIterator(). This means that SplFixedArray is now safe to use in nested loops.

Addresses PHP-CS-Fixer#4702
@SpacePossum SpacePossum mentioned this issue Oct 13, 2020
SpacePossum added a commit that referenced this issue Oct 13, 2020
This PR was merged into the 2.15 branch.

Discussion
----------

PHP8

#4702

some background on the transformers:

d76397e

Commits
-------

a4d92e8 PHP8 care package
@GrahamCampbell
Copy link
Contributor

Related: PHP-CS-Fixer/diff#19.

@SanderSander
Copy link
Contributor

SanderSander commented Dec 16, 2020

Based on the reasoning to not allow usage of the tool in #5341

The result running PHP CS Fixer on code with it is not known, currently.
We already had a case when PHP 8 code was parsed, changed by fixer and produced valid PHP syntax, but with different meaning. We don't want to offer that to users of the tool. In our opinion it's better to not offer the tool than offer the tool and break the code logic.

Would it be an idea to give the user a warning when someone uses ./vendor/bin/php-cs-fixer with PHP 8.0 instead of terminating execution?

@keradus
Copy link
Member

keradus commented Dec 16, 2020

or at least the info how to use PHP_IGNORE_ENV.
would you mind to add that note, @SanderSander ?

@SanderSander
Copy link
Contributor

SanderSander commented Dec 16, 2020

@keradus Ah sorry guess I'm getting tired that would be way better 😅 I will add some documentation about PHP_CS_FIXER_IGNORE_ENV

keradus added a commit that referenced this issue Dec 17, 2020
…rSander, keradus)

This PR was squashed before being merged into the 2.17 branch.

Discussion
----------

Update documentation about PHP_CS_FIXER_IGNORE_ENV

See #4702

Commits
-------

acf24b5 Update documentation about PHP_CS_FIXER_IGNORE_ENV
@keradus keradus mentioned this issue Dec 28, 2020
6 tasks
keradus added a commit that referenced this issue Jan 18, 2021
This PR was squashed before being merged into the 2.17 branch.

Discussion
----------

Add PHP8 integration test

ref #4702

There are ~3~ 4 cases still crashing in the spec files:
- [x] nullsafe operator -> PHP 8.0.1 to be released at 7th
- [x] attributes #5406
- [x] union types for method parameters #5405
- [x] union types for class properties #5439

I also found the following issues that I raised as separated PRs:
- [x] ~#5396~ -> #5397
- [x] ~#5400~ -> #5404

Commits
-------

324929f Add PHP8 integration test
@keradus
Copy link
Member

keradus commented Jan 18, 2021

2.18 with PHP8 enabled was released. If you would spot any issues with it, raise a new ticket.

I'm leaving this meta-ticket open, as it still contains plenty of ideas for new rules.

@SpacePossum
Copy link
Contributor

I'm going to close here as a lot of work has been done and having new dedicated issues for each item will be much easier to track going forward. Thanks all for sharing information, thoughts and options, great work 👍 : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/meta Set of actions to be broken down into single issues/PR's topic/PHP8.0 Related to features available in PHP 8.0+
Projects
None yet
Development

No branches or pull requests