From bdf1e50d105d7213b3f9a7efb1a0dce4e182fb19 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Fri, 31 Mar 2023 11:26:42 -0400 Subject: [PATCH] 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 --- .../ArrowFunctionTest.php | 4 + .../fixtures/ArrowFunctionFixture.php | 35 +++++++ VariableAnalysis/Lib/Helpers.php | 94 +++++++++++++++++-- 3 files changed, 123 insertions(+), 10 deletions(-) diff --git a/Tests/VariableAnalysisSniff/ArrowFunctionTest.php b/Tests/VariableAnalysisSniff/ArrowFunctionTest.php index cf4b41f..60b25ab 100644 --- 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 index f81518d..89f466e 100644 --- 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 index 5e961e7..720b00b 100644 --- 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,