Skip to content

Commit 113c268

Browse files
kukulichkukulich
committed
UselessVariableSniff: Make the sniff partly fixable
1 parent f60c48b commit 113c268

File tree

5 files changed

+176
-7
lines changed

5 files changed

+176
-7
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ Looks for useless semicolons.
173173

174174
Looks for unused variables.
175175

176-
#### SlevomatCodingStandard.Variables.UselessVariable
176+
#### SlevomatCodingStandard.Variables.UselessVariable 🔧
177177

178178
Looks for useless variables.
179179

‎SlevomatCodingStandard/Sniffs/Variables/UselessVariableSniff.php‎

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PHP_CodeSniffer\Sniffs\Sniff;
77
use SlevomatCodingStandard\Helpers\TokenHelper;
88
use const T_AND_EQUAL;
9+
use const T_CLOSE_CURLY_BRACKET;
910
use const T_CONCAT_EQUAL;
1011
use const T_DIV_EQUAL;
1112
use const T_EQUAL;
@@ -21,6 +22,7 @@
2122
use const T_SR_EQUAL;
2223
use const T_VARIABLE;
2324
use const T_XOR_EQUAL;
25+
use function array_key_exists;
2426
use function array_reverse;
2527
use function count;
2628
use function in_array;
@@ -56,8 +58,8 @@ public function process(File $phpcsFile, $returnPointer): void
5658
return;
5759
}
5860

59-
$pointerAfterVariable = TokenHelper::findNextEffective($phpcsFile, $variablePointer + 1);
60-
if ($tokens[$pointerAfterVariable]['code'] !== T_SEMICOLON) {
61+
$returnSemicolonPointer = TokenHelper::findNextEffective($phpcsFile, $variablePointer + 1);
62+
if ($tokens[$returnSemicolonPointer]['code'] !== T_SEMICOLON) {
6163
return;
6264
}
6365

@@ -92,8 +94,9 @@ public function process(File $phpcsFile, $returnPointer): void
9294
return;
9395
}
9496

