Skip to content

Commit dcc257f

Browse files
jrfnljrfnl
committed
PEAR/FunctionDeclaration: prevent fixer conflict
If a return type declaration was not confined to one line, the sniff could have a fixer conflict with itself. The fixer would also potentially remove a close curly on the same line, causing parse errors. Fixed now. The diff will be most straight forward to review while ignoring whitespace changes. Includes unit tests.
1 parent ed6c437 commit dcc257f

File tree

5 files changed

+86
-46
lines changed

5 files changed

+86
-46
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ The file documents changes to the PHP_CodeSniffer project.
111111
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
112112
- Fixed bug #3736 : PEAR/FunctionDeclaration: prevent fixer removing the close brace (and creating a parse error) when there is no space between the open brace and close brace of a function
113113
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
114+
- Fixed bug #3739 : PEAR/FunctionDeclaration: prevent fixer conflict (and potentially creating a parse error) for unconventionally formatted return types
115+
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
114116
- Fixed bug #3770 : Squiz/NonExecutableCode: prevent false positives for switching between PHP and HTML
115117
- Thanks to Dan Wallis (@fredden) for the patch
116118
- Fixed bug #3773 : Tokenizer/PHP: tokenization of the readonly keyword when used in combination with PHP 8.2 disjunctive normal types

‎src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php

+55-46
Original file line numberDiff line numberDiff line change
@@ -297,65 +297,74 @@ public function processMultiLineDeclaration($phpcsFile, $stackPtr, $tokens)
297297
return;
298298
}
299299

300-
// The opening brace needs to be one space away from the closing parenthesis.
300+
// The opening brace needs to be on the same line as the closing parenthesis.
301+
// There should only be one space between the closing parenthesis - or the end of the
302+
// return type - and the opening brace.
301303
$opener = $tokens[$stackPtr]['scope_opener'];
302304
if ($tokens[$opener]['line'] !== $tokens[$closeBracket]['line']) {
303305
$error = 'The closing parenthesis and the opening brace of a multi-line function declaration must be on the same line';
304-
$fix = $phpcsFile->addFixableError($error, $opener, 'NewlineBeforeOpenBrace');
305-
if ($fix === true) {
306-
$prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), $closeBracket, true);
307-
$phpcsFile->fixer->beginChangeset();
308-
$phpcsFile->fixer->addContent($prev, ' {');
309-
310-
// If the opener is on a line by itself, removing it will create
311-
// an empty line, so remove the entire line instead.
312-
$prev = $phpcsFile->findPrevious(T_WHITESPACE, ($opener - 1), $closeBracket, true);
313-
$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);
306+
$code = 'NewlineBeforeOpenBrace';
314307

