Skip to content

Commit fb11145

Browse files
kukulichkukulich
committed
ControlStructureSpacingSniff: Fixed reporting and fixer
1 parent 8559df0 commit fb11145

File tree

6 files changed

+161
-14
lines changed

6 files changed

+161
-14
lines changed

‎SlevomatCodingStandard/Sniffs/ControlStructures/ControlStructureSpacingSniff.php‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,14 @@ private function checkLinesBefore(File $phpcsFile, int $controlStructurePointer)
150150
in_array($tokens[$pointerBefore]['code'], Tokens::$commentTokens, true)
151151
&& $tokens[$pointerBefore]['line'] + 1 === $tokens[$controlStructurePointer]['line']
152152
) {
153-
$controlStructureStartPointer = array_key_exists('comment_opener', $tokens[$pointerBefore])
154-
? $tokens[$pointerBefore]['comment_opener']
155-
: TokenHelper::findPreviousExcluding($phpcsFile, T_COMMENT, $pointerBefore - 1) + 1;
156-
157-
/** @var int $pointerBefore */
158-
$pointerBefore = TokenHelper::findPreviousEffective($phpcsFile, $pointerBefore - 1);
153+
$pointerBeforeComment = TokenHelper::findPreviousEffective($phpcsFile, $pointerBefore - 1);
154+
if ($tokens[$pointerBeforeComment]['line'] !== $tokens[$pointerBefore]['line']) {
155+
$controlStructureStartPointer = array_key_exists('comment_opener', $tokens[$pointerBefore])
156+
? $tokens[$pointerBefore]['comment_opener']
157+
: TokenHelper::findPreviousExcluding($phpcsFile, T_COMMENT, $pointerBefore - 1) + 1;
158+
/** @var int $pointerBefore */
159+
$pointerBefore = TokenHelper::findPreviousEffective($phpcsFile, $pointerBefore - 1);
160+
}
159161
}
160162

161163
$isFirstControlStructure = in_array($tokens[$pointerBefore]['code'], [T_OPEN_CURLY_BRACKET, T_COLON], true);
@@ -166,6 +168,11 @@ private function checkLinesBefore(File $phpcsFile, int $controlStructurePointer)
166168
$whitespaceBefore .= substr($tokens[$pointerBefore]['content'], strlen('<?php'));
167169
}
168170

171+
$hasCommentWithLineEndBefore = $tokens[$pointerBefore]['code'] === T_COMMENT && substr($tokens[$pointerBefore]['content'], -strlen($phpcsFile->eolChar)) === $phpcsFile->eolChar;
172+
if ($hasCommentWithLineEndBefore) {
173+
$whitespaceBefore .= $phpcsFile->eolChar;
174+
}
175+
169176
if ($pointerBefore + 1 !== $controlStructurePointer) {
170177
$whitespaceBefore .= TokenHelper::getContent($phpcsFile, $pointerBefore + 1, $controlStructureStartPointer - 1);
171178
}
@@ -198,9 +205,12 @@ private function checkLinesBefore(File $phpcsFile, int $controlStructurePointer)
198205
for ($i = $pointerBefore + 1; $i <= $endOfLineBeforePointer; $i++) {
199206
$phpcsFile->fixer->replaceToken($i, '');
200207
}
201-
for ($i = 0; $i <= $requiredLinesCountBefore; $i++) {
208+
209+
$linesToAdd = $hasCommentWithLineEndBefore ? $requiredLinesCountBefore - 1 : $requiredLinesCountBefore;
210+
for ($i = 0; $i <= $linesToAdd; $i++) {
202211
$phpcsFile->fixer->addNewline($pointerBefore);
203212
}
213+
204214
$phpcsFile->fixer->endChangeset();
205215
}
206216

@@ -209,12 +219,15 @@ private function checkLinesAfter(File $phpcsFile, int $controlStructurePointer):
209219
$tokens = $phpcsFile->getTokens();
210220

211221
$controlStructureEndPointer = $this->findControlStructureEnd($phpcsFile, $controlStructurePointer);
212-
$pointerAfter = TokenHelper::findNextExcluding($phpcsFile, T_WHITESPACE, $controlStructureEndPointer + 1);
222+
$notWhitespacePointerAfter = TokenHelper::findNextExcluding($phpcsFile, T_WHITESPACE, $controlStructureEndPointer + 1);
213223

