PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

Does not work with readonly classes introduced in PHP 8.2

Open siketyan opened this issue 1 year ago • 2 comments

May be closed by ~#3686~ #3728

Describe the bug In PHP 8.2, "readonly class" feature was introduced. Using readonly classes, we can shorten readonly property declarations as follows:

class Foo {
    public function __construct(
        private readonly string $foo,
        private readonly int $bar,
    ) {
    }
}

equals to:

readonly class Foo {
    public function __construct(
        private string $foo,
        private string $bar,
    ) {
    }
}

However, the latest version of phpcs v3.7.1 does not work on the classes which are marked as "readonly". (The phpcs error output is in "To reproduce" section)

Code sample

readonly class Foo {
    public function __construct(
        private string $foo,
        private string $bar,
    ) {
    }
}

Custom ruleset N/A

To reproduce Steps to reproduce the behavior:

  1. Run phpcs with the default ruleset, with the code sample above.
FILE: /project/Foo.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | WARNING | A file should declare new symbols (classes, functions,
   |         | constants, etc.) and cause no other side effects, or
   |         | it should execute logic with side effects, but should
   |         | not do both. The first symbol is defined on line 13
   |         | and the first side effect is on line 13.
   |         | (PSR1.Files.SideEffects.FoundWithSymbols)
----------------------------------------------------------------------

Expected behavior A clear and concise description of what you expected to happen.

Versions (please complete the following information):

  • OS: macOS Ventura 13.0.1 (Apple M1 Max; aarch64-apple-darwin)
  • PHP: 8.2.0
  • PHPCS: 3.7.1
  • Standard: PSR12

Additional context N/A

siketyan avatar Dec 10 '22 11:12 siketyan

Thanks for reporting this.

PR #3686 would not fix this, though I have a number of follow-up commits waiting on #3686 to be merged to add support for readonly classes to various sniffs. However, looks like I missed the PSR1.Files.SideEffects sniff and as the fix for that sniff does not depend on #3686, I have pulled PR #3728 now to fix this.

jrfnl avatar Dec 10 '22 12:12 jrfnl

@jrfnl Thank you for the update and your work!

siketyan avatar Dec 10 '22 14:12 siketyan

PHP 8.2 beta came out July 21, 2022. The first RC came out on Sep 1. The official stable release was on Dec 8, 2022. And here we are, April 30, 2023, and we still have to choose between dropping PHP_CodeSniffer and actually making use of the readonly classes feature.

still-dreaming-1 avatar Apr 30 '23 13:04 still-dreaming-1

@still-dreaming-1 Have you tried contributing ?

jrfnl avatar Apr 30 '23 13:04 jrfnl

@jrfnl Would contributing help though? There are so many pull requests already waiting (you seem to have already fixed this issue for quite some time, for example, thanks for that!) and no avenue to help out or indication what would help. I would donate some money for the project or for a maintainer (as I do for other open source projects), but there is no information about that either. It does seem that this project is somehow stuck.

iquito avatar Apr 30 '23 14:04 iquito

@iquito We're working on getting it unstuck, but comments like the one I responded to are not helpful and highly demotivating.

jrfnl avatar Apr 30 '23 14:04 jrfnl

Another readonly classes issue was reported in https://github.com/squizlabs/PHP_CodeSniffer/issues/3799#issuecomment-1532870980. A fix for that one has been pulled in https://github.com/squizlabs/PHP_CodeSniffer/pull/3816

jrfnl avatar May 03 '23 12:05 jrfnl

I've merged the fix. Thanks for the bug report.

gsherwood avatar May 22 '23 23:05 gsherwood

@gsherwood Is this something that you could tag soon? It'd be nice to stop disabling the busted rule

KorvinSzanto avatar Jun 29 '23 17:06 KorvinSzanto

@jrfnl @gsherwood any ETA on getting this out?

kasperhendriks avatar Aug 03 '23 08:08 kasperhendriks

Just ran into this myself; any chance for a new release of PHPCS? Thank you for considering!

kallesommernielsen avatar Sep 11 '23 02:09 kallesommernielsen

How can we help to tag this hotfix ?

alamirault avatar Sep 25 '23 12:09 alamirault

@alamirault There's nothing anyone can do, but wait patiently for @gsherwood to have some time again. Also see: https://github.com/squizlabs/PHP_CodeSniffer/issues/3861

jrfnl avatar Sep 25 '23 13:09 jrfnl

Tough to find time for development when you're a CTO now 😉 It certainly aligns with his activity on GitHub. Hopefully he hasn't forgotten his roots! image

kczx3 avatar Oct 20 '23 20:10 kczx3

FYI: the fix for this issue is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).

jrfnl avatar Dec 08 '23 22:12 jrfnl