PHP 8.1 | PSR2/PropertyDeclaration: add check for visibility before r…

…eadonly 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.

[PSR PER 2.0.0](https://www.php-fig.org/per/coding-style/#46-modifier-keywords) dictates that visibility is declared before the `readonly` keyword.

All code samples in both the RFC as well as the PHP manual also 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 this sniff is included in PSR2 and PSR12, the new check will automatically apply the PSR-PER property modifier keyword order rules to PSR2/PSR12.

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)

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

@@ -118,30 +118,70 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
             $phpcsFile->addError($error, $stackPtr, 'ScopeMissing', $data);
         }
 
+        /*
+         * Note: per PSR-PER section 4.6, the order should be:
+         * - Inheritance modifier: `abstract` or `final`.
+         * - Visibility modifier: `public`, `protected`, or `private`.
+         * - Scope modifier: `static`.
+         * - Mutation modifier: `readonly`.
+         * - Type declaration.
+         * - Name.
+         *
+         * Ref: https://www.php-fig.org/per/coding-style/#46-modifier-keywords
+         *
+         * At this time (PHP 8.2), inheritance modifiers cannot be applied to properties and
+         * the `static` and `readonly` modifiers are mutually exclusive and cannot be used together.
+         *
+         * Based on that, the below modifier keyword order checks are sufficient (for now).
+         */
+
         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
+
+        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->replaceToken($i, '');
+                    }
 
-                $phpcsFile->fixer->replaceToken($scopePtr, '');
-                $phpcsFile->fixer->addContentBefore($staticPtr, $propertyInfo['scope'].' ');
+                    $phpcsFile->fixer->replaceToken($scopePtr, '');
+                    $phpcsFile->fixer->addContentBefore($readonlyPtr, $propertyInfo['scope'].' ');
 
-                $phpcsFile->fixer->endChangeset();
+                    $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()