From c019db09aafe18c0d929f9c55a47b55c5698f438 Mon Sep 17 00:00:00 2001
From: Payton Swick <payton@foolord.com>
Date: Sun, 24 Mar 2024 17:44:22 -0400
Subject: [PATCH] Make sure that recursive search of list assignments stays
 inside them (#318)

* Add tests for destructuring assignment with array arg

* Make sure that recursive search of list assignments stays inside them
---
 .../fixtures/DestructuringFixture.php         | 10 +++++
 VariableAnalysis/Lib/Helpers.php              | 38 ++++++++++++++++---
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Tests/VariableAnalysisSniff/fixtures/DestructuringFixture.php b/Tests/VariableAnalysisSniff/fixtures/DestructuringFixture.php
index 8c037d26..14c113f6 100644
--- a/Tests/VariableAnalysisSniff/fixtures/DestructuringFixture.php
+++ b/Tests/VariableAnalysisSniff/fixtures/DestructuringFixture.php
@@ -48,3 +48,13 @@ function function_with_nested_destructure_using_short_list() {
     echo $foo;
     echo $bar;
 }
+
+function function_with_short_destructuring_assignment_and_array_arg(int $baz) {
+	[$bar] = doSomething([$baz]);
+	return $bar;
+}
+
+function function_with_destructuring_assignment_and_array_arg(int $baz) {
+	list($bar) = doSomething([$baz]);
+	return $bar;
+}
diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php
index 1050de96..18d34770 100644
--- a/VariableAnalysis/Lib/Helpers.php
+++ b/VariableAnalysis/Lib/Helpers.php
@@ -37,6 +37,9 @@ public static function getIntOrNull($value)
 	}
 
 	/**
+	 * Find the position of the square bracket containing the token at $stackPtr,
+	 * if any.
+	 *
 	 * @param File $phpcsFile
 	 * @param int  $stackPtr
 	 *
@@ -44,8 +47,26 @@ public static function getIntOrNull($value)
 	 */
 	public static function findContainingOpeningSquareBracket(File $phpcsFile, $stackPtr)
 	{
+		// Find the previous bracket within this same statement.
 		$previousStatementPtr = self::getPreviousStatementPtr($phpcsFile, $stackPtr);
-		return self::getIntOrNull($phpcsFile->findPrevious([T_OPEN_SHORT_ARRAY, T_OPEN_SQUARE_BRACKET], $stackPtr - 1, $previousStatementPtr));
+		$openBracketPosition = self::getIntOrNull($phpcsFile->findPrevious([T_OPEN_SHORT_ARRAY, T_OPEN_SQUARE_BRACKET], $stackPtr - 1, $previousStatementPtr));
+		if (empty($openBracketPosition)) {
+			return null;
+		}
+		// Make sure we are inside the pair of brackets we found.
+		$tokens = $phpcsFile->getTokens();
+		$openBracketToken = $tokens[$openBracketPosition];
+		if (empty($openBracketToken) || empty($tokens[$openBracketToken['bracket_closer']])) {
+			return null;
+		}
+		$closeBracketPosition = $openBracketToken['bracket_closer'];
+		if (empty($closeBracketPosition)) {
+			return null;
+		}
+		if ($stackPtr > $closeBracketPosition) {
+			return null;
+		}
+		return $openBracketPosition;
 	}
 
 	/**
@@ -835,10 +856,17 @@ public static function getListAssignments(File $phpcsFile, $listOpenerIndex)
 			$parents = isset($tokens[$listOpenerIndex]['nested_parenthesis']) ? $tokens[$listOpenerIndex]['nested_parenthesis'] : [];
 			// There's no record of nested brackets for short lists; we'll have to find the parent ourselves
 			if (empty($parents)) {
-				$parentSquareBracket = self::findContainingOpeningSquareBracket($phpcsFile, $listOpenerIndex);
-				if (is_int($parentSquareBracket)) {
-					// Collect the opening index, but we don't actually need the closing paren index so just make that 0
-					$parents = [$parentSquareBracket => 0];
+				$parentSquareBracketPtr = self::findContainingOpeningSquareBracket($phpcsFile, $listOpenerIndex);
+				if (is_int($parentSquareBracketPtr)) {
+					// Make sure that the parent is really a parent by checking that its
+					// closing index is outside of the current bracket's closing index.
+					$parentSquareBracketToken = $tokens[$parentSquareBracketPtr];
+					$parentSquareBracketClosePtr = $parentSquareBracketToken['bracket_closer'];
+					if ($parentSquareBracketClosePtr && $parentSquareBracketClosePtr > $closePtr) {
+						self::debug("found enclosing bracket for {$listOpenerIndex}: {$parentSquareBracketPtr}");
+						// Collect the opening index, but we don't actually need the closing paren index so just make that 0
+						$parents = [$parentSquareBracketPtr => 0];
+					}
 				}
 			}
 			// If we have no parents, this is not a nested assignment and therefore is not an assignment