214-
if ($pointerAfter === null) {
224+
if ($notWhitespacePointerAfter === null) {
215225
return;
216226
}
217227

228+
$hasCommentAfter = in_array($tokens[$notWhitespacePointerAfter]['code'], Tokens::$commentTokens, true) && $tokens[$notWhitespacePointerAfter]['line'] === $tokens[$controlStructureEndPointer]['line'];
229+
$pointerAfter = $hasCommentAfter ? TokenHelper::findNextEffective($phpcsFile, $controlStructureEndPointer + 1) : $notWhitespacePointerAfter;
230+
218231
$isLastControlStructure = in_array($tokens[$pointerAfter]['code'], [T_CLOSE_CURLY_BRACKET, T_CASE, T_DEFAULT], true);
219232

220233
$requiredLinesCountAfter = SniffSettingsHelper::normalizeInteger($isLastControlStructure ? $this->linesCountAfterLastControlStructure : $this->linesCountAroundControlStructure);
@@ -236,13 +249,21 @@ private function checkLinesAfter(File $phpcsFile, int $controlStructurePointer):
236249

237250
$phpcsFile->fixer->beginChangeset();
238251

252+
$replaceStartPointer = $hasCommentAfter ? $notWhitespacePointerAfter : $controlStructureEndPointer;
239253
$endOfLineBeforeAfterPointer = TokenHelper::findPreviousContent($phpcsFile, T_WHITESPACE, $phpcsFile->eolChar, $pointerAfter - 1);
240254

241-
for ($i = $controlStructureEndPointer + 1; $i <= $endOfLineBeforeAfterPointer; $i++) {
255+
for ($i = $replaceStartPointer + 1; $i <= $endOfLineBeforeAfterPointer; $i++) {
242256
$phpcsFile->fixer->replaceToken($i, '');
243257
}
244-
for ($i = 0; $i <= $requiredLinesCountAfter; $i++) {
245-
$phpcsFile->fixer->addNewline($controlStructureEndPointer);
258+
259+
if ($hasCommentAfter) {
260+
for ($i = 0; $i < $requiredLinesCountAfter; $i++) {
261+
$phpcsFile->fixer->addNewline($notWhitespacePointerAfter);
262+
}
263+
} else {
264+
for ($i = 0; $i <= $requiredLinesCountAfter; $i++) {
265+
$phpcsFile->fixer->addNewline($controlStructureEndPointer);
266+
}
246267
}
247268

248269
$phpcsFile->fixer->endChangeset();

‎tests/Sniffs/ControlStructures/ControlStructureSpacingSniffTest.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public function testDefaultSettingsErrors(): void
1919
{
2020
$report = self::checkFile(__DIR__ . '/data/controlStructureSpacingWithDefaultSettingsErrors.php');
2121

22-
self::assertSame(41, $report->getErrorCount());
22+
self::assertSame(46, $report->getErrorCount());
2323

2424
self::assertSniffError($report, 4, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_BEFORE_CONTROL_STRUCTURE, 'Expected 1 lines before "if", found 2.');
2525
self::assertSniffError($report, 4, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_CONTROL_STRUCTURE, 'Expected 1 lines after "if", found 0.');
@@ -61,6 +61,11 @@ public function testDefaultSettingsErrors(): void
6161
self::assertSniffError($report, 104, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_LAST_CONTROL_STRUCTURE, 'Expected 0 lines after "return", found 2.');
6262
self::assertSniffError($report, 108, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_BEFORE_CONTROL_STRUCTURE, 'Expected 1 lines before "yield", found 0.');
6363
self::assertSniffError($report, 108, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_LAST_CONTROL_STRUCTURE, 'Expected 0 lines after "yield", found 1.');
64+
self::assertSniffError($report, 124, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_BEFORE_CONTROL_STRUCTURE, 'Expected 1 lines before "if", found 0.');
65+
self::assertSniffError($report, 133, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_BEFORE_CONTROL_STRUCTURE, 'Expected 1 lines before "if", found 2.');
66+
self::assertSniffError($report, 142, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_LAST_CONTROL_STRUCTURE, 'Expected 0 lines after "break", found 1.');
67+
self::assertSniffError($report, 148, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_CONTROL_STRUCTURE, 'Expected 1 lines after "if", found 0.');
68+
self::assertSniffError($report, 155, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_CONTROL_STRUCTURE, 'Expected 1 lines after "if", found 2.');
6469

6570
self::assertAllFixedInFile($report);
6671
}
@@ -113,7 +118,7 @@ public function testModifiedSettingsErrors(): void
113118
],
114119
]);
115120

