Skip to content

Commit edddabb

Browse files
SpacePossumSpacePossum
committed
NoEmptyStatementFixer - fix more cases
1 parent 1ff5457 commit edddabb

File tree

5 files changed

+119
-27
lines changed

5 files changed

+119
-27
lines changed

‎doc/rules/index.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ Semicolon
476476
- `multiline_whitespace_before_semicolons <./semicolon/multiline_whitespace_before_semicolons.rst>`_
477477
Forbid multi-line whitespace before the closing semicolon or move the semicolon to the new line for chained calls.
478478
- `no_empty_statement <./semicolon/no_empty_statement.rst>`_
479-
Remove useless semicolon statements.
479+
Remove useless (semicolon) statements.
480480
- `no_multiline_whitespace_before_semicolons <./semicolon/no_multiline_whitespace_before_semicolons.rst>`_ *(deprecated)*
481481
Multi-line whitespace before closing semicolon are prohibited.
482482
- `no_singleline_whitespace_before_semicolons <./semicolon/no_singleline_whitespace_before_semicolons.rst>`_

‎doc/rules/semicolon/no_empty_statement.rst‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Rule ``no_empty_statement``
33
===========================
44

5-
Remove useless semicolon statements.
5+
Remove useless (semicolon) statements.
66

77
Examples
88
--------
@@ -18,6 +18,30 @@ Example #1
1818
-<?php $a = 1;;
1919
+<?php $a = 1;
2020
21+
Example #2
22+
~~~~~~~~~~
23+
24+
.. code-block:: diff
25+
26+
--- Original
27+
+++ New
28+
@@ -1 +1 @@
29+
-<?php echo 1;2;
30+
+<?php echo 1;
31+
32+
Example #3
33+
~~~~~~~~~~
34+
35+
.. code-block:: diff
36+
37+
--- Original
38+
+++ New
39+
@@ -1,3 +1,3 @@
40+
<?php while(foo()){
41+
- continue 1;
42+
+ continue ;
43+
}
44+
2145
Rule sets
2246
---------
2347

‎src/Fixer/Semicolon/NoEmptyStatementFixer.php‎

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,12 @@ final class NoEmptyStatementFixer extends AbstractFixer
3030
public function getDefinition()
3131
{
3232
return new FixerDefinition(
33-
'Remove useless semicolon statements.',
34-
[new CodeSample("<?php \$a = 1;;\n")]
33+
'Remove useless (semicolon) statements.',
34+
[
35+
new CodeSample("<?php \$a = 1;;\n"),
36+
new CodeSample("<?php echo 1;2;\n"),
37+
new CodeSample("<?php while(foo()){\n continue 1;\n}\n"),
38+
]
3539
);
3640
}
3741

@@ -59,7 +63,17 @@ public function isCandidate(Tokens $tokens)
5963
protected function applyFix(\SplFileInfo $file, Tokens $tokens)
6064
{
6165
for ($index = 0, $count = $tokens->count(); $index < $count; ++$index) {
62-
// skip T_FOR parenthesis to ignore duplicated `;` like `for ($i = 1; ; ++$i) {...}`
66+
if ($tokens[$index]->isGivenKind([T_BREAK, T_CONTINUE])) {
67+
$index = $tokens->getNextMeaningfulToken($index);
68+
69+
if ($tokens[$index]->equals([T_LNUMBER, '1'])) {
70+
$tokens->clearTokenAndMergeSurroundingWhitespace($index);
71+
}
72+
73+
continue;
74+
}
75+
76+
// skip T_FOR parenthesis to ignore double `;` like `for ($i = 1; ; ++$i) {...}`
6377
if ($tokens[$index]->isGivenKind(T_FOR)) {
6478
$index = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $tokens->getNextMeaningfulToken($index)) + 1;
6579

@@ -82,6 +96,19 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
8296
// A semicolon might be removed if it follows a '}' but only if the brace is part of certain structures.
8397
if ($tokens[$previousMeaningfulIndex]->equals('}')) {
8498
$this->fixSemicolonAfterCurlyBraceClose($tokens, $index, $previousMeaningfulIndex);
99+
100+
continue;
101+
}
102+
103+
// A semicolon might be removed together with its noop statement, for example "<?php 1;"
104+
$prePreviousMeaningfulIndex = $tokens->getPrevMeaningfulToken($previousMeaningfulIndex);
105+
106+
if (
107+
$tokens[$prePreviousMeaningfulIndex]->equalsAny([';', '{', '}', [T_OPEN_TAG]])
108+
&& $tokens[$previousMeaningfulIndex]->isGivenKind([T_CONSTANT_ENCAPSED_STRING, T_DNUMBER, T_LNUMBER, T_STRING, T_VARIABLE])
109+
) {
110+
$tokens->clearTokenAndMergeSurroundingWhitespace($index);
111+
$tokens->clearTokenAndMergeSurroundingWhitespace($previousMeaningfulIndex);
85112
}
86113
}
87114
}
@@ -112,27 +139,27 @@ private function fixSemicolonAfterCurlyBraceClose(Tokens $tokens, $index, $curly
112139
}
113140