315-
if ($tokens[$prev]['line'] < $tokens[$opener]['line']
316-
&& $tokens[$next]['line'] > $tokens[$opener]['line']
317-
) {
318-
// Clear the whole line.
319-
for ($i = ($prev + 1); $i < $next; $i++) {
320-
if ($tokens[$i]['line'] === $tokens[$opener]['line']) {
321-
$phpcsFile->fixer->replaceToken($i, '');
322-
}
323-
}
324-
} else {
325-
// Just remove the opener.
326-
$phpcsFile->fixer->replaceToken($opener, '');
327-
if ($tokens[$next]['line'] === $tokens[$opener]['line']
328-
&& ($opener + 1) !== $next
329-
) {
330-
$phpcsFile->fixer->replaceToken(($opener + 1), '');
331-
}
332-
}
333-
334-
$phpcsFile->fixer->endChangeset();
335-
}//end if
336-
} else {
337-
$prev = $tokens[($opener - 1)];
338-
if ($prev['code'] !== T_WHITESPACE) {
339-
$length = 0;
308+
$prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), $closeBracket, true);
309+
if ($tokens[$prev]['line'] === $tokens[$opener]['line']) {
310+
// End of the return type is not on the same line as the close parenthesis.
311+
$phpcsFile->addError($error, $opener, $code);
340312
} else {
341-
$length = strlen($prev['content']);
342-
}
343-
344-
if ($length !== 1) {
345-
$error = 'There must be a single space between the closing parenthesis and the opening brace of a multi-line function declaration; found %s spaces';
346-
$fix = $phpcsFile->addFixableError($error, ($opener - 1), 'SpaceBeforeOpenBrace', [$length]);
313+
$fix = $phpcsFile->addFixableError($error, $opener, $code);
347314
if ($fix === true) {
348-
if ($length === 0) {
349-
$phpcsFile->fixer->addContentBefore($opener, ' ');
315+
$phpcsFile->fixer->beginChangeset();
316+
$phpcsFile->fixer->addContent($prev, ' {');
317+
318+
// If the opener is on a line by itself, removing it will create
319+
// an empty line, so remove the entire line instead.
320+
$prev = $phpcsFile->findPrevious(T_WHITESPACE, ($opener - 1), $closeBracket, true);
321+
$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);
322+
323+
if ($tokens[$prev]['line'] < $tokens[$opener]['line']
324+
&& $tokens[$next]['line'] > $tokens[$opener]['line']
325+
) {
326+
// Clear the whole line.
327+
for ($i = ($prev + 1); $i < $next; $i++) {
328+
if ($tokens[$i]['line'] === $tokens[$opener]['line']) {
329+
$phpcsFile->fixer->replaceToken($i, '');
330+
}
331+
}
350332
} else {
351-
$phpcsFile->fixer->replaceToken(($opener - 1), ' ');
333+
// Just remove the opener.
334+
$phpcsFile->fixer->replaceToken($opener, '');
335+
if ($tokens[$next]['line'] === $tokens[$opener]['line']
336+
&& ($opener + 1) !== $next
337+
) {
338+
$phpcsFile->fixer->replaceToken(($opener + 1), '');
339+
}
352340
}
353-
}
341+
342+
$phpcsFile->fixer->endChangeset();
343+
}//end if
354344

355345
return;
356346
}//end if
357347
}//end if
358348

349+
$prev = $tokens[($opener - 1)];
350+
if ($prev['code'] !== T_WHITESPACE) {
351+
$length = 0;
352+
} else {
353+
$length = strlen($prev['content']);
354+
}
355+
356+
if ($length !== 1) {
357+
$error = 'There must be a single space between the closing parenthesis/return type and the opening brace of a multi-line function declaration; found %s spaces';
358+
$fix = $phpcsFile->addFixableError($error, ($opener - 1), 'SpaceBeforeOpenBrace', [$length]);
359+
if ($fix === true) {
360+
if ($length === 0) {
361+
$phpcsFile->fixer->addContentBefore($opener, ' ');
362+
} else {
363+
$phpcsFile->fixer->replaceToken(($opener - 1), ' ');
364+
}
365+
}
366+
}
367+
359368
}//end processMultiLineDeclaration()
360369

361370

‎src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc

+14
Original file line numberDiff line numberDiff line change
@@ -474,3 +474,17 @@ class Test
474474
)
475475
{}
476476
}
477+
478+
// Prevent fixer conflict with itself.
479+
function foo(
480+
$param1,
481+
)
482+
: \SomeClass
483+
{
484+
}
485+
486+
function foo(
487+
$param1,
488+
$param2
489+
) : // comment.
490+
\Package\Sub\SomeClass {}

‎src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed

+13
Original file line numberDiff line numberDiff line change
@@ -472,3 +472,16 @@ class Test
472472
) {
473473
}
474474
}
475+
476+
// Prevent fixer conflict with itself.
477+
function foo(
478+
$param1,
479+
)
480+
: \SomeClass {
481+
}
482+
483+
function foo(
484+
$param1,
485+
$param2
486+
) : // comment.
487+
\Package\Sub\SomeClass {}

‎src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php

+2
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ public function getErrorList($testFile='FunctionDeclarationUnitTest.inc')
100100
402 => 1,
101101
406 => 1,
102102
475 => 1,
103+
483 => 1,
104+
490 => 2,
103105
];
104106
} else {
105107
$errors = [

0 commit comments

Comments
 (0)