116-
self::assertSame(21, $report->getErrorCount());
121+
self::assertSame(23, $report->getErrorCount());
117122

118123
self::assertSniffError($report, 21, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_BEFORE_CONTROL_STRUCTURE, 'Expected 0 lines before "while", found 2.');
119124
self::assertSniffError($report, 21, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_CONTROL_STRUCTURE, 'Expected 0 lines after "while", found 1.');
@@ -136,6 +141,8 @@ public function testModifiedSettingsErrors(): void
136141
self::assertSniffError($report, 110, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_BEFORE_CONTROL_STRUCTURE, 'Expected 0 lines before "return", found 2.');
137142
self::assertSniffError($report, 110, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_LAST_CONTROL_STRUCTURE, 'Expected 3 lines after "return", found 2.');
138143
self::assertSniffError($report, 114, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_LAST_CONTROL_STRUCTURE, 'Expected 3 lines after "yield", found 1.');
144+
self::assertSniffError($report, 121, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_BEFORE_CONTROL_STRUCTURE, 'Expected 0 lines before "foreach", found 1.');
145+
self::assertSniffError($report, 131, ControlStructureSpacingSniff::CODE_INCORRECT_LINES_COUNT_AFTER_LAST_CONTROL_STRUCTURE, 'Expected 3 lines after "foreach", found 0.');
139146

140147
self::assertAllFixedInFile($report);
141148
}

‎tests/Sniffs/ControlStructures/data/controlStructureSpacingWithDefaultSettingsErrors.fixed.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,43 @@ function () {
104104

105105
return yield from $array;
106106
};
107+
108+
function () {
109+
doSomething(); // Do something
110+
111+
if (true) {
112+
113+
}
114+
};
115+
116+
function () {
117+
doSomething(); // Do something
118+
119+
if (true) {
120+
121+
}
122+
};
123+
124+
function () {
125+
foreach ([] as $value) {
126+
doSomething($value);
127+
128+
break; // Break
129+
}
130+
};
131+
132+
function () {
133+
if (true) {
134+
135+
} // If
136+
137+
doSomething();
138+
};
139+
140+
function () {
141+
if (true) {
142+
143+
} // If
144+
145+
doSomething();
146+
};

‎tests/Sniffs/ControlStructures/data/controlStructureSpacingWithDefaultSettingsErrors.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,44 @@ function () {
118118
$array = [];
119119
return yield from $array;
120120
};
121+
122+
function () {
123+
doSomething(); // Do something
124+
if (true) {
125+
126+
}
127+
};
128+
129+
function () {
130+
doSomething(); // Do something
131+
132+
133+
if (true) {
134+
135+
}
136+
};
137+
138+
function () {
139+
foreach ([] as $value) {
140+
doSomething($value);
141+
142+
break; // Break
143+
144+
}
145+
};
146+
147+
function () {
148+
if (true) {
149+
150+
} // If
151+
doSomething();
152+
};
153+
154+
function () {
155+
if (true) {
156+
157+
} // If
158+
159+
160+
doSomething();
161+
};

‎tests/Sniffs/ControlStructures/data/controlStructureSpacingWithModifiedSettingsErrors.fixed.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,4 +125,24 @@ function () {
125125

126126

127127

128+
};
129+
130+
function () {
131+
doSomething(); // Do something
132+
foreach ([] as $value) {
133+
134+
}
135+
136+
137+
138+
};
139+
140+
function () {
141+
doSomething();
142+
foreach ([] as $value) {
143+
144+
} // Foreach
145+
146+
147+
128148
};

‎tests/Sniffs/ControlStructures/data/controlStructureSpacingWithModifiedSettingsErrors.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,21 @@ function () {
114114
yield [];
115115

116116
};
117+
118+
function () {
119+
doSomething(); // Do something
120+
121+
foreach ([] as $value) {
122+
123+
}
124+
125+
126+
127+
};
128+
129+
function () {
130+
doSomething();
131+
foreach ([] as $value) {
132+
133+
} // Foreach
134+
};

0 commit comments

Comments
 (0)