Skip to content

Commit 8defc6e

Browse files
committed
Generic/OpeningFunctionBraceBsdAllman: bug fix - prevent removing return types
As reported in 3357, the `Generic.Functions.OpeningFunctionBraceBsdAllman` sniff would remove return types (and comments) when fixing code where blank lines existed between the end of the function declaration and the open brace. This commit fixes that bug. In the case of comments, the `BraceSpacing` error will no longer auto-fix as a dev should decide where the comment should go and/or whether it should be removed. Includes unit tests. Fixes 3357
1 parent fde816c commit 8defc6e

File tree

4 files changed

+72
-8
lines changed

4 files changed

+72
-8
lines changed

src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceBsdAllmanSniff.php

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,17 +139,31 @@ public function process(File $phpcsFile, $stackPtr)
139139
} else if ($lineDifference > 1) {
140140
$error = 'Opening brace should be on the line after the declaration; found %s blank line(s)';
141141
$data = [($lineDifference - 1)];
142-
$fix = $phpcsFile->addFixableError($error, $openingBrace, 'BraceSpacing', $data);
143-
if ($fix === true) {
144-
for ($i = ($tokens[$stackPtr]['parenthesis_closer'] + 1); $i < $openingBrace; $i++) {
145-
if ($tokens[$i]['line'] === $braceLine) {
146-
$phpcsFile->fixer->addNewLineBefore($i);
147-
break;
142+
143+
$prevNonWs = $phpcsFile->findPrevious(T_WHITESPACE, ($openingBrace - 1), $closeBracket, true);
144+
if ($prevNonWs !== $prev) {
145+
// There must be a comment between the end of the function declaration and the open brace.
146+
// Report, but don't fix.
147+
$phpcsFile->addError($error, $openingBrace, 'BraceSpacing', $data);
148+
} else {
149+
$fix = $phpcsFile->addFixableError($error, $openingBrace, 'BraceSpacing', $data);
150+
if ($fix === true) {
151+
$phpcsFile->fixer->beginChangeset();
152+
for ($i = $openingBrace; $i > $prev; $i--) {
153+
if ($tokens[$i]['line'] === $tokens[$openingBrace]['line']) {
154+
if ($tokens[$i]['column'] === 1) {
155+
$phpcsFile->fixer->addNewLineBefore($i);
156+
}
157+
158+
continue;
159+
}
160+
161+
$phpcsFile->fixer->replaceToken($i, '');
148162
}
149163

150-
$phpcsFile->fixer->replaceToken($i, '');
164+
$phpcsFile->fixer->endChangeset();
151165
}
152-
}
166+
}//end if
153167
}//end if
154168

155169
$ignore = Tokens::$phpcsCommentTokens;

src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,3 +236,28 @@ function myFunction($a, $lot, $of, $params)
236236
: array /* comment */ {
237237
return null;
238238
}
239+
240+
class Issue3357
241+
{
242+
public function extraLine(string: $a): void
243+
244+
{
245+
// code here.
246+
}
247+
}
248+
249+
function issue3357WithoutIndent(string: $a): void
250+
251+
252+
{
253+
// code here.
254+
}
255+
256+
class Issue3357WithComment
257+
{
258+
public function extraLine(string: $a): void
259+
// Comment.
260+
{
261+
// code here.
262+
}
263+
}

src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc.fixed

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,3 +256,25 @@ function myFunction($a, $lot, $of, $params)
256256
{
257257
return null;
258258
}
259+
260+
class Issue3357
261+
{
262+
public function extraLine(string: $a): void
263+
{
264+
// code here.
265+
}
266+
}
267+
268+
function issue3357WithoutIndent(string: $a): void
269+
{
270+
// code here.
271+
}
272+
273+
class Issue3357WithComment
274+
{
275+
public function extraLine(string: $a): void
276+
// Comment.
277+
{
278+
// code here.
279+
}
280+
}

src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ public function getErrorList()
5858
220 => 1,
5959
231 => 1,
6060
236 => 1,
61+
244 => 1,
62+
252 => 1,
63+
260 => 1,
6164
];
6265

6366
}//end getErrorList()

0 commit comments

Comments
 (0)