PSR2/PropertyDeclaration: update for properties with asymmetric visibility [2]

jrfnl · jrfnl · commit d9f18793db7b · 2025-06-06T02:10:38.000+02:00
Per the FIG PSR-PER updates (next version), the "set-visibility MUST be _after_ the (read-)visibility".

This commit implements this, but as there may be two visibility modifiers, a minor refactor of the code involved in the wrong order checks for the `final` keyword and the `static` and `readonly` keywords is also needed as we can no longer presume that we can use the visibility keyword found with a search backwards to re-position other keywords.

Includes plenty of tests.
Includes a minor update to pre-existing test expectations (whitespace only), which is related to the refactor of the "fix"-code. As this sniff does not enforce any spacing requirements after the keywords, this difference is neglicable for the purpose of this sniff.

Note: in a future iteration, we may want to look at making the code for the "order" checks more efficient, but for now, at least it will work correctly again.

Refs:
* php-fig/per-coding-style 99
* php-fig/per-coding-style 114
diff --git a/src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php b/src/Standards/PSR2/Sniffs/Classes/PropertyDeclarationSniff.php
@@ -121,23 +121,57 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
         }
 
         /*
-         * Note: per PSR-PER section 4.6, the order should be:
+         * Note: per PSR-PER section 4.6 v 2.1/3.0, the order should be:
          * - Inheritance modifier: `abstract` or `final`.
          * - Visibility modifier: `public`, `protected`, or `private`.
+         * - Set-visibility modifier: `public(set)`, `protected(set)`, or `private(set)`
          * - Scope modifier: `static`.
          * - Mutation modifier: `readonly`.
          * - Type declaration.
          * - Name.
          *
-         * Ref: https://www.php-fig.org/per/coding-style/#46-modifier-keywords
+         * Ref:
+         * - https://www.php-fig.org/per/coding-style/#46-modifier-keywords
+         * - https://github.com/php-fig/per-coding-style/pull/99
          *
          * 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_final'] === true) {
-            $scopePtr = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
+        $hasVisibilityModifier   = ($propertyInfo['scope_specified'] === true || $propertyInfo['set_scope'] !== false);
+        $lastVisibilityModifier  = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
+        $firstVisibilityModifier = $lastVisibilityModifier;
+
+        if ($propertyInfo['scope_specified'] === true && $propertyInfo['set_scope'] !== false) {
+            $scopePtr    = $phpcsFile->findPrevious([T_PUBLIC, T_PROTECTED, T_PRIVATE], ($stackPtr - 1));
+            $setScopePtr = $phpcsFile->findPrevious([T_PUBLIC_SET, T_PROTECTED_SET, T_PRIVATE_SET], ($stackPtr - 1));
+            if ($scopePtr > $setScopePtr) {
+                $error = 'The "read"-visibility must come before the "write"-visibility';
+                $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'AvizKeywordOrder');
+                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($setScopePtr, $tokens[$scopePtr]['content'].' ');
+
+                    $phpcsFile->fixer->endChangeset();
+                }
+            }
+
+            $firstVisibilityModifier = min($scopePtr, $setScopePtr);
+        }//end if
+
+        if ($hasVisibilityModifier === true && $propertyInfo['is_final'] === true) {
+            $scopePtr = $firstVisibilityModifier;
             $finalPtr = $phpcsFile->findPrevious(T_FINAL, ($stackPtr - 1));
             if ($finalPtr > $scopePtr) {
                 $error = 'The final declaration must come before the visibility declaration';
@@ -161,50 +195,50 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
             }
         }//end if
 
-        if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_static'] === true) {
-            $scopePtr  = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
+        if ($hasVisibilityModifier === true && $propertyInfo['is_static'] === true) {
+            $scopePtr  = $lastVisibilityModifier;
             $staticPtr = $phpcsFile->findPrevious(T_STATIC, ($stackPtr - 1));
             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();
 
-                    for ($i = ($scopePtr + 1); $scopePtr < $stackPtr; $i++) {
+                    for ($i = ($staticPtr + 1); $staticPtr < $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($staticPtr, '');
+                    $phpcsFile->fixer->addContent($scopePtr, ' '.$tokens[$staticPtr]['content']);
 
                     $phpcsFile->fixer->endChangeset();
                 }
             }
         }//end if
 
-        if ($propertyInfo['scope_specified'] === true && $propertyInfo['is_readonly'] === true) {
-            $scopePtr    = $phpcsFile->findPrevious(Tokens::$scopeModifiers, ($stackPtr - 1));
+        if ($hasVisibilityModifier === true && $propertyInfo['is_readonly'] === true) {
+            $scopePtr    = $lastVisibilityModifier;
             $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++) {
+                    for ($i = ($readonlyPtr + 1); $readonlyPtr < $stackPtr; $i++) {
                         if ($tokens[$i]['code'] !== T_WHITESPACE) {
                             break;
                         }
 
                         $phpcsFile->fixer->replaceToken($i, '');
                     }
 
-                    $phpcsFile->fixer->replaceToken($scopePtr, '');
-                    $phpcsFile->fixer->addContentBefore($readonlyPtr, $propertyInfo['scope'].' ');
+                    $phpcsFile->fixer->replaceToken($readonlyPtr, '');
+                    $phpcsFile->fixer->addContent($scopePtr, ' '.$tokens[$readonlyPtr]['content']);
 
                     $phpcsFile->fixer->endChangeset();
                 }
diff --git a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc
@@ -109,4 +109,20 @@ class AsymmetricVisibility {
     protected(set) public int $wrongOrder1;
 
     private(set) protected ?string $wrongOrder2;
+
+    final protected private(set) static bool $correctOrder;
+
+    private(set) static final protected bool $wrongOrder3;
+    private(set) static protected final bool $wrongOrder4;
+    final protected(set) static public bool $wrongOrder5;
+    static public(set) final public bool $wrongOrder6;
+
+    protected private(set) static final bool $wrongOrder7;
+    protected final private(set) static bool $wrongOrder8;
+    static public final protected(set) bool $wrongOrder9;
+    public static public(set) final bool $wrongOrder10;
+
+    private(set) static final bool $wrongOrder11;
+    final static protected(set) bool $wrongOrder12;
+    static public(set) final bool $wrongOrder13;
 }
diff --git a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.inc.fixed
@@ -37,8 +37,8 @@ class ABC {
     private static $propC;
     public static $propD;
     protected static
-        $propE;
-    private static /*comment*/   $propF;
+            $propE;
+    private static    /*comment*/   $propF;
 }
 
 class MyClass