95-
$pointerAfterPreviousVariable = TokenHelper::findNextEffective($phpcsFile, $previousVariablePointer + 1);
96-
if (!in_array($tokens[$pointerAfterPreviousVariable]['code'], [
97+
/** @var int $assigmentPointer */
98+
$assigmentPointer = TokenHelper::findNextEffective($phpcsFile, $previousVariablePointer + 1);
99+
if (!in_array($tokens[$assigmentPointer]['code'], [
97100
T_EQUAL,
98101
T_PLUS_EQUAL,
99102
T_MINUS_EQUAL,
@@ -133,7 +136,81 @@ public function process(File $phpcsFile, $returnPointer): void
133136
}
134137
}
135138

136-
$phpcsFile->addError(sprintf('Useless variable %s.', $variableName), $previousVariablePointer, self::CODE_USELESS_VARIABLE);
139+
$errorParameters = [
140+
sprintf('Useless variable %s.', $variableName),
141+
$previousVariablePointer,
142+
self::CODE_USELESS_VARIABLE,
143+
];
144+
145+
$pointerAfterReturnSemicolon = TokenHelper::findNextEffective($phpcsFile, $returnSemicolonPointer + 1);
146+
147+
if (
148+
$tokens[$pointerAfterReturnSemicolon]['code'] !== T_CLOSE_CURLY_BRACKET
149+
|| !array_key_exists('scope_condition', $tokens[$pointerAfterReturnSemicolon])
150+
|| !in_array($tokens[$tokens[$pointerAfterReturnSemicolon]['scope_condition']]['code'], TokenHelper::$functionTokenCodes, true)
151+
) {
152+
$phpcsFile->addError(...$errorParameters);
153+
return;
154+
}
155+
156+
$previousVariableSemicolonPointer = null;
157+
for ($i = $previousVariablePointer + 1; $i < $returnPointer - 1; $i++) {
158+
if ($tokens[$i]['code'] !== T_SEMICOLON) {
159+
continue;
160+
}
161+
162+
if (!$this->isInSameScope($phpcsFile, $previousVariablePointer, $i)) {
163+
continue;
164+
}
165+
166+
$previousVariableSemicolonPointer = $i;
167+
break;
168+
}
169+
$pointerAfterPreviousVariableSemicolon = TokenHelper::findNextEffective($phpcsFile, $previousVariableSemicolonPointer + 1);
170+
171+
if ($returnPointer !== $pointerAfterPreviousVariableSemicolon) {
172+
$phpcsFile->addError(...$errorParameters);
173+
return;
174+
}
175+
176+
$fix = $phpcsFile->addFixableError(...$errorParameters);
177+
178+
if (!$fix) {
179+
return;
180+
}
181+
182+
$assigmentFixerMapping = [
183+
T_PLUS_EQUAL => '+',
184+
T_MINUS_EQUAL => '-',
185+
T_MUL_EQUAL => '*',
186+
T_DIV_EQUAL => '/',
187+
T_POW_EQUAL => '**',
188+
T_MOD_EQUAL => '%',
189+
T_AND_EQUAL => '&',
190+
T_OR_EQUAL => '|',
191+
T_XOR_EQUAL => '^',
192+
T_SL_EQUAL => '<<',
193+
T_SR_EQUAL => '>>',
194+
T_CONCAT_EQUAL => '.',
195+
];
196+
197+
$phpcsFile->fixer->beginChangeset();
198+
199+
if ($tokens[$assigmentPointer]['code'] === T_EQUAL) {
200+
for ($i = $previousVariablePointer; $i < $assigmentPointer; $i++) {
201+
$phpcsFile->fixer->replaceToken($i, '');
202+
}
203+
$phpcsFile->fixer->replaceToken($assigmentPointer, 'return');
204+
} else {
205+
$phpcsFile->fixer->addContentBefore($previousVariablePointer, 'return ');
206+
$phpcsFile->fixer->replaceToken($assigmentPointer, $assigmentFixerMapping[$tokens[$assigmentPointer]['code']]);
207+
}
208+
209+
for ($i = $previousVariableSemicolonPointer + 1; $i <= $returnSemicolonPointer; $i++) {
210+
$phpcsFile->fixer->replaceToken($i, '');
211+
}
212+
213+
$phpcsFile->fixer->endChangeset();
137214
}
138215

139216
private function isInSameScope(File $phpcsFile, int $firstPointer, int $secondPointer): bool

‎tests/Sniffs/Variables/UselessVariableSniffTest.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public function testErrors(): void
1717
{
1818
$report = self::checkFile(__DIR__ . '/data/uselessVariableErrors.php');
1919

20-
self::assertSame(15, $report->getErrorCount());
20+
self::assertSame(16, $report->getErrorCount());
2121

2222
self::assertSniffError($report, 4, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $a.');
2323
self::assertSniffError($report, 9, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $b.');
@@ -34,6 +34,9 @@ public function testErrors(): void
3434
self::assertSniffError($report, 64, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $m.');
3535
self::assertSniffError($report, 69, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $n.');
3636
self::assertSniffError($report, 77, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $o.');
37+
self::assertSniffError($report, 90, UselessVariableSniff::CODE_USELESS_VARIABLE, 'Useless variable $result.');
38+
39+
self::assertAllFixedInFile($report);
3740
}
3841

3942
}

‎tests/Sniffs/Variables/data/uselessVariableErrors.fixed.php‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
<?php
2+
3+
function () {
4+
return true;
5+
};
6+
7+
function () {
8+
return $b + 1;
9+
};
10+
11+
function () {
12+
return $c - 1;
13+
};
14+
15+
function () {
16+
return $d * 1;
17+
};
18+
19+
function () {
20+
return $e / 1;
21+
};
22+
23+
function () {
24+
return $f ** 1;
25+
};
26+
27+
function () {
28+
return $g % 1;
29+
};
30+
31+
function () {
32+
return $h & 1;
33+
};
34+
35+
function () {
36+
return $i | 1;
37+
};
38+
39+
function () {
40+
return $j ^ 1;
41+
};
42+
43+
function () {
44+
return $k << 1;
45+
};
46+
47+
function () {
48+
return $l >> 1;
49+
};
50+
51+
function () {
52+
return $m . 1;
53+
};
54+
55+
function sameVariableInDifferentScope() {
56+
return array_map(function () {
57+
return $n + 1;
58+
}, []);
59+
}
60+
61+
function differentVariableAfterReturn() {
62+
$o = 0;
63+
64+
if (true) {
65+
return $o;
66+
}
67+
68+
$p = 1;
69+
}
70+
71+
function moreReturns() {
72+
try {
73+
$result = true;
74+
} catch (Throwable $e) {
75+
$result = false;
76+
}
77+
78+
return $result;
79+
}

‎tests/Sniffs/Variables/data/uselessVariableErrors.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,13 @@ function differentVariableAfterReturn() {
8282

8383
$p = 1;
8484
}
85+
86+
function moreReturns() {
87+
try {
88+
$result = true;
89+
} catch (Throwable $e) {
90+
$result = false;
91+
}
92+
93+
return $result;
94+
}

0 commit comments

Comments
 (0)