Skip to content

Handle compact inside arrow functions #339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 1, 2024
Merged

Handle compact inside arrow functions #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
99e3049
Add test for compact used within arrow function
sirbrillig Nov 30, 2024
78f099a
Process every variable in compact separately
sirbrillig Dec 1, 2024
445ed17
Also track position of compact variables
sirbrillig Dec 1, 2024
a792e23
Guard against missing compact variable position
sirbrillig Dec 1, 2024
b1b9871
Fix linting errors
sirbrillig Dec 1, 2024
e2f7e3d
Also add test for outside var
sirbrillig Dec 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Tests/VariableAnalysisSniff/VariableAnalysisTest.php
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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