Skip to content

Commit 4f8e440

Browse files
jrfnlsirbrillig
authored andcommitted
Bug fix: false negative unused var (#120)
* Bug fix: false negative unused var As reported in 111, if a variable is assigned a value with a combined assignment operator, it would not be reported as unused. Includes unit tests. Fixes 111 * PHPStan config: don't report unmatched errors Now the `PHP_CodeSniffer\Util\Tokens` file is imported, the `Constant T_\w+ not found` errors should no longer be thrown. However, it depends on the order in which the files are analysed as that file is not (yet) `use`d in all files using the `T_` constants. If I remove the `ignoreErrors`, [the build fails](https://circleci.com/gh/sirbrillig/phpcs-variable-analysis/281) on those errors for the `VariableAnalysisSniff` file, while if I leave the exclusion in, [the build fails](https://circleci.com/gh/sirbrillig/phpcs-variable-analysis/283) on a "Ignored error not matched". So, for now, I'm leaving the ignore pattern in place and turning `reportUnmatchedIgnoredErrors` off to allow the build to pass.
1 parent 58002d1 commit 4f8e440

File tree

4 files changed

+38
-1
lines changed

4 files changed

+38
-1
lines changed

VariableAnalysis/Lib/Helpers.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace VariableAnalysis\Lib;
44

55
use PHP_CodeSniffer\Files\File;
6+
use PHP_CodeSniffer\Util\Tokens;
67

78
class Helpers {
89
/**
@@ -256,7 +257,11 @@ public static function getNextAssignPointer(File $phpcsFile, $stackPtr) {
256257

257258
// Is the next non-whitespace an assignment?
258259
$nextPtr = $phpcsFile->findNext(T_WHITESPACE, $stackPtr + 1, null, true, null, true);
259-
if (is_int($nextPtr) && $tokens[$nextPtr]['code'] === T_EQUAL) {
260+
if (is_int($nextPtr)
261+
&& isset(Tokens::$assignmentTokens[$tokens[$nextPtr]['code']])
262+
// Ignore double arrow to prevent triggering on `foreach ( $array as $k => $v )`.
263+
&& $tokens[$nextPtr]['code'] !== T_DOUBLE_ARROW
264+
) {
260265
return $nextPtr;
261266
}
262267
return null;

VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,4 +923,17 @@ public function testThisWithinNestedClosedScope() {
923923
];
924924
$this->assertEquals($expectedWarnings, $lines);
925925
}
926+
927+
public function testUnusedVarWithValueChange() {
928+
$fixtureFile = $this->getFixture('UnusedVarWithValueChangeFixture.php');
929+
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
930+
$phpcsFile->process();
931+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
932+
$expectedWarnings = [
933+
5,
934+
8,
935+
11,
936+
];
937+
$this->assertEquals($expectedWarnings, $lines);
938+
}
926939
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
// Issue #111.
4+
function foo() {
5+
$var = 'abc';
6+
$var = 'def';
7+
8+
$var2 = 'def';
9+
$var2 .= 'ghi';
10+
11+
$var3 = 10;
12+
$var3 += 20;
13+
}
14+
15+
// Safeguard that this change doesn't influence (not) reporting on assignments to parameters passed by reference.
16+
function bar(&$param) {
17+
$param .= 'foo';
18+
}

phpstan.neon.dist

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
parameters:
22
level: 7
3+
reportUnmatchedIgnoredErrors: false
34
autoload_files:
45
- %currentWorkingDirectory%/vendor/squizlabs/php_codesniffer/autoload.php
56
paths:

0 commit comments

Comments
 (0)