Skip to content

Commit b6cd334

Browse files
illusoriillusori
committed
Add support for compact() - literal string/array arguments only.
1 parent 8542524 commit b6cd334

File tree

3 files changed

+126
-11
lines changed

3 files changed

+126
-11
lines changed

‎Sniffs/CodeAnalysis/VariableAnalysisSniff.php‎

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ class Generic_Sniffs_CodeAnalysis_VariableAnalysisSniff implements PHP_CodeSniff
9494
*/
9595
private $_scopes = array();
9696

97+
/**
98+
* A regexp for matching variable names in double-quoted strings.
99+
*/
100+
private $_double_quoted_variable_regexp = '|(?<!\\\\)(?:\\\\{2})*\${?([a-zA-Z0-9_]+)}?|';
101+
97102
/**
98103
* Array of known pass-by-reference functions and the argument(s) which are passed
99104
* by reference, the arguments are numbered starting from 1 and an elipsis '...'
@@ -367,6 +372,7 @@ public function register() {
367372
T_DOUBLE_QUOTED_STRING,
368373
T_HEREDOC,
369374
T_CLOSE_CURLY_BRACKET,
375+
T_STRING,
370376
);
371377
}//end register()
372378

@@ -398,6 +404,9 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr) {
398404
($token['code'] === T_HEREDOC)) {
399405
return $this->processVariableInString($phpcsFile, $stackPtr);
400406
}
407+
if (($token['code'] === T_STRING) && ($token['content'] === 'compact')) {
408+
return $this->processCompact($phpcsFile, $stackPtr);
409+
}
401410
if (($token['code'] === T_CLOSE_CURLY_BRACKET) &&
402411
isset($token['scope_condition'])) {
403412
return $this->processScopeClose($phpcsFile, $token['scope_condition']);
@@ -669,7 +678,9 @@ function findFunctionCallArguments(
669678
) {
670679
$tokens = $phpcsFile->getTokens();
671680

672-
if ($tokens[$stackPtr]['code'] !== T_STRING) {
681+
// Slight hack: also allow this to find args for array constructor.
682+
// TODO: probably should refactor into three functions: arg-finding and bracket-finding
683+
if (($tokens[$stackPtr]['code'] !== T_STRING) && ($tokens[$stackPtr]['code'] !== T_ARRAY)) {
673684
// Assume $stackPtr is something within the brackets, find our function call
674685
if (($stackPtr = $this->findFunctionCall($phpcsFile, $stackPtr)) === false) {
675686
return false;
@@ -1318,8 +1329,7 @@ protected function processVariableInString(
13181329
$tokens = $phpcsFile->getTokens();
13191330
$token = $tokens[$stackPtr];
13201331

1321-
$pattern = '|(?<!\\\\)(?:\\\\{2})*\${?([a-zA-Z0-9_]+)}?|';
1322-
if (!preg_match_all($pattern, $token['content'], $matches)) {
1332+
if (!preg_match_all($this->_double_quoted_variable_regexp, $token['content'], $matches)) {
13231333
return;
13241334
}
13251335

@@ -1337,6 +1347,85 @@ protected function processVariableInString(
13371347
}
13381348
}
13391349

1350+
protected function processCompactArguments(
1351+
PHP_CodeSniffer_File
1352+
$phpcsFile,
1353+
$stackPtr,
1354+
$arguments,
1355+
$currScope
1356+
) {
1357+
$tokens = $phpcsFile->getTokens();
1358+
$token = $tokens[$stackPtr];
1359+
1360+
foreach ($arguments as $argumentPtrs) {
1361+
$argumentPtrs = array_values(array_filter($argumentPtrs,
1362+
function ($argumentPtr) use ($tokens) {
1363+
return $tokens[$argumentPtr]['code'] !== T_WHITESPACE;
1364+
}));
1365+
if (empty($argumentPtrs)) {
1366+
continue;
1367+
}
1368+
if (!isset($tokens[$argumentPtrs[0]])) {
1369+
continue;
1370+
}
1371+
$argument_first_token = $tokens[$argumentPtrs[0]];
1372+
if ($argument_first_token['code'] === T_ARRAY) {
1373+
// It's an array argument, recurse.
1374+
if (($array_arguments = $this->findFunctionCallArguments($phpcsFile, $argumentPtrs[0])) !== false) {
1375+
$this->processCompactArguments($phpcsFile, $stackPtr, $array_arguments, $currScope);
1376+
}
1377+
continue;
1378+
}
1379+
if (count($argumentPtrs) > 1) {
1380+
// Complex argument, we can't handle it, ignore.
1381+
continue;
1382+
}
1383+
if ($argument_first_token['code'] === T_CONSTANT_ENCAPSED_STRING) {
1384+
// Single-quoted string literal, ie compact('whatever').
1385+
// Substr is to strip the enclosing single-quotes.
1386+
$varName = substr($argument_first_token['content'], 1, -1);
1387+
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $argumentPtrs[0], $currScope);
1388+
continue;
1389+
}
1390+
if ($argument_first_token['code'] === T_DOUBLE_QUOTED_STRING) {
1391+
// Double-quoted string literal.
1392+
if (preg_match($this->_double_quoted_variable_regexp, $argument_first_token['content'])) {
1393+
// Bail if the string needs variable expansion, that's runtime stuff.
1394+
continue;
1395+
}
1396+
// Substr is to strip the enclosing double-quotes.
1397+
$varName = substr($argument_first_token['content'], 1, -1);
1398+
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $argumentPtrs[0], $currScope);
1399+
continue;
1400+
}
1401+
}
1402+
}
1403+
1404+
/**
1405+
* Called to process variables named in a call to compact().
1406+
*
1407+
* @param PHP_CodeSniffer_File $phpcsFile The PHP_CodeSniffer file where this
1408+
* token was found.
1409+
* @param int $stackPtr The position where the call to compact()
1410+
* was found.
1411+
*
1412+
* @return void
1413+
*/
1414+
protected function processCompact(
1415+
PHP_CodeSniffer_File
1416+
$phpcsFile,
1417+
$stackPtr
1418+
) {
1419+
$tokens = $phpcsFile->getTokens();
1420+
$token = $tokens[$stackPtr];
1421+
1422+
$currScope = $this->findVariableScope($phpcsFile, $stackPtr);
1423+
1424+
if (($arguments = $this->findFunctionCallArguments($phpcsFile, $stackPtr)) !== false) {
1425+
$this->processCompactArguments($phpcsFile, $stackPtr, $arguments, $currScope);
1426+
}
1427+
}
1428+
13401429
/**
13411430
* Called to process the end of a scope.
13421431
*

‎Tests/CodeAnalysis/VariableAnalysisUnitTest.inc‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -433,14 +433,34 @@ class ClassWithLateStaticBinding {
433433
}
434434
}
435435

436-
function function_with_compact($param1, $param2, $param3, $param4) {
436+
function function_with_literal_compact($param1, $param2, $param3, $param4) {
437437
$var1 = 'value1';
438438
$var2 = 'value2';
439439
$var4 = 'value4';
440-
$squish1 = compact('var1');
441-
$squish2 = compact('var3');
442-
$squish3 = compact('param1');
443-
$squish4 = compact('var2', 'param3');
444-
$squish5 = compact(array('var4'), array('param4', 'var5'));
445-
echo $squish1, $squish2, $squish3, $squish4, $squish5;
440+
$squish = compact('var1');
441+
$squish = compact('var3');
442+
$squish = compact('param1');
443+
$squish = compact('var2', 'param3');
444+
$squish = compact(array('var4'), array('param4', 'var5'));
445+
echo $squish;
446+
}
447+
448+
function function_with_expression_compact($param1, $param2, $param3, $param4) {
449+
$var1 = "value1";
450+
$var2 = "value2";
451+
$var4 = "value4";
452+
$var6 = "value6";
453+
$var7 = "value7";
454+
$var8 = "value8";
455+
$var9 = "value9";
456+
$squish = compact("var1");
457+
$squish = compact("var3");
458+
$squish = compact("param1");
459+
$squish = compact("var2", "param3");
460+
$squish = compact(array("var4"), array("param4", "var5"));
461+
$squish = compact($var6);
462+
$squish = compact("var" . "7");
463+
$squish = compact("blah $var8");
464+
$squish = compact("$var9");
465+
echo $squish;
446466
}

‎Tests/CodeAnalysis/VariableAnalysisUnitTest.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,17 @@ private function _getWarningAndErrorList() {
224224
// method_with_late_static_binding() line (+5)
225225
($base += 5) => 0,
226226
($base + 2) => 1, // $var
227-
// function_with_compact() line (+7)
227+
// function_with_single_quoted_compact() line (+7)
228228
($base += 7) => 0,
229229
($base + 0) => 1, // unused $param2
230230
($base + 5) => 1, // undefined $var3
231231
($base + 8) => 1, // undefined $var5
232+
// function_with_expression_compact() line (+12)
233+
($base += 12) => 0,
234+
($base + 0) => 1, // unused $param2
235+
($base + 5) => 1, // unused $var7
236+
($base + 9) => 1, // undefined $var3
237+
($base + 12) => 1, // undefined $var5
232238
);
233239
}//end _getWarningAndErrorList()
234240

0 commit comments

Comments
 (0)