@@ -103,7 +103,23 @@ class AsymmetricVisibility {
 
     protected(set) array $unfixed;
 
-    protected(set) public int $wrongOrder1;
+    public protected(set) int $wrongOrder1;
 
-    private(set) protected ?string $wrongOrder2;
+    protected private(set) ?string $wrongOrder2;
+
+    final protected private(set) static bool $correctOrder;
+
+    final protected private(set) static bool $wrongOrder3;
+    final protected private(set) static bool $wrongOrder4;
+    final public protected(set) static bool $wrongOrder5;
+    final public public(set) static bool $wrongOrder6;
+
+    final protected private(set) static bool $wrongOrder7;
+    final protected private(set) static bool $wrongOrder8;
+    final public protected(set) static bool $wrongOrder9;
+    final public public(set) static bool $wrongOrder10;
+
+    final private(set) static bool $wrongOrder11;
+    final protected(set) static bool $wrongOrder12;
+    final public(set) static bool $wrongOrder13;
 }
diff --git a/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php b/src/Standards/PSR2/Tests/Classes/PropertyDeclarationUnitTest.php
@@ -63,6 +63,19 @@ public function getErrorList()
             97  => 2,
             101 => 1,
             105 => 1,
+            109 => 1,
+            111 => 1,
+            115 => 3,
+            116 => 3,
+            117 => 2,
+            118 => 3,
+            120 => 1,
+            121 => 1,
+            122 => 2,
+            123 => 2,
+            125 => 1,
+            126 => 1,
+            127 => 2,
         ];
 
     }//end getErrorList()
