Skip to content

Commit

Permalink
Option for undefined vars (#56)
Browse files Browse the repository at this point in the history
* Add option to ignore variable seen as undefined

Rather than declaring `global $post` on every single WordPress theme part, I find this option useful, especially when reviewing third-party themes that would otherwise have many warnings.

* Add tests

* update readme
  • Loading branch information
brandonkal authored and sirbrillig committed Nov 24, 2018
1 parent 25823a9 commit 1d89f26
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 3 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ The available options are as follows:
- `allowUnusedCaughtExceptions` (bool, default `false`): if set to true, caught Exception variables will never be marked as unused.
- `validUnusedVariableNames` (string, default `null`): a space-separated list of names of placeholder variables that you want to ignore from unused variable warnings. For example, to ignore the variables `$junk` and `$unused`, this could be set to `'junk unused'`.
- `ignoreUnusedRegexp` (string, default `null`): a PHP regexp string (note that this requires explicit delimiters) for variables that you want to ignore from unused variable warnings. For example, to ignore the variables `$_junk` and `$_unused`, this could be set to `'/^_/'`.
- `validUndefinedVariableNames` (string, default `null`): a space-separated list of names of placeholder variables that you want to ignore from undefined variable warnings. For example, to ignore the variables `$post` and `$undefined`, this could be set to `'post undefined'`.

To set these these options, you must use XML in your ruleset. For details, see the [phpcs customizable sniff properties page](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Customisable-Sniff-Properties). Here is an example that ignores all variables that start with an underscore:

Expand Down
1 change: 1 addition & 0 deletions VariableAnalysis/Lib/VariableInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class VariableInfo {
public $firstInitialized;
public $firstRead;
public $ignoreUnused = false;
public $ignoreUndefined = false;

public static $scopeTypeDescriptions = array(
'local' => 'variable',
Expand Down
16 changes: 16 additions & 0 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ class VariableAnalysisSniff implements Sniff {
*/
public $ignoreUnusedRegexp = null;

/**
* A space-separated list of names of placeholder variables that you want to
* ignore from undefined variable warnings. For example, to ignore the variables
* `$post` and `$undefined`, this could be set to `'post undefined'`.
*/
public $validUdefinedVariableNames = null;

public function register() {
return [
T_VARIABLE,
Expand Down Expand Up @@ -130,12 +137,18 @@ protected function getOrCreateVariableInfo($varName, $currScope) {
$validUnusedVariableNames = (empty($this->validUnusedVariableNames))
? []
: preg_split('/\s+/', trim($this->validUnusedVariableNames));
$validUndefinedVariableNames = (empty($this->validUndefinedVariableNames))
? []
: preg_split('/\s+/', trim($this->validUndefinedVariableNames));
if (in_array($varName, $validUnusedVariableNames)) {
$scopeInfo->variables[$varName]->ignoreUnused = true;
}
if (isset($this->ignoreUnusedRegexp) && preg_match($this->ignoreUnusedRegexp, $varName) === 1) {
$scopeInfo->variables[$varName]->ignoreUnused = true;
}
if (in_array($varName, $validUndefinedVariableNames)) {
$scopeInfo->variables[$varName]->ignoreUndefined = true;
}
}
return $scopeInfo->variables[$varName];
}
Expand Down Expand Up @@ -206,6 +219,9 @@ protected function isVariableInitialized($varName, $stackPtr, $currScope) {

protected function isVariableUndefined($varName, $stackPtr, $currScope) {
$varInfo = $this->getVariableInfo($varName, $currScope);
if ($varInfo->ignoreUndefined) {
return false;
}
if (isset($varInfo->firstDeclared) && $varInfo->firstDeclared <= $stackPtr) {
// TODO: do we want to check scopeType here?
return false;
Expand Down
52 changes: 49 additions & 3 deletions VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ public function testFunctionWithGlobalVarWarnings() {
$expectedWarnings = [
4,
7,
22,
8,
23,
28,
29
];
$this->assertEquals($expectedWarnings, $lines);
}
Expand Down Expand Up @@ -355,8 +358,12 @@ public function testClassReferenceWarnings() {
$expectedWarnings = [
10,
11,
20,
21,
12,
13,
22,
23,
24,
25
];
$this->assertEquals($expectedWarnings, $lines);
}
Expand Down Expand Up @@ -553,4 +560,43 @@ public function testAllowDestructuringAssignment() {
];
$this->assertEquals($expectedWarnings, $lines);
}

public function testValidUndefinedVariableNamesIgnoresVarsInGlobalScope() {
$fixtureFile = $this->getFixture('FunctionWithGlobalVarFixture.php');
$phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
$phpcsFile->ruleset->setSniffProperty(
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
'validUndefinedVariableNames',
'ice_cream'
);
$phpcsFile->process();
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
$expectedWarnings = [
4,
7,
23,
];
$this->assertEquals($expectedWarnings, $lines);
}

public function testValidUndefinedVariableNamesIgnoresUndefinedProperties() {
$fixtureFile = $this->getFixture('ClassReferenceFixture.php');
$phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
$phpcsFile->ruleset->setSniffProperty(
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
'validUndefinedVariableNames',
'ignored_property'
);
$phpcsFile->process();
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
$expectedWarnings = [
10,
11,
22,
23,
24,
25
];
$this->assertEquals($expectedWarnings, $lines);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ function method_with_symbolic_ref_property() {
$this -> $property = 'some value';
$this->$undefined_property = 'some value';
$this -> $undefined_property = 'some value';
$this->$ignored_property = 'some value';
$this -> $ignored_property = 'some value';
}
}

Expand All @@ -19,6 +21,8 @@ function method_with_symbolic_ref_method() {
$this -> $method();
$this->$undefined_method();
$this -> $undefined_method();
$this->$ignored_method();
$this -> $ignored_method();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ function function_with_global_var() {

echo $var;
echo $var3;
echo $ice_cream;
return $var2;
}

Expand All @@ -21,3 +22,10 @@ function function_with_superglobals() {
echo "{$GLOBALS['whatever']}";
echo "{$GLOBALS['whatever']} $var";
}

// Variables within the global scope
$cherry = 'topping';
$sunday = $ice_cream . 'and a ' . $cherry;
if ( $ice_cream ) {
echo 'Two scoops please!';
}

0 comments on commit 1d89f26

Please sign in to comment.