PHP 8.1 | PSR2/PropertyDeclaration: add check for visibility before readonly keyword

The `PSR2.Classes.PropertyDeclaration` sniff includes a check to verify that the `static` keyword is _after_ the visibility in a property declaration.

PHP 8.1 introduced the `readonly` keyword for properties, but as PSR2/PSR12 were both released before PHP 8.1, no PSR rules have (yet) been defined for the modifier keyword order for `readonly` properties.

Having said that, all code samples in both the RFC as well as the PHP manual use the `visibility - readonly` modifier keyword order, so it is likely that this will become the standard modifier keyword order.

A search for PHP projects which have started to use the `readonly` keyword, also shows these predominantly use the `visibility - readonly` modifier keyword order.

With that in mind, I'm proposing to add a check for this order to the `PSR2.Classes.PropertyDeclaration` sniff - this being the only PHPCS native sniff which checks the modifier keyword order for properties.

As PSR2/PSR12 do not formally have rules for the order (yet), the new error code being introduced is excluded for those rulesets (for the time being).

Includes unit tests.

Ref:
* https://wiki.php.net/rfc/readonly_properties_v2
* https://www.php.net/manual/en/language.oop5.properties.php#language.oop5.properties.readonly-properties
* https://sourcegraph.com/search?q=context:global+%5Cs%28public%7Cprotected%7Cprivate%29%5Cs%2Breadonly%5Cs%2B+lang:php+-file:%28%5E%7C/%29%28vendor%7Ctests%3F%29/+fork:no+archived:no&patternType=regexp (searching `visibility - readonly` order - > 500 results)
* https://sourcegraph.com/search?q=context:global+%5Csreadonly%5Cs%2B%28public%7Cprotected%7Cprivate%29%5Cs%2B+lang:php+-file:%28%5E%7C/%29%28vendor%7Ctests%3F%29/+fork:no+archived:no&patternType=regexp (searching `readonly - visibility` order - 41 (non false positive) results)

Author: jrfnl

src/Standards/PSR12/ruleset.xml

@@ -143,6 +143,9 @@
     That is, an underscore prefix explicitly has no meaning. -->
     <!-- There MUST be a space between type declaration and property name. -->
     <rule ref="PSR2.Classes.PropertyDeclaration"/>
+    <rule ref="PSR2.Classes.PropertyDeclaration.ReadonlyBeforeVisibility">
+        <severity>0</severity>
+    </rule>
 
     <!-- Visibility MUST be declared on all constants if your project PHP minimum version supports constant visibilities (PHP 7.1 or later). -->
     <!-- checked by PSR12.Properties.ConstantVisibility -->

src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php

@@ -121,27 +121,50 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
         if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_static'] === true) {
             $scopePtr  = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
             $staticPtr = $phpcsFile->findPrevious(T_STATIC, ($stackPtr - 1));
-            if ($scopePtr < $staticPtr) {
-                return;
-            }
+            if ($scopePtr > $staticPtr) {
+                $error = 'The static declaration must come after the visibility declaration';
+                $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'StaticBeforeVisibility');
+                if ($fix === true) {
+                    $phpcsFile->fixer->beginChangeset();
 
-            $error = 'The static declaration must come after the visibility declaration';
-            $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'StaticBeforeVisibility');
-            if ($fix === true) {
-                $phpcsFile->fixer->beginChangeset();
+                    for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
+                        if ($tokens[$i]['code'] !== T_WHITESPACE) {
+                            break;
+                        }
 
-                for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
-                    if ($tokens[$i]['code'] !== T_WHITESPACE) {
-                        break;
+                        $phpcsFile->fixer->replaceToken($i, '');
                     }
 
-                    $phpcsFile->fixer->replaceToken($i, '');
+                    $phpcsFile->fixer->replaceToken($scopePtr, '');
+                    $phpcsFile->fixer->addContentBefore($staticPtr, $propertyInfo['scope'].' ');
+
+                    $phpcsFile->fixer->endChangeset();
                 }
+            }
+        }//end if
 
-                $phpcsFile->fixer->replaceToken($scopePtr, '');
-                $phpcsFile->fixer->addContentBefore($staticPtr, $propertyInfo['scope'].' ');
+        if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_readonly'] === true) {
+            $scopePtr    = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
+            $readonlyPtr = $phpcsFile->findPrevious(T_READONLY, ($stackPtr - 1));
+            if ($scopePtr > $readonlyPtr) {
+                $error = 'The readonly declaration must come after the visibility declaration';
+                $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'ReadonlyBeforeVisibility');
+                if ($fix === true) {
+                    $phpcsFile->fixer->beginChangeset();
+
+                    for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
+                        if ($tokens[$i]['code'] !== T_WHITESPACE) {
+                            break;
+                        }
 
-                $phpcsFile->fixer->endChangeset();
+                        $phpcsFile->fixer->replaceToken($i, '');
+                    }
+
+                    $phpcsFile->fixer->replaceToken($scopePtr, '');
+                    $phpcsFile->fixer->addContentBefore($readonlyPtr, $propertyInfo['scope'].' ');
+
+                    $phpcsFile->fixer->endChangeset();
+                }
             }
         }//end if
 

src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc

@@ -80,4 +80,8 @@ class ReadOnlyProp {
     protected readonly ?string    $foo;
 
     readonly array $foo;
+
+    readonly public int $wrongOrder1;
+
+    readonly protected ?string $wrongOrder2;
 }

src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed

@@ -77,4 +77,8 @@ class ReadOnlyProp {
     protected readonly ?string $foo;
 
     readonly array $foo;
+
+    public readonly int $wrongOrder1;
+
+    protected readonly ?string $wrongOrder2;
 }

src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php

@@ -49,6 +49,8 @@ public function getErrorList()
             76 => 1,
             80 => 1,
             82 => 1,
+            84 => 1,
+            86 => 1,
         ];
 
     }//end getErrorList()

src/Standards/PSR2/ruleset.xml

@@ -104,6 +104,9 @@
     <rule ref="PSR2.Classes.PropertyDeclaration.SpacingAfterType">
         <severity>0</severity>
     </rule>
+    <rule ref="PSR2.Classes.PropertyDeclaration.ReadonlyBeforeVisibility">
+        <severity>0</severity>
+    </rule>
 
     <!-- 4.3 Methods -->