From c3780f281e2e82ab1f2de7fa85a16532b1e44592 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 1 Dec 2024 17:31:44 -0500 Subject: [PATCH] Handle compact inside arrow functions (#339) * Add test for compact used within arrow function * Process every variable in compact separately * Also track position of compact variables * Guard against missing compact variable position * Fix linting errors * Also add test for outside var --- .../VariableAnalysisTest.php | 4 ++ .../fixtures/CompactFixture.php | 14 ++++ VariableAnalysis/Lib/Helpers.php | 63 ++++++++++++++++ .../CodeAnalysis/VariableAnalysisSniff.php | 71 +++---------------- 4 files changed, 91 insertions(+), 61 deletions(-) diff --git a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php index 4fe7516..ed11f48 100644 --- a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php +++ b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php @@ -522,6 +522,8 @@ public function testCompactWarnings() 23, 26, 36, + 41, + 42, ]; $this->assertSame($expectedWarnings, $lines); } @@ -543,6 +545,8 @@ public function testCompactWarningsHaveCorrectSniffCodes() $this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[26][66][0]['source']); $this->assertSame(self::UNUSED_ERROR_CODE, $warnings[36][5][0]['source']); $this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[36][23][0]['source']); + $this->assertSame(self::UNUSED_ERROR_CODE, $warnings[41][22][0]['source']); + $this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[42][39][0]['source']); } public function testTraitAllowsThis() diff --git a/Tests/VariableAnalysisSniff/fixtures/CompactFixture.php b/Tests/VariableAnalysisSniff/fixtures/CompactFixture.php index d259b1b..e10a2f9 100644 --- a/Tests/VariableAnalysisSniff/fixtures/CompactFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/CompactFixture.php @@ -35,3 +35,17 @@ function foo() { $a = 'Hello'; $c = compact( $a, $b ); // Unused variable c and undefined variable b } + +function function_with_arrow_function_and_compact() { + $make_array = fn ($arg) => compact('arg'); + $make_nothing = fn ($arg) => []; // Unused variable $arg + $make_no_variable = fn () => compact('arg'); + echo $make_array('hello'); + echo $make_nothing('hello'); + echo $make_no_variable(); + $make_array_multiple = fn ($arg1, $arg2, $arg3) => compact('arg1', 'arg2', 'arg3'); + echo $make_array_multiple(); + $outside_var = 'hello world'; + $use_outside_var = fn($inside_var) => compact('outside_var', 'inside_var'); + echo $use_outside_var(); +} diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index 6af1b22..73164e1 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -4,6 +4,7 @@ use PHP_CodeSniffer\Files\File; use VariableAnalysis\Lib\ScopeInfo; +use VariableAnalysis\Lib\Constants; use VariableAnalysis\Lib\ForLoopInfo; use VariableAnalysis\Lib\EnumInfo; use VariableAnalysis\Lib\ScopeType; @@ -445,6 +446,68 @@ public static function findVariableScope(File $phpcsFile, $stackPtr, $varName = return self::findVariableScopeExceptArrowFunctions($phpcsFile, $stackPtr); } + /** + * Return the variable names and positions of each variable targetted by a `compact()` call. + * + * @param File $phpcsFile + * @param int $stackPtr + * @param array> $arguments The stack pointers of each argument; see findFunctionCallArguments + * + * @return array each variable's firstRead position and its name; other VariableInfo properties are not set! + */ + public static function getVariablesInsideCompact(File $phpcsFile, $stackPtr, $arguments) + { + $tokens = $phpcsFile->getTokens(); + $variablePositionsAndNames = []; + + foreach ($arguments as $argumentPtrs) { + $argumentPtrs = array_values(array_filter($argumentPtrs, function ($argumentPtr) use ($tokens) { + return isset(Tokens::$emptyTokens[$tokens[$argumentPtr]['code']]) === false; + })); + if (empty($argumentPtrs)) { + continue; + } + if (!isset($tokens[$argumentPtrs[0]])) { + continue; + } + $argumentFirstToken = $tokens[$argumentPtrs[0]]; + if ($argumentFirstToken['code'] === T_ARRAY) { + // It's an array argument, recurse. + $arrayArguments = self::findFunctionCallArguments($phpcsFile, $argumentPtrs[0]); + $variablePositionsAndNames = array_merge($variablePositionsAndNames, self::getVariablesInsideCompact($phpcsFile, $stackPtr, $arrayArguments)); + continue; + } + if (count($argumentPtrs) > 1) { + // Complex argument, we can't handle it, ignore. + continue; + } + if ($argumentFirstToken['code'] === T_CONSTANT_ENCAPSED_STRING) { + // Single-quoted string literal, ie compact('whatever'). + // Substr is to strip the enclosing single-quotes. + $varName = substr($argumentFirstToken['content'], 1, -1); + $variable = new VariableInfo($varName); + $variable->firstRead = $argumentPtrs[0]; + $variablePositionsAndNames[] = $variable; + continue; + } + if ($argumentFirstToken['code'] === T_DOUBLE_QUOTED_STRING) { + // Double-quoted string literal. + $regexp = Constants::getDoubleQuotedVarRegexp(); + if (! empty($regexp) && preg_match($regexp, $argumentFirstToken['content'])) { + // Bail if the string needs variable expansion, that's runtime stuff. + continue; + } + // Substr is to strip the enclosing double-quotes. + $varName = substr($argumentFirstToken['content'], 1, -1); + $variable = new VariableInfo($varName); + $variable->firstRead = $argumentPtrs[0]; + $variablePositionsAndNames[] = $variable; + continue; + } + } + return $variablePositionsAndNames; + } + /** * Return the token index of the scope start for a token * diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 531cd40..5eaea94 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -1869,61 +1869,6 @@ protected function processVariableInString(File $phpcsFile, $stackPtr) } } - /** - * @param File $phpcsFile - * @param int $stackPtr - * @param array> $arguments The stack pointers of each argument - * @param int $currScope - * - * @return void - */ - protected function processCompactArguments(File $phpcsFile, $stackPtr, $arguments, $currScope) - { - $tokens = $phpcsFile->getTokens(); - - foreach ($arguments as $argumentPtrs) { - $argumentPtrs = array_values(array_filter($argumentPtrs, function ($argumentPtr) use ($tokens) { - return isset(Tokens::$emptyTokens[$tokens[$argumentPtr]['code']]) === false; - })); - if (empty($argumentPtrs)) { - continue; - } - if (!isset($tokens[$argumentPtrs[0]])) { - continue; - } - $argumentFirstToken = $tokens[$argumentPtrs[0]]; - if ($argumentFirstToken['code'] === T_ARRAY) { - // It's an array argument, recurse. - $arrayArguments = Helpers::findFunctionCallArguments($phpcsFile, $argumentPtrs[0]); - $this->processCompactArguments($phpcsFile, $stackPtr, $arrayArguments, $currScope); - continue; - } - if (count($argumentPtrs) > 1) { - // Complex argument, we can't handle it, ignore. - continue; - } - if ($argumentFirstToken['code'] === T_CONSTANT_ENCAPSED_STRING) { - // Single-quoted string literal, ie compact('whatever'). - // Substr is to strip the enclosing single-quotes. - $varName = substr($argumentFirstToken['content'], 1, -1); - $this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $argumentPtrs[0], $currScope); - continue; - } - if ($argumentFirstToken['code'] === T_DOUBLE_QUOTED_STRING) { - // Double-quoted string literal. - $regexp = Constants::getDoubleQuotedVarRegexp(); - if (! empty($regexp) && preg_match($regexp, $argumentFirstToken['content'])) { - // Bail if the string needs variable expansion, that's runtime stuff. - continue; - } - // Substr is to strip the enclosing double-quotes. - $varName = substr($argumentFirstToken['content'], 1, -1); - $this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $argumentPtrs[0], $currScope); - continue; - } - } - } - /** * Called to process variables named in a call to compact(). * @@ -1934,13 +1879,17 @@ protected function processCompactArguments(File $phpcsFile, $stackPtr, $argument */ protected function processCompact(File $phpcsFile, $stackPtr) { - $currScope = Helpers::findVariableScope($phpcsFile, $stackPtr); - if ($currScope === null) { - return; - } - + Helpers::debug("processCompact at {$stackPtr}"); $arguments = Helpers::findFunctionCallArguments($phpcsFile, $stackPtr); - $this->processCompactArguments($phpcsFile, $stackPtr, $arguments, $currScope); + $variables = Helpers::getVariablesInsideCompact($phpcsFile, $stackPtr, $arguments); + foreach ($variables as $variable) { + $currScope = Helpers::findVariableScope($phpcsFile, $stackPtr, $variable->name); + if ($currScope === null) { + continue; + } + $variablePosition = $variable->firstRead ? $variable->firstRead : $stackPtr; + $this->markVariableReadAndWarnIfUndefined($phpcsFile, $variable->name, $variablePosition, $currScope); + } } /**