Fix scope closer detection in arrow function detection (#298)

* Add test for ternary in arrow function * Do not treat closing paren as a scope closer for arrow func * Add more tests * Add detection for function parameters in arrow function detection * Stop using null coalesce * Fix formatting * Add test for array in args * Support scope closer from phpcs if we could not find one * Add test for simple arrow function in ternary * Change parsing for end token to track pairs * Remove function call inside for loop condition * Add test for arrow function return type * Support arrow functions with return types * Remove accidental artifact
sirbrillig · Mar 31, 2023 · bdf1e50 · bdf1e50
1 parent 4cca2aa
commit bdf1e50
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 10 deletions.
diff --git a/Tests/VariableAnalysisSniff/ArrowFunctionTest.php b/Tests/VariableAnalysisSniff/ArrowFunctionTest.php
@@ -30,6 +30,8 @@ public function testArrowFunctions()
 			67,
 			71,
 			87,
+			102,
+			112,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}
@@ -60,6 +62,8 @@ public function testArrowFunctionsWithoutUnusedBeforeUsed()
 			67,
 			71,
 			87,
+			102,
+			112,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}

diff --git a/Tests/VariableAnalysisSniff/fixtures/ArrowFunctionFixture.php b/Tests/VariableAnalysisSniff/fixtures/ArrowFunctionFixture.php
@@ -92,3 +92,38 @@ function arrowFunctionAsExpressionInArgumentWithArray() {
     $type = do_something(fn($array, $needle) => $array[2] === $needle);
     echo $type;
 }
+
+function arrowFunctionAsExpressionInArgumentWithInnerCall() {
+    $type = do_something(fn(Thing $func) => $func->call() ? $func : null);
+    echo $type;
+}
+
+function arrowFunctionAsExpressionInArgumentWithInnerCallAndUndefinedAfterTernary() {
+    $type = do_something(fn(Thing $func) => $func->call() ? $func : $foo); // undefined variable $foo
+    echo $type;
+}
+
+function arrowFunctionAsExpressionInArgumentWithInnerCallAndArgs() {
+    $type = do_something(fn(Thing $func) => $func->call(1,2) ? $func : null);
+    echo $type;
+}
+
+function arrowFunctionAsExpressionWithUndefinedAfterComma() {
+    $type = do_something(fn(Thing $func, $bar) => $func->call(1,2) ? $bar : null, $bar); // undefined variable $bar
+    echo $type;
+}
+
+function arrowFunctionAsExpressionInArgumentWithInnerArrayAndArgs() {
+    $type = do_something(fn(Thing $func) => $func->call([1,2]) ? $func : null);
+    echo $type;
+}
+
+function arrowFunctionAsExpressionInArgumentWithSimpleTernary() {
+    $type = do_something(fn(Thing $func) => $func ? $func : null);
+    echo $type;
+}
+
+function arrowFunctionWithReturnType() {
+    $type = do_something(fn(string $func): string => $func ? $func : '');
+    echo $type;
+}
diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php
@@ -638,26 +638,100 @@ public static function getArrowFunctionOpenClose(File $phpcsFile, $stackPtr)
 		}
 		// Find the associated close parenthesis
 		$closeParenIndex = $tokens[$openParenIndex]['parenthesis_closer'];
-		// Make sure the next token is a fat arrow
+		// Make sure the next token is a fat arrow or a return type
 		$fatArrowIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $closeParenIndex + 1, null, true);
 		if (! is_int($fatArrowIndex)) {
 			return null;
 		}
-		if ($tokens[$fatArrowIndex]['code'] !== T_DOUBLE_ARROW && $tokens[$fatArrowIndex]['type'] !== 'T_FN_ARROW') {
+		if (
+			$tokens[$fatArrowIndex]['code'] !== T_DOUBLE_ARROW &&
+			$tokens[$fatArrowIndex]['type'] !== 'T_FN_ARROW' &&
+			$tokens[$fatArrowIndex]['code'] !== T_COLON
+		) {
 			return null;
 		}
+
 		// Find the scope closer
-		$endScopeTokens = [
-			T_COMMA,
-			T_SEMICOLON,
-			T_CLOSE_PARENTHESIS,
-			T_CLOSE_CURLY_BRACKET,
-			T_CLOSE_SHORT_ARRAY,
-		];
-		$scopeCloserIndex = $phpcsFile->findNext($endScopeTokens, $fatArrowIndex	+ 1);
+		$scopeCloserIndex = null;
+		$foundCurlyPairs = 0;
+		$foundArrayPairs = 0;
+		$foundParenPairs = 0;
+		$arrowBodyStart = $tokens[$stackPtr]['parenthesis_closer'] + 1;
+		$lastToken = self::getLastNonEmptyTokenIndexInFile($phpcsFile);
+		for ($index = $arrowBodyStart; $index < $lastToken; $index++) {
+			$token = $tokens[$index];
+			if (empty($token['code'])) {
+				$scopeCloserIndex = $index;
+				break;
+			}
+
+			// A line break is always a closer.
+			if ($token['line'] !== $tokens[$stackPtr]['line']) {
+				$scopeCloserIndex = $index;
+				break;
+			}
+			$code = $token['code'];
+
+			// A semicolon is always a closer.
+			if ($code === T_SEMICOLON) {
+				$scopeCloserIndex = $index;
+				break;
+			}
+
+			// Track pair opening tokens.
+			if ($code === T_OPEN_CURLY_BRACKET) {
+				$foundCurlyPairs += 1;
+				continue;
+			}
+			if ($code === T_OPEN_SHORT_ARRAY || $code === T_OPEN_SQUARE_BRACKET) {
+				$foundArrayPairs += 1;
+				continue;
+			}
+			if ($code === T_OPEN_PARENTHESIS) {
+				$foundParenPairs += 1;
+				continue;
+			}
+
+			// A pair closing is only an arrow func closer if there was no matching opening token.
+			if ($code === T_CLOSE_CURLY_BRACKET) {
+				if ($foundCurlyPairs === 0) {
+					$scopeCloserIndex = $index;
+					break;
+				}
+				$foundCurlyPairs -= 1;
+				continue;
+			}
+			if ($code === T_CLOSE_SHORT_ARRAY || $code === T_CLOSE_SQUARE_BRACKET) {
+				if ($foundArrayPairs === 0) {
+					$scopeCloserIndex = $index;
+					break;
+				}
+				$foundArrayPairs -= 1;
+				continue;
+			}
+			if ($code === T_CLOSE_PARENTHESIS) {
+				if ($foundParenPairs === 0) {
+					$scopeCloserIndex = $index;
+					break;
+				}
+				$foundParenPairs -= 1;
+				continue;
+			}
+
+			// A comma is a closer only if we are not inside an opening token.
+			if ($code === T_COMMA) {
+				if (empty($foundArrayPairs) && empty($foundParenPairs) && empty($foundCurlyPairs)) {
+					$scopeCloserIndex = $index;
+					break;
+				}
+				continue;
+			}
+		}
+
 		if (! is_int($scopeCloserIndex)) {
 			return null;
 		}
+
 		return [
 			'scope_opener' => $stackPtr,
 			'scope_closer' => $scopeCloserIndex,