Skip to content

Commit c3780f2

Browse files
sirbrilligsirbrillig
authored
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
1 parent 8d402fc commit c3780f2

File tree

4 files changed

+91
-61
lines changed

4 files changed

+91
-61
lines changed

‎Tests/VariableAnalysisSniff/VariableAnalysisTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,8 @@ public function testCompactWarnings()
522522
23,
523523
26,
524524
36,
525+
41,
526+
42,
525527
];
526528
$this->assertSame($expectedWarnings, $lines);
527529
}
@@ -543,6 +545,8 @@ public function testCompactWarningsHaveCorrectSniffCodes()
543545
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[26][66][0]['source']);
544546
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[36][5][0]['source']);
545547
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[36][23][0]['source']);
548+
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[41][22][0]['source']);
549+
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[42][39][0]['source']);
546550
}
547551

548552
public function testTraitAllowsThis()

‎Tests/VariableAnalysisSniff/fixtures/CompactFixture.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,17 @@ function foo() {
3535
$a = 'Hello';
3636
$c = compact( $a, $b ); // Unused variable c and undefined variable b
3737
}
38+
39+
function function_with_arrow_function_and_compact() {
40+
$make_array = fn ($arg) => compact('arg');
41+
$make_nothing = fn ($arg) => []; // Unused variable $arg
42+
$make_no_variable = fn () => compact('arg');
43+
echo $make_array('hello');
44+
echo $make_nothing('hello');
45+
echo $make_no_variable();
46+
$make_array_multiple = fn ($arg1, $arg2, $arg3) => compact('arg1', 'arg2', 'arg3');
47+
echo $make_array_multiple();
48+
$outside_var = 'hello world';
49+
$use_outside_var = fn($inside_var) => compact('outside_var', 'inside_var');
50+
echo $use_outside_var();
51+
}

‎VariableAnalysis/Lib/Helpers.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHP_CodeSniffer\Files\File;
66
use VariableAnalysis\Lib\ScopeInfo;
7+
use VariableAnalysis\Lib\Constants;
78
use VariableAnalysis\Lib\ForLoopInfo;
89
use VariableAnalysis\Lib\EnumInfo;
910
use VariableAnalysis\Lib\ScopeType;
@@ -445,6 +446,68 @@ public static function findVariableScope(File $phpcsFile, $stackPtr, $varName =
445446
return self::findVariableScopeExceptArrowFunctions($phpcsFile, $stackPtr);
446447
}
447448

