From 0ab692aba09f4483ae0bea416421f4354a7801f3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 8 Sep 2024 21:14:50 +0200 Subject: [PATCH] Squiz/DisallowMultipleAssignments: bug fix - dynamic property assignment 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 --- .../PHP/DisallowMultipleAssignmentsSniff.php | 24 ++++++++++++------- .../DisallowMultipleAssignmentsUnitTest.1.inc | 14 +++++++++++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php b/src/Standards/Squiz/Sniffs/PHP/DisallowMultipleAssignmentsSniff.php index 688d0bca70..2a953b4625 100644 --- 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 index b4d63fcad0..960f907de2 100644 --- 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'); +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;