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 support #3182

Closed
Ilyes512 opened this issue Dec 7, 2020 · 20 comments
Closed

PHP 8 support #3182

Ilyes512 opened this issue Dec 7, 2020 · 20 comments

Comments

@Ilyes512
Copy link

Ilyes512 commented Dec 7, 2020

Does anyone have an overview of tasks to be done before this library supports (as in, it does not break) PHP 8?

@jrfnl
Copy link
Contributor

jrfnl commented Dec 7, 2020

I do ;-)

@jrfnl
Copy link
Contributor

jrfnl commented Dec 7, 2020

But without kidding, there are different levels of PHP 8 "readiness" for a library like PHPCS, see the answer I left to a similar question here for a more extensive explanation: PHPCompatibility/PHPCompatibility#1236 (comment)

As things stand at this moment:

  • As of version 3.5.7, PHPCS will run fine on PHP 8 when scanning code without PHP 8 specific syntax.
  • As for properly supporting the PHP 8 specific syntaxes in the tokenizer: from what I've found in changes based on the RFCs and my own testing, nearly everything is done.
    • You can find an overview of most PRs which were pulled and what milestone they are in here: https://github.com/squizlabs/PHP_CodeSniffer/pulls?q=is%3Apr+PHP+8.0+ (might miss one or two which haven't got "PHP 8.0" in the subject line)
    • Of these, there is only one currently open and waiting to be merged - PHP 8.0 | Add support for named function call arguments #3178 (named parameters support), everything else has been merged between version 3.5.6 and the current master, so partially these changes have been released already, partially they will be in the 3.6.0 release.
    • There are two tokenizer changes left to still be addressed: attributes and match control structures. I started work on match a while back, see PHP 8.0 | Add support for match expressions #3037, I just need time to finish it.
      AFAIK, no work has been done (yet) to add support for attributes, partially because of the time and again changes to the syntax used for this, which made working on it until that had crystallized out kind of useless.
  • For most of the changes I pulled, the relevant changes to sniffs with PHPCS have been made as well, safe for a few.
    See: https://github.com/squizlabs/PHP_CodeSniffer/issues?q=is%3Aissue+PHP+8.0 for related issues + Support PHP 8's mixed type #2968 (might miss one or two which haven't got "PHP 8.0" in the subject line)
    These tickets often also contain lists of sniffs which needed adjusting and their status.

As for external PHPCS standards - the status of those will vary widely.

Does this answer your question sufficiently ?

@jrfnl
Copy link
Contributor

jrfnl commented Dec 11, 2020

@Ilyes512 Just checking: did my answer help ?

@Ilyes512
Copy link
Author

Yes. Thank you!

@kukulich
Copy link
Contributor

@jrfnl Are attributes supported in master too? I may be blind but I've not found any issue with the support.

@jrfnl
Copy link
Contributor

jrfnl commented Dec 11, 2020

@kukulich As per above:

AFAIK, no work has been done (yet) to add support for attributes, partially because of the time and again changes to the syntax used for this, which made working on it until that had crystallized out kind of useless.

@kukulich
Copy link
Contributor

@jrfnl Ok, I'm blind :)

@jrfnl
Copy link
Contributor

jrfnl commented Jan 16, 2021

@gsherwood May I suggest leaving this ticket open & making it a pinned issue ? I imagine more people will have this question and can benefit from the answer being easy to find. This will hopefully reduce the number of duplicate issues being opened.

Current status summary:

As of PHPCS 3.5.7, PHPCS has runtime support for PHP 8.0.

At this time, there are still two PHP 8.0 syntaxes not yet supported in PHPCS master:

  • match control structures (WIP 90% done, I just need time to finish this off)
  • attributes

Support for all other PHP 8.0 specific syntaxes has been added, though not all released yet. The upcoming PHPCS 3.6.0 release will contain the latest fixes.

While an effort has been made to verify that sniffs will function correctly with the PHP 8.0 syntaxes for which support has been added, it is always possible that something has been missed.
For bugs in individual sniffs related to PHP 8.0 syntaxes for which support has already been added to PHPCS, new tickets should be opened.

@gsherwood gsherwood reopened this Jan 17, 2021
@gsherwood gsherwood pinned this issue Jan 17, 2021
@gsherwood
Copy link
Member

Good idea. Have reopened and pinned.

