Skip to content

Commit 77234a0

Browse files
SpacePossumSpacePossum
committed
BlankLineBeforeStatementFixer - handle comment case
1 parent e5de921 commit 77234a0

File tree

3 files changed

+167
-59
lines changed

3 files changed

+167
-59
lines changed

‎src/Fixer/Whitespace/BlankLineBeforeStatementFixer.php‎

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ function A() {
212212
'<?php
213213
if (null === $a) {
214214
$foo->bar();
215-
throw new \UnexpectedValueException("A cannot be null");
215+
throw new \UnexpectedValueException("A cannot be null.");
216216
}
217217
',
218218
[
@@ -271,40 +271,23 @@ public function isCandidate(Tokens $tokens)
271271
*/
272272
protected function applyFix(\SplFileInfo $file, Tokens $tokens)
273273
{
274-
$lineEnding = $this->whitespacesConfig->getLineEnding();
275274
$tokenKinds = array_values($this->fixTokenMap);
276275
$analyzer = new TokensAnalyzer($tokens);
277276

278-
for ($index = 0, $limit = $tokens->count(); $index < $limit; ++$index) {
277+
for ($index = $tokens->count() - 1; $index > 0; --$index) {
279278
$token = $tokens[$index];
280279

281280
if (!$token->isGivenKind($tokenKinds) || ($token->isGivenKind(T_WHILE) && $analyzer->isWhilePartOfDoWhile($index))) {
282281
continue;
283282
}
284283

285-
$prevNonWhitespaceToken = $tokens[$tokens->getPrevNonWhitespace($index)];
284+
$prevNonWhitespace = $tokens->getPrevNonWhitespace($index);
286285

287-
if (!$prevNonWhitespaceToken->equalsAny([';', '}'])) {
288-
continue;
286+
if ($this->shouldAddBlankLine($tokens, $prevNonWhitespace)) {
287+
$this->insertBlankLine($tokens, $index);
289288
}
290289

291-
$prevIndex = $index - 1;
292-
$prevToken = $tokens[$prevIndex];
293-
294-
if ($prevToken->isWhitespace()) {
295-
$countParts = substr_count($prevToken->getContent(), "\n");
296-
297-
if (0 === $countParts) {
298-
$tokens[$prevIndex] = new Token([T_WHITESPACE, rtrim($prevToken->getContent(), " \t").$lineEnding.$lineEnding]);
299-
} elseif (1 === $countParts) {
300-
$tokens[$prevIndex] = new Token([T_WHITESPACE, $lineEnding.$prevToken->getContent()]);
301-
}
302-
} else {
303-
$tokens->insertAt($index, new Token([T_WHITESPACE, $lineEnding.$lineEnding]));
304-
305-
++$index;
306-
++$limit;
307-
}
290+
$index = $prevNonWhitespace;
308291
}
309292
}
310293

@@ -328,4 +311,52 @@ protected function createConfigurationDefinition()
328311
->getOption(),
329312
]);
330313
}
314+
315+
/**
316+
* @param int $prevNonWhitespace
317+
*
318+
* @return bool
319+
*/
320+
private function shouldAddBlankLine(Tokens $tokens, $prevNonWhitespace)
321+
{
322+
$prevNonWhitespaceToken = $tokens[$prevNonWhitespace];
323+
324+
if ($prevNonWhitespaceToken->isComment()) {
325+
for ($j = $prevNonWhitespace - 1; $j >= 0; --$j) {
326+
if (false !== strpos($tokens[$j]->getContent(), "\n")) {
327+
return false;
328+
}
329+
330+
if ($tokens[$j]->isWhitespace() || $tokens[$j]->isComment()) {
331+
continue;
332+
}
333+
334+
return !$tokens[$j]->equals('{');
335+
}
336+
}
337+
338+
return $prevNonWhitespaceToken->equalsAny([';', '}']);
339+
}
340+
341+
/**
342+
* @param int $index
343+
*/
344+
private function insertBlankLine(Tokens $tokens, $index)
345+
{
346+
$prevIndex = $index - 1;
347+
$prevToken = $tokens[$prevIndex];
348+
$lineEnding = $this->whitespacesConfig->getLineEnding();
349+
350+
if ($prevToken->isWhitespace()) {
351+
$newlinesCount = substr_count($prevToken->getContent(), "\n");
352+
353+
if (0 === $newlinesCount) {
354+
$tokens[$prevIndex] = new Token([T_WHITESPACE, rtrim($prevToken->getContent(), " \t").$lineEnding.$lineEnding]);
355+
} elseif (1 === $newlinesCount) {
356+
$tokens[$prevIndex] = new Token([T_WHITESPACE, $lineEnding.$prevToken->getContent()]);
357+
}
358+
} else {
359+
$tokens->insertAt($index, new Token([T_WHITESPACE, $lineEnding.$lineEnding]));
360+
}
361+
}
331362
}

‎tests/AutoReview/ComposerTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public function testBranchAlias()
3232

3333
if (!isset($composerJson['extra']['branch-alias'])) {
3434
$this->addToAssertionCount(1); // composer.json doesn't contain branch alias, all good!
35+
3536
return;
3637
}
3738

‎tests/Fixer/Whitespace/BlankLineBeforeStatementFixerTest.php‎

