Skip to content

Commit c1fb8f6

Browse files
sirbrilligsirbrillig
authored
Allow unused function arguments as regexp (#22)
* Tests: Add tests for unused parameters * Tests: update tests to allow unused variables * Improve phpdoc about validUnusedVariableNames * Convert validUnusedVariableNames to array late Previously the property was converted to an array when the Sniff was registered. This could cause issues if the property was changed (I know it's unlikely but it can happen in tests pretty easily) after registration. Furthermore, it was convered to an array and then used to replace the original string property, which introduces risk by both mutating a property during an object lifecycle and even changing its type. With this change, we never change the property from a string, and instead we convert it to an array just before it is used. * Tests: set validUnusedVariableNames for testUnusedParams * Tests: add tests for ignoreUnusedRegexp * Add ignoreUnusedRegexp * Tests: add tests for allowUnusedCaughtExceptions * Tests: add tests for multi-line unused function params * Tests: add test for allowUnusedFunctionParameters * README: add docs for specific options
1 parent 57e4231 commit c1fb8f6

File tree

4 files changed

+214
-9
lines changed

4 files changed

+214
-9
lines changed

‎README.md

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,24 @@ If you already have installed paths, [use a comma to separate them](https://gith
5858

5959
## Customization
6060

61-
There's a variety of options to customize the behaviour of VariableAnalysis, take
62-
a look at the included ruleset.xml.example for commented examples of a configuration.
61+
There's a variety of options to customize the behaviour of VariableAnalysis, take a look at the included ruleset.xml.example for commented examples of a configuration.
62+
63+
The available options are as follows:
64+
65+
- `allowUnusedFunctionParameters` (bool, default `false`): if set to true, function arguments will never be marked as unused.
66+
- `allowUnusedCaughtExceptions` (bool, default `false`): if set to true, caught Exception variables will never be marked as unused.
67+
- `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'`.
68+
- `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 `'/^_/'`.
69+
70+
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:
71+
72+
```xml
73+
<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis">
74+
<properties>
75+
<property name="ignoreUnusedRegexp" value="/^_/"/>
76+
</properties>
77+
</rule>
78+
```
6379

6480
## Original
6581

‎VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,20 @@ class VariableAnalysisSniff implements Sniff {
4141
public $allowUnusedFunctionParameters = false;
4242

4343
/**
44-
* A list of names of placeholder variables that you want to ignore from
45-
* unused variable warnings, ie things like $junk.
44+
* A space-separated list of names of placeholder variables that you want to
45+
* ignore from unused variable warnings. For example, to ignore the variables
46+
* `$junk` and `$unused`, this could be set to `'junk unused'`.
4647
*/
4748
public $validUnusedVariableNames = null;
4849

50+
/**
51+
* A PHP regexp string for variables that you want to ignore from unused
52+
* variable warnings. For example, to ignore the variables `$_junk` and
53+
* `$_unused`, this could be set to `'/^_/'`.
54+
*/
55+
public $ignoreUnusedRegexp = null;
56+
4957
public function register() {
50-
if (!empty($this->validUnusedVariableNames)) {
51-
$this->validUnusedVariableNames =
52-
preg_split('/\s+/', trim($this->validUnusedVariableNames));
53-
}
5458
return [
5559
T_VARIABLE,
5660
T_DOUBLE_QUOTED_STRING,
@@ -123,7 +127,13 @@ protected function getOrCreateVariableInfo($varName, $currScope) {
123127
$scopeInfo = $this->getOrCreateScopeInfo($currScope);
124128
if (!isset($scopeInfo->variables[$varName])) {
125129
$scopeInfo->variables[$varName] = new VariableInfo($varName);
126-
if ($this->validUnusedVariableNames && in_array($varName, $this->validUnusedVariableNames)) {
130+
$validUnusedVariableNames = (empty($this->validUnusedVariableNames))
131+
? []
132+
: preg_split('/\s+/', trim($this->validUnusedVariableNames));
133+
if (in_array($varName, $validUnusedVariableNames)) {
134+
$scopeInfo->variables[$varName]->ignoreUnused = true;
135+
}
136+
if (isset($this->ignoreUnusedRegexp) && preg_match($this->ignoreUnusedRegexp, $varName) === 1) {
127137
$scopeInfo->variables[$varName]->ignoreUnused = true;
128138
}
129139
}

‎VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,4 +413,99 @@ public function testTraitsAllowPropertyDefinitions() {
413413
$expectedErrors = [];
414414
$this->assertEquals($expectedErrors, $lines);
415415
}
416+
417+
public function testUnusedParamsAreReported() {
418+
$fixtureFile = $this->getFixture('FunctionWithUnusedParamsFixture.php');
419+
$phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
420+
$phpcsFile->process();
421+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
422+
$expectedWarnings = [
423+
4,
424+
16,
425+
27,
426+
39,
427+
66,
428+
72,
429+
73,
430+
];
431+
$this->assertEquals($expectedWarnings, $lines);
432+
}
433+
434+
public function testValidUnusedVariableNamesIgnoresUnusedVariables() {
435+
$fixtureFile = $this->getFixture('FunctionWithUnusedParamsFixture.php');
436+
$phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
437+
$phpcsFile->ruleset->setSniffProperty(
438+
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
439+
'validUnusedVariableNames',
440+
'ignored'
441+
);
442+
$phpcsFile->process();
443+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
444+
$expectedWarnings = [
445+
4,
446+
16,
447+
39,
448+
66,
449+
72,
450+
73,
451+
];
452+
$this->assertEquals($expectedWarnings, $lines);
453+
}
454+
455+
public function testAllowUnusedFunctionParametersIgnoresUnusedVariables() {
456+
$fixtureFile = $this->getFixture('FunctionWithUnusedParamsFixture.php');
457+
$phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
458+
$phpcsFile->ruleset->setSniffProperty(
459+
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
460+
'allowUnusedFunctionParameters',
461+
'true'
462+
);
463+
$phpcsFile->process();
464+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
465+
$expectedWarnings = [
466+
66,
467+
];
468+
$this->assertEquals($expectedWarnings, $lines);
469+
}
470+
471+
public function testAllowUnusedCaughtExceptionsIgnoresUnusedVariables() {
472+
$fixtureFile = $this->getFixture('FunctionWithUnusedParamsFixture.php');
473+
$phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
474+
$phpcsFile->ruleset->setSniffProperty(
475+
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
476+
'allowUnusedCaughtExceptions',
477+
'true'
478+
);
479+
$phpcsFile->process();
480+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
481+
$expectedWarnings = [
482+
4,
483+
16,
484+
27,
485+
39,
486+
72,
487+
73,
488+
];
489+
$this->assertEquals($expectedWarnings, $lines);
490+
}
491+
492+
public function testIgnoreUnusedRegexpIgnoresUnusedVariables() {
493+
$fixtureFile = $this->getFixture('FunctionWithUnusedParamsFixture.php');
494+
$phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
495+
$phpcsFile->ruleset->setSniffProperty(
496+
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
497+
'ignoreUnusedRegexp',
498+
'/^unused_/'
499+
);
500+
$phpcsFile->process();
501+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
502+
$expectedWarnings = [
503+
4,
504+
16,
505+
27,
506+
39,
507+
72,
508+
];
509+
$this->assertEquals($expectedWarnings, $lines);
510+
}
416511
}

‎VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithUnusedParamsFixture.php

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
// The following line should report an unused variable
4+
function function_with_first_unused_param($unused, $param) {
5+
echo $param;
6+
echo "xxx $param xxx";
7+
echo "xxx {$param} xxx";
8+
$param = 'set the param';
9+
echo $param;
10+
echo "xxx $param xxx";
11+
echo "xxx {$param} xxx";
12+
return $param;
13+
}
14+
15+
// The following line should report an unused variable
16+
function function_with_second_unused_param($param, $unused) {
17+
echo $param;
18+
echo "xxx $param xxx";
19+
echo "xxx {$param} xxx";
20+
$param = 'set the param';
21+
echo $param;
22+
echo "xxx $param xxx";
23+
echo "xxx {$param} xxx";
24+
return $param;
25+
}
26+
27+
function function_with_second_unused_param_ignored($param, $ignored) {
28+
echo $param;
29+
echo "xxx $param xxx";
30+
echo "xxx {$param} xxx";
31+
$param = 'set the param';
32+
echo $param;
33+
echo "xxx $param xxx";
34+
echo "xxx {$param} xxx";
35+
return $param;
36+
}
37+
38+
// The following line should report an unused variable
39+
function function_with_all_unused_params($unused, $unused_two) {
40+
$param = 'hello';
41+
echo $param;
42+
echo "xxx $param xxx";
43+
echo "xxx {$param} xxx";
44+
$param = 'set the param';
45+
echo $param;
46+
echo "xxx $param xxx";
47+
echo "xxx {$param} xxx";
48+
return $param;
49+
}
50+
51+
function function_with_no_unused_params($param, $param_two) {
52+
echo $param;
53+
echo $param_two;
54+
echo "xxx $param xxx";
55+
echo "xxx {$param} xxx";
56+
$param = 'set the param';
57+
echo $param;
58+
echo "xxx $param xxx";
59+
echo "xxx {$param} xxx";
60+
return $param;
61+
}
62+
63+
function function_with_try_catch_and_unused_exception() {
64+
try {
65+
doAThing();
66+
} catch (Exception $unused_param) {
67+
echo "unused";
68+
}
69+
}
70+
71+
function function_with_multi_line_unused_params(
72+
$unused,
73+
$unused_two
74+
) {
75+
$param = 'hello';
76+
echo $param;
77+
echo "xxx $param xxx";
78+
echo "xxx {$param} xxx";
79+
$param = 'set the param';
80+
echo $param;
81+
echo "xxx $param xxx";
82+
echo "xxx {$param} xxx";
83+
return $param;
84+
}

0 commit comments

Comments
 (0)