449+
/**
450+
* Return the variable names and positions of each variable targetted by a `compact()` call.
451+
*
452+
* @param File $phpcsFile
453+
* @param int $stackPtr
454+
* @param array<int, array<int>> $arguments The stack pointers of each argument; see findFunctionCallArguments
455+
*
456+
* @return array<VariableInfo> each variable's firstRead position and its name; other VariableInfo properties are not set!
457+
*/
458+
public static function getVariablesInsideCompact(File $phpcsFile, $stackPtr, $arguments)
459+
{
460+
$tokens = $phpcsFile->getTokens();
461+
$variablePositionsAndNames = [];
462+
463+
foreach ($arguments as $argumentPtrs) {
464+
$argumentPtrs = array_values(array_filter($argumentPtrs, function ($argumentPtr) use ($tokens) {
465+
return isset(Tokens::$emptyTokens[$tokens[$argumentPtr]['code']]) === false;
466+
}));
467+
if (empty($argumentPtrs)) {
468+
continue;
469+
}
470+
if (!isset($tokens[$argumentPtrs[0]])) {
471+
continue;
472+
}
473+
$argumentFirstToken = $tokens[$argumentPtrs[0]];
474+
if ($argumentFirstToken['code'] === T_ARRAY) {
475+
// It's an array argument, recurse.
476+
$arrayArguments = self::findFunctionCallArguments($phpcsFile, $argumentPtrs[0]);
477+
$variablePositionsAndNames = array_merge($variablePositionsAndNames, self::getVariablesInsideCompact($phpcsFile, $stackPtr, $arrayArguments));
478+
continue;
479+
}
480+
if (count($argumentPtrs) > 1) {
481+
// Complex argument, we can't handle it, ignore.
482+
continue;
483+
}
484+
if ($argumentFirstToken['code'] === T_CONSTANT_ENCAPSED_STRING) {
485+
// Single-quoted string literal, ie compact('whatever').
486+
// Substr is to strip the enclosing single-quotes.
487+
$varName = substr($argumentFirstToken['content'], 1, -1);
488+
$variable = new VariableInfo($varName);
489+
$variable->firstRead = $argumentPtrs[0];
490+
$variablePositionsAndNames[] = $variable;
491+
continue;
492+
}
493+
if ($argumentFirstToken['code'] === T_DOUBLE_QUOTED_STRING) {
494+
// Double-quoted string literal.
495+
$regexp = Constants::getDoubleQuotedVarRegexp();
496+
if (! empty($regexp) && preg_match($regexp, $argumentFirstToken['content'])) {
497+
// Bail if the string needs variable expansion, that's runtime stuff.
498+
continue;
499+
}
500+
// Substr is to strip the enclosing double-quotes.
501+
$varName = substr($argumentFirstToken['content'], 1, -1);
502+
$variable = new VariableInfo($varName);
503+
$variable->firstRead = $argumentPtrs[0];
504+
$variablePositionsAndNames[] = $variable;
505+
continue;
506+
}
507+
}
508+
return $variablePositionsAndNames;
509+
}
510+
448511
/**
449512
* Return the token index of the scope start for a token
450513
*

‎VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php‎

Lines changed: 10 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,61 +1869,6 @@ protected function processVariableInString(File $phpcsFile, $stackPtr)
18691869
}
18701870
}
18711871

1872-
/**
1873-
* @param File $phpcsFile
1874-
* @param int $stackPtr
1875-
* @param array<int, array<int>> $arguments The stack pointers of each argument
1876-
* @param int $currScope
1877-
*
1878-
* @return void
1879-
*/
1880-
protected function processCompactArguments(File $phpcsFile, $stackPtr, $arguments, $currScope)
1881-
{
1882-
$tokens = $phpcsFile->getTokens();
1883-
1884-
foreach ($arguments as $argumentPtrs) {
1885-
$argumentPtrs = array_values(array_filter($argumentPtrs, function ($argumentPtr) use ($tokens) {
1886-
return isset(Tokens::$emptyTokens[$tokens[$argumentPtr]['code']]) === false;
1887-
}));
1888-
if (empty($argumentPtrs)) {
1889-
continue;
1890-
}
1891-
if (!isset($tokens[$argumentPtrs[0]])) {
1892-
continue;
1893-
}
1894-
$argumentFirstToken = $tokens[$argumentPtrs[0]];
1895-
if ($argumentFirstToken['code'] === T_ARRAY) {
1896-
// It's an array argument, recurse.
1897-
$arrayArguments = Helpers::findFunctionCallArguments($phpcsFile, $argumentPtrs[0]);
1898-
$this->processCompactArguments($phpcsFile, $stackPtr, $arrayArguments, $currScope);
1899-
continue;
1900-
}
1901-
if (count($argumentPtrs) > 1) {
1902-
// Complex argument, we can't handle it, ignore.
1903-
continue;
1904-
}
1905-
if ($argumentFirstToken['code'] === T_CONSTANT_ENCAPSED_STRING) {
1906-
// Single-quoted string literal, ie compact('whatever').
1907-
// Substr is to strip the enclosing single-quotes.
1908-
$varName = substr($argumentFirstToken['content'], 1, -1);
1909-
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $argumentPtrs[0], $currScope);
1910-
continue;
1911-
}
1912-
if ($argumentFirstToken['code'] === T_DOUBLE_QUOTED_STRING) {
1913-
// Double-quoted string literal.
1914-
$regexp = Constants::getDoubleQuotedVarRegexp();
1915-
if (! empty($regexp) && preg_match($regexp, $argumentFirstToken['content'])) {
1916-
// Bail if the string needs variable expansion, that's runtime stuff.
1917-
continue;
1918-
}
1919-
// Substr is to strip the enclosing double-quotes.
1920-
$varName = substr($argumentFirstToken['content'], 1, -1);
1921-
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $argumentPtrs[0], $currScope);
1922-
continue;
1923-
}
1924-
}
1925-
}
1926-
19271872
/**
19281873
* Called to process variables named in a call to compact().
19291874
*
@@ -1934,13 +1879,17 @@ protected function processCompactArguments(File $phpcsFile, $stackPtr, $argument
19341879
*/
19351880
protected function processCompact(File $phpcsFile, $stackPtr)
19361881
{
1937-
$currScope = Helpers::findVariableScope($phpcsFile, $stackPtr);
1938-
if ($currScope === null) {
1939-
return;
1940-
}
1941-
1882+
Helpers::debug("processCompact at {$stackPtr}");
19421883
$arguments = Helpers::findFunctionCallArguments($phpcsFile, $stackPtr);
1943-
$this->processCompactArguments($phpcsFile, $stackPtr, $arguments, $currScope);
1884+
$variables = Helpers::getVariablesInsideCompact($phpcsFile, $stackPtr, $arguments);
1885+
foreach ($variables as $variable) {
1886+
$currScope = Helpers::findVariableScope($phpcsFile, $stackPtr, $variable->name);
1887+
if ($currScope === null) {
1888+
continue;
1889+
}
1890+
$variablePosition = $variable->firstRead ? $variable->firstRead : $stackPtr;
1891+
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $variable->name, $variablePosition, $currScope);
1892+
}
19441893
}
19451894

19461895
/**

0 commit comments

Comments
 (0)