Lines changed: 112 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,15 @@ public function provideFixWithBreakCases()
160160
echo $baz;
161161
break 1;
162162
}
163+
}',
164+
],
165+
[
166+
'<?php
167+
while (true) {
168+
if ($foo === $bar) {
169+
/** X */
170+
break 1;
171+
}
163172
}',
164173
],
165174
];
@@ -658,44 +667,17 @@ public function testFixWithIf($expected, $input = null)
658667
$this->doTest($expected, $input);
659668
}
660669

661-
/**
662-
* @dataProvider provideFixWithForEachCases
663-
*
664-
* @param string $expected
665-
* @param null|string $input
666-
*/
667-
public function testFixWithForEach($expected, $input = null)
668-
{
669-
$this->fixer->configure([
670-
'statements' => ['foreach'],
671-
]);
672-
673-
$this->doTest($expected, $input);
674-
}
675-
676-
public function provideFixWithForEachCases()
677-
{
678-
return [
679-
[
680-
'<?php
681-
echo 1;
682-
683-
foreach($a as $b){break;}
684-
',
685-
'<?php
686-
echo 1;
687-
foreach($a as $b){break;}
688-
',
689-
],
690-
];
691-
}
692-
693670
/**
694671
* @return array
695672
*/
696673
public function provideFixWithIfCases()
697674
{
698675
return [
676+
[
677+
'<?php if (true) {
678+
echo $bar;
679+
}',
680+
],
699681
[
700682
'<?php
701683
if (true) {
@@ -723,6 +705,38 @@ public function provideFixWithIfCases()
723705
];
724706
}
725707

708+
/**
709+
* @dataProvider provideFixWithForEachCases
710+
*
711+
* @param string $expected
712+
* @param null|string $input
713+
*/
714+
public function testFixWithForEach($expected, $input = null)
715+
{
716+
$this->fixer->configure([
717+
'statements' => ['foreach'],
718+
]);
719+
720+
$this->doTest($expected, $input);
721+
}
722+
723+
public function provideFixWithForEachCases()
724+
{
725+
return [
726+
[
727+
'<?php
728+
echo 1;
729+
730+
foreach($a as $b){break;}
731+
',
732+
'<?php
733+
echo 1;
734+
foreach($a as $b){break;}
735+
',
736+
],
737+
];
738+
}
739+
726740
/**
727741
* @dataProvider provideFixWithIncludeCases
728742
*
@@ -892,6 +906,20 @@ public function testFixWithReturn($expected, $input = null)
892906
public function provideFixWithReturnCases()
893907
{
894908
return [
909+
[
910+
'<?php
911+
if ($a) { /* 1 */ /* 2 */ /* 3 */ // something about $a
912+
return $b;
913+
}
914+
',
915+
],
916+
[
917+
'<?php
918+
if ($a) { // something about $a
919+
return $b;
920+
}
921+
',
922+
],
895923
[
896924
'
897925
$a = $a;
@@ -974,7 +1002,7 @@ public function provideFixWithReturnCases()
9741002
],
9751003
[
9761004
'<?php
977-
throw new Exception("return true;");',
1005+
throw new Exception("return true; //.");',
9781006
],
9791007
[
9801008
'<?php
@@ -1100,20 +1128,20 @@ public function provideFixWithThrowCases()
11001128
[
11011129
'<?php
11021130
if (false) {
1103-
throw new \Exception("Something unexpected happened");
1131+
throw new \Exception("Something unexpected happened.");
11041132
}',
11051133
],
11061134
[
11071135
'<?php
11081136
if (false) {
11091137
$log->error("No");
11101138
1111-
throw new \Exception("Something unexpected happened");
1139+
throw new \Exception("Something unexpected happened.");
11121140
}',
11131141
'<?php
11141142
if (false) {
11151143
$log->error("No");
1116-
throw new \Exception("Something unexpected happened");
1144+
throw new \Exception("Something unexpected happened.");
11171145
}',
11181146
],
11191147
];
@@ -1253,6 +1281,54 @@ public function provideFixWithYieldCases()
12531281
return [
12541282
[
12551283
'<?php
1284+
function foo() {
1285+
yield $a; /* a *//* b */ /* c */ /* d *//* e *//* etc */
1286+
'.'
1287+
yield $b;
1288+
}',
1289+
'<?php
1290+
function foo() {
1291+
yield $a; /* a *//* b */ /* c */ /* d *//* e *//* etc */ '.'
1292+
yield $b;
1293+
}',
1294+
],
1295+
[
1296+
'<?php
1297+
function foo() {
1298+
yield $a; // test
1299+
1300+
yield $b; // test /* A */
1301+
1302+
yield $c;
1303+
1304+
yield $d;
1305+
1306+
yield $e;#
1307+
1308+
yield $f;
1309+
/* @var int $g */
1310+
yield $g;
1311+
/* @var int $h */
1312+
yield $i;
1313+
1314+
yield $j;
1315+
}',
1316+
'<?php
1317+
function foo() {
1318+
yield $a; // test
1319+
yield $b; // test /* A */
1320+
yield $c;
1321+
yield $d;yield $e;#
1322+
yield $f;
1323+
/* @var int $g */
1324+
yield $g;
1325+
/* @var int $h */
1326+
yield $i;
1327+
yield $j;
1328+
}',
1329+
],
1330+
[
1331+
'<?php
12561332
function foo() {
12571333
yield $a;
12581334
}',

0 commit comments

Comments
 (0)