Squiz/DisallowMultipleAssignments: bug fix - dynamic property assignm…

…ent on object stored in array The sniff would try to find whether the first "relevant" variable found matches the start of the statement and if not, throw an error, but in the case of dynamic property access on objects stored in an array, the first "relevant" variable determination was off and would get stuck on the `]` close bracket of the array access. Fixed now. Includes ample tests. This should also make the sniff slightly more efficient for property access code snippets which the sniff already handled correctly. Fixes 598
PHPCSStandards · Sep 14, 2024 · 0ab692a · 0ab692a
1 parent b87dafd
commit 0ab692a
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 9 deletions.
diff --git a/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php b/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php
@@ -105,6 +105,13 @@ public function process(File $phpcsFile, $stackPtr)
             }
 
             if ($tokens[$varToken]['code'] === T_VARIABLE) {
+                $prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($varToken - 1), null, true);
+                if ($tokens[$prevNonEmpty]['code'] === T_OBJECT_OPERATOR) {
+                    // Dynamic property access, the real "start" variable still needs to be found.
+                    $varToken = $prevNonEmpty;
+                    continue;
+                }
+
                 // We found our variable.
                 break;
             }
@@ -119,15 +126,14 @@ public function process(File $phpcsFile, $stackPtr)
 
         $allowed = Tokens::$emptyTokens;
 
-        $allowed[T_STRING]          = T_STRING;
-        $allowed[T_NS_SEPARATOR]    = T_NS_SEPARATOR;
-        $allowed[T_DOUBLE_COLON]    = T_DOUBLE_COLON;
-        $allowed[T_OBJECT_OPERATOR] = T_OBJECT_OPERATOR;
-        $allowed[T_ASPERAND]        = T_ASPERAND;
-        $allowed[T_DOLLAR]          = T_DOLLAR;
-        $allowed[T_SELF]            = T_SELF;
-        $allowed[T_PARENT]          = T_PARENT;
-        $allowed[T_STATIC]          = T_STATIC;
+        $allowed[T_STRING]       = T_STRING;
+        $allowed[T_NS_SEPARATOR] = T_NS_SEPARATOR;
+        $allowed[T_DOUBLE_COLON] = T_DOUBLE_COLON;
+        $allowed[T_ASPERAND]     = T_ASPERAND;
+        $allowed[T_DOLLAR]       = T_DOLLAR;
+        $allowed[T_SELF]         = T_SELF;
+        $allowed[T_PARENT]       = T_PARENT;
+        $allowed[T_STATIC]       = T_STATIC;
 
         $varToken = $phpcsFile->findPrevious($allowed, ($varToken - 1), null, true);
 

diff --git a/src/Standards/Squiz/Tests/PHP/DisallowMultipleAssignmentsUnitTest.1.inc b/src/Standards/Squiz/Tests/PHP/DisallowMultipleAssignmentsUnitTest.1.inc
@@ -134,3 +134,17 @@ list ($c, $d) = explode(',', '1,2');
 <?= $a * $b ?>
 <?= [$c, $d] = explode(',', '1,2');
 ?>
+<?php
+
+// Issue #598.
+$filtered_results->field = $result;
+$filtered_results->$field = $result;
+$filtered_results->$row->field = $result;
+$filtered_results->$row->$field = $result;
+
+$filtered_results[ $i ]->field = $result;
+$filtered_results[ $i ]->$field = $result;
+$filtered_results[ $i ]->$row->field = $result;
+$filtered_results[ $i ]->$row->$field = $result;
+$filtered_results[ $i ]->$row[0]->field = $result;
+$filtered_results[ $i ]->$row[$j]->$field = $result;