@jrfnl
Copy link
Contributor

jrfnl commented Feb 28, 2021

For the record/for anyone coming upon this issue: match expression support has been merged and there is an open PR for supporting attributes (testing appreciated).

@morozov
Copy link
Contributor

morozov commented Mar 4, 2021

@jrfnl please see #3255.

@juan-morales
Copy link

please keep this pinned, and thanks for the very detailed explanation @jrfnl

@Jeroeny
Copy link

Jeroeny commented Mar 19, 2021

Similar to #3255, there is a conflict with 'Constructor property promotion' indenting:

Given:

    public function __construct(
        private Type $param
    ) {}

Result:

 15 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
    |       |     8
    |       |     (Generic.WhiteSpace.ScopeIndent.IncorrectExact)

From PHP_CodeSniffer\Standards\Generic\Sniffs\WhiteSpace\ScopeIndentSniff

Given:

    public function __construct(
    private Type $param
    ) {}

Result:

 15 | ERROR | [x] Multi-line function declaration not indented
    |       |     correctly; expected 8 spaces but found 4
    |       |     (Squiz.Functions.MultiLineFunctionDeclaration.Indent)

From PHP_CodeSniffer\Standards\Squiz\Sniffs\Functions\MultiLineFunctionDeclarationSniff

Is this actually a bug or should I change some configuration? Couldn't find an existing GitHub issue for this yet.

@fefas
Copy link

fefas commented Mar 20, 2021

I am not sure if this was already reported.

There is a bug/not-covered-case similar to the one above, but it is related to the default of a match.

The following code:

    public function method(string $arg)
    {
        return match ($arg) {
            null => 1,
            default => 2,
        };
    }

generates the following error message:

34 | ERROR | [x] Line indented incorrectly; expected 8 spaces, found 12 (Generic.WhiteSpace.ScopeIndent.IncorrectExact)

and the fix from phpcbf is:

    public function method(string $arg)
    {
        return match ($arg) {
            null => 1,
        default => 2,
        };
    }

So, it does not recognize that the default line should also be indented inside the match statement.

@gsherwood
Copy link
Member

Similar to #3255, there is a conflict with 'Constructor property promotion' indenting:

This is fixed in master.

There is a bug/not-covered-case similar to the one above, but it is related to the default of a match.

This is also fixed in master.

Both of these fixes will be in the 3.6.0 release, which is just waiting for attribute support to be merged in now.

@gsherwood
Copy link
Member

I've just merged the attribute MR, which was the last for PHP8 language feature support. There are still some sniffs that need reviewing for attribute support, but a release is close now.

All testing help on master very much appreciated.

bishopb pushed a commit to rollbar/rollbar-php that referenced this issue Mar 31, 2021
There's a [bug in its processing of constructor property promotion][1]
that is purportedly fixed in 3.6 (not released yet, so fallback to 3.x
dev head).

[1]:squizlabs/PHP_CodeSniffer#3182 (comment)
@mrmkrs
Copy link

mrmkrs commented Apr 1, 2021

Hi, i've been watching this issue and here is a small test result: phpcs 3.5.8 reported ~ 160 issues on our PHP 8 codebase. Ran phpcbf on dev-master and it fixed all issues without errors.

@caugner
Copy link

caugner commented Apr 9, 2021

🎉 phpcs 3.6.0 was released today with PHP 8 Language Feature Support:

https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/3.6.0

@jrfnl
Copy link
Contributor

jrfnl commented Apr 9, 2021

Good catch @caugner. I think this issue can now be closed (but should probably stay pinned for a while longer).

If anyone finds any PHP 8 related bugs in the PHPCS 3.6.0 release, please check if there is an open issue for it and if not, open one for each bug you find.

@andypost
Copy link
Contributor

Thank you for release!!!

Running it on Drupal's core on PHP 8.1RC3 produces 2 deprecations for me

PHP Deprecated:  Implicit conversion from float 235309.85403060913 to int loses precision in /var/www/html/web/vendor/squizlabs/php_codesniffer/src/Util/Timing.php on line 67

Deprecated: Implicit conversion from float 235309.85403060913 to int loses precision in /var/www/html/web/vendor/squizlabs/php_codesniffer/src/Util/Timing.php on line 67
Time: 3 mins, 55.31 secs; Memory: 158MB

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

No branches or pull requests