Skip to content

Commit

Permalink
Handle compact inside arrow functions (#339)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@sirbrillig
sirbrillig authored Dec 1, 2024
1 parent 8d402fc commit c3780f2
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 61 deletions.
4 changes: 4 additions & 0 deletions Tests/VariableAnalysisSniff/VariableAnalysisTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,8 @@ public function testCompactWarnings()
23,
26,
36,
41,
42,
];
$this->assertSame($expectedWarnings, $lines);
}
Expand All @@ -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()
Expand Down
14 changes: 14 additions & 0 deletions Tests/VariableAnalysisSniff/fixtures/CompactFixture.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
63 changes: 63 additions & 0 deletions VariableAnalysis/Lib/Helpers.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<int, array<int>> $arguments The stack pointers of each argument; see findFunctionCallArguments
*
* @return array<VariableInfo> 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
*
Expand Down
71 changes: 10 additions & 61 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1869,61 +1869,6 @@ protected function processVariableInString(File $phpcsFile, $stackPtr)
}
}

/**
* @param File $phpcsFile
* @param int $stackPtr
* @param array<int, array<int>> $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().
*
Expand All @@ -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);
}
}

/**
Expand Down

0 comments on commit c3780f2

Please sign in to comment.