114141
$curlyOpeningIndex = $tokens->findBlockStart(Tokens::BLOCK_TYPE_CURLY_BRACE, $curlyCloseIndex);
115-
$beforeCurlyOpening = $tokens->getPrevMeaningfulToken($curlyOpeningIndex);
142+
$beforeCurlyOpeningIndex = $tokens->getPrevMeaningfulToken($curlyOpeningIndex);
116143

117-
if ($tokens[$beforeCurlyOpening]->isGivenKind($beforeCurlyOpeningKinds) || $tokens[$beforeCurlyOpening]->equalsAny([';', '{', '}'])) {
144+
if ($tokens[$beforeCurlyOpeningIndex]->isGivenKind($beforeCurlyOpeningKinds) || $tokens[$beforeCurlyOpeningIndex]->equalsAny([';', '{', '}'])) {
118145
$tokens->clearTokenAndMergeSurroundingWhitespace($index);
119146

120147
return;
121148
}
122149

123150
// check for namespaces and class, interface and trait definitions
124-
if ($tokens[$beforeCurlyOpening]->isGivenKind(T_STRING)) {
125-
$classyTest = $tokens->getPrevMeaningfulToken($beforeCurlyOpening);
151+
if ($tokens[$beforeCurlyOpeningIndex]->isGivenKind(T_STRING)) {
152+
$classyTestIndex = $tokens->getPrevMeaningfulToken($beforeCurlyOpeningIndex);
126153

127-
while ($tokens[$classyTest]->equals(',') || $tokens[$classyTest]->isGivenKind([T_STRING, T_NS_SEPARATOR, T_EXTENDS, T_IMPLEMENTS])) {
128-
$classyTest = $tokens->getPrevMeaningfulToken($classyTest);
154+
while ($tokens[$classyTestIndex]->equals(',') || $tokens[$classyTestIndex]->isGivenKind([T_STRING, T_NS_SEPARATOR, T_EXTENDS, T_IMPLEMENTS])) {
155+
$classyTestIndex = $tokens->getPrevMeaningfulToken($classyTestIndex);
129156
}
130157

131158
$tokensAnalyzer = new TokensAnalyzer($tokens);
132159

133160
if (
134-
$tokens[$classyTest]->isGivenKind(T_NAMESPACE) ||
135-
($tokens[$classyTest]->isClassy() && !$tokensAnalyzer->isAnonymousClass($classyTest))
161+
$tokens[$classyTestIndex]->isGivenKind(T_NAMESPACE)
162+
|| ($tokens[$classyTestIndex]->isClassy() && !$tokensAnalyzer->isAnonymousClass($classyTestIndex))
136163
) {
137164
$tokens->clearTokenAndMergeSurroundingWhitespace($index);
138165
}
@@ -141,24 +168,24 @@ private function fixSemicolonAfterCurlyBraceClose(Tokens $tokens, $index, $curly
141168
}
142169

143170
// early return check, below only control structures with conditions are fixed
144-
if (!$tokens[$beforeCurlyOpening]->equals(')')) {
171+
if (!$tokens[$beforeCurlyOpeningIndex]->equals(')')) {
145172
return;
146173
}
147174

148-
$openingBrace = $tokens->findBlockStart(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $beforeCurlyOpening);
149-
$beforeOpeningBrace = $tokens->getPrevMeaningfulToken($openingBrace);
175+
$openingBraceIndex = $tokens->findBlockStart(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $beforeCurlyOpeningIndex);
176+
$beforeOpeningBraceIndex = $tokens->getPrevMeaningfulToken($openingBraceIndex);
150177

151-
if ($tokens[$beforeOpeningBrace]->isGivenKind([T_IF, T_ELSEIF, T_FOR, T_FOREACH, T_WHILE, T_SWITCH, T_CATCH, T_DECLARE])) {
178+
if ($tokens[$beforeOpeningBraceIndex]->isGivenKind([T_IF, T_ELSEIF, T_FOR, T_FOREACH, T_WHILE, T_SWITCH, T_CATCH, T_DECLARE])) {
152179
$tokens->clearTokenAndMergeSurroundingWhitespace($index);
153180

154181
return;
155182
}
156183

157184
// check for function definition
158-
if ($tokens[$beforeOpeningBrace]->isGivenKind(T_STRING)) {
159-
$beforeString = $tokens->getPrevMeaningfulToken($beforeOpeningBrace);
185+
if ($tokens[$beforeOpeningBraceIndex]->isGivenKind(T_STRING)) {
186+
$beforeStringIndex = $tokens->getPrevMeaningfulToken($beforeOpeningBraceIndex);
160187

161-
if ($tokens[$beforeString]->isGivenKind(T_FUNCTION)) {
188+
if ($tokens[$beforeStringIndex]->isGivenKind(T_FUNCTION)) {
162189
$tokens->clearTokenAndMergeSurroundingWhitespace($index); // implicit return
163190
}
164191
}

‎tests/Fixer/Semicolon/NoEmptyStatementFixerTest.php‎

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function testNoEmptyStatements($expected, $input = null)
3737

3838
public function provideNoEmptyStatementsCases()
3939
{
40-
return [
40+
$tests = [
4141
[
4242
'<?php
4343
abstract class TestClass0 extends Test IMPLEMENTS TestInterface, TestInterface2
@@ -255,13 +255,13 @@ function foo()
255255
while($a > 1){
256256
}
257257
do {
258-
} while($a>1);
258+
} while($a>1); // 1
259259
',
260260
'<?php
261261
while($a > 1){
262262
};
263263
do {
264-
} while($a>1);
264+
} while($a>1); 1; // 1
265265
',
266266
],
267267
[
@@ -373,22 +373,62 @@ trait TestTrait
373373
[
374374
'<?php
375375
try {
376-
throw new \Exception("a");
376+
throw new \Exception("Foo.");
377377
} catch (\Exception $e){
378378
//
379379
} finally {
380380
} '.'
381381
',
382382
'<?php
383383
try {
384-
throw new \Exception("a");
384+
throw new \Exception("Foo.");
385385
} catch (\Exception $e){
386386
//
387387
} finally {
388388
} ;
389389
',
390390
],
391391
];
392+
393+
foreach ($tests as $index => $test) {
394+
yield $index => $test;
395+
}
396+
397+
foreach (['break', 'continue'] as $ops) {
398+
yield [
399+
sprintf('<?php while(true) {%s ;}', $ops),
400+
sprintf('<?php while(true) {%s 1;}', $ops),
401+
];
402+
}
403+
404+
foreach (['1', '1.0', '"foo"', '$foo'] as $noop) {
405+
yield [
406+
'<?php echo "foo"; ',
407+
sprintf('<?php echo "foo"; %s ;', $noop),
408+
];
409+
}
410+
411+
yield [
412+
'<?php /* 1 */ /* 2 */ /* 3 */ ',
413+
'<?php /* 1 */ ; /* 2 */ 1 /* 3 */ ;',
414+
];
415+
416+
yield [
417+
'<?php
418+
while(true) {while(true) {break 2;}}
419+
while(true) {continue;}
420+
',
421+
];
422+
423+
yield [
424+
'<?php if ($foo1) {} ',
425+
'<?php if ($foo1) {} 1;',
426+
];
427+
428+
yield [
429+
'<?php if ($foo2) {}',
430+
'<?php if ($foo2) {1;}',
431+
];
392432
}
393433

394434
/**
@@ -497,6 +537,7 @@ public function testCasesWithShortOpenTag($expected, $input = null)
497537
if (!ini_get('short_open_tag')) {
498538
static::markTestSkipped('No short tag tests possible.');
499539
}
540+
500541
$this->doTest($expected, $input);
501542
}
502543

‎tests/Fixtures/Integration/priority/no_empty_statement,no_trailing_whitespace.test‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ Integration of fixers: no_empty_statement,no_trailing_whitespace.
44
{"no_empty_statement": true, "no_trailing_whitespace": true}
55
--EXPECT--
66
<?php $foo = 2;
7-
$a;
7+
++$a;
88

99
--INPUT--
1010
<?php $foo = 2;
11-
$a; ;
11+
++$a; ;

0 commit comments

Comments
 (0)