diff --git a/CHANGELOG.md b/CHANGELOG.md index e14018ba8e..c01e8dd7ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,10 @@ The file documents changes to the PHP_CodeSniffer project. - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Fixed bug #3728 : PHP 8.2 | PSR1/SideEffects: allow for readonly classes - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch +- 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 + - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch +- Fixed bug #3739 : PEAR/FunctionDeclaration: prevent fixer conflict (and potentially creating a parse error) for unconventionally formatted return types + - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Fixed bug #3770 : Squiz/NonExecutableCode: prevent false positives for switching between PHP and HTML - Thanks to Dan Wallis (@fredden) for the patch - Fixed bug #3773 : Tokenizer/PHP: tokenization of the readonly keyword when used in combination with PHP 8.2 disjunctive normal types diff --git a/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php b/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php index af18a2b82d..4cc24a2d19 100644 --- a/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php +++ b/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php @@ -297,63 +297,74 @@ public function processMultiLineDeclaration($phpcsFile, $stackPtr, $tokens) return; } - // The opening brace needs to be one space away from the closing parenthesis. + // The opening brace needs to be on the same line as the closing parenthesis. + // There should only be one space between the closing parenthesis - or the end of the + // return type - and the opening brace. $opener = $tokens[$stackPtr]['scope_opener']; if ($tokens[$opener]['line'] !== $tokens[$closeBracket]['line']) { $error = 'The closing parenthesis and the opening brace of a multi-line function declaration must be on the same line'; - $fix = $phpcsFile->addFixableError($error, $opener, 'NewlineBeforeOpenBrace'); - if ($fix === true) { - $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), $closeBracket, true); - $phpcsFile->fixer->beginChangeset(); - $phpcsFile->fixer->addContent($prev, ' {'); - - // If the opener is on a line by itself, removing it will create - // an empty line, so just remove the entire line instead. - $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($opener - 1), $closeBracket, true); - $next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true); + $code = 'NewlineBeforeOpenBrace'; - if ($tokens[$prev]['line'] < $tokens[$opener]['line'] - && $tokens[$next]['line'] > $tokens[$opener]['line'] - ) { - // Clear the whole line. - for ($i = ($prev + 1); $i < $next; $i++) { - if ($tokens[$i]['line'] === $tokens[$opener]['line']) { - $phpcsFile->fixer->replaceToken($i, ''); - } - } - } else { - // Just remove the opener. - $phpcsFile->fixer->replaceToken($opener, ''); - if ($tokens[$next]['line'] === $tokens[$opener]['line']) { - $phpcsFile->fixer->replaceToken(($opener + 1), ''); - } - } - - $phpcsFile->fixer->endChangeset(); - }//end if - } else { - $prev = $tokens[($opener - 1)]; - if ($prev['code'] !== T_WHITESPACE) { - $length = 0; + $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), $closeBracket, true); + if ($tokens[$prev]['line'] === $tokens[$opener]['line']) { + // End of the return type is not on the same line as the close parenthesis. + $phpcsFile->addError($error, $opener, $code); } else { - $length = strlen($prev['content']); - } - - if ($length !== 1) { - $error = 'There must be a single space between the closing parenthesis and the opening brace of a multi-line function declaration; found %s spaces'; - $fix = $phpcsFile->addFixableError($error, ($opener - 1), 'SpaceBeforeOpenBrace', [$length]); + $fix = $phpcsFile->addFixableError($error, $opener, $code); if ($fix === true) { - if ($length === 0) { - $phpcsFile->fixer->addContentBefore($opener, ' '); + $phpcsFile->fixer->beginChangeset(); + $phpcsFile->fixer->addContent($prev, ' {'); + + // If the opener is on a line by itself, removing it will create + // an empty line, so remove the entire line instead. + $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($opener - 1), $closeBracket, true); + $next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true); + + if ($tokens[$prev]['line'] < $tokens[$opener]['line'] + && $tokens[$next]['line'] > $tokens[$opener]['line'] + ) { + // Clear the whole line. + for ($i = ($prev + 1); $i < $next; $i++) { + if ($tokens[$i]['line'] === $tokens[$opener]['line']) { + $phpcsFile->fixer->replaceToken($i, ''); + } + } } else { - $phpcsFile->fixer->replaceToken(($opener - 1), ' '); + // Just remove the opener. + $phpcsFile->fixer->replaceToken($opener, ''); + if ($tokens[$next]['line'] === $tokens[$opener]['line'] + && ($opener + 1) !== $next + ) { + $phpcsFile->fixer->replaceToken(($opener + 1), ''); + } } - } + + $phpcsFile->fixer->endChangeset(); + }//end if return; }//end if }//end if + $prev = $tokens[($opener - 1)]; + if ($prev['code'] !== T_WHITESPACE) { + $length = 0; + } else { + $length = strlen($prev['content']); + } + + if ($length !== 1) { + $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'; + $fix = $phpcsFile->addFixableError($error, ($opener - 1), 'SpaceBeforeOpenBrace', [$length]); + if ($fix === true) { + if ($length === 0) { + $phpcsFile->fixer->addContentBefore($opener, ' '); + } else { + $phpcsFile->fixer->replaceToken(($opener - 1), ' '); + } + } + } + }//end processMultiLineDeclaration() diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc index fb7cb52f4d..6ba3bd9f6a 100644 --- a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc +++ b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc @@ -465,3 +465,26 @@ new ExceptionMessage(), ) { } } + +// Issue #3736 - prevent the fixer creating a parse error by removing the function close brace. +class Test +{ + public function __construct( + protected int $id + ) + {} +} + +// Prevent fixer conflict with itself. +function foo( + $param1, +) +: \SomeClass + { + } + +function foo( + $param1, + $param2 +) : // comment. + \Package\Sub\SomeClass {} diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed index f82b5665a5..8248ee1dc8 100644 --- a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed +++ b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed @@ -463,3 +463,25 @@ new ExceptionMessage(), ) { } } + +// Issue #3736 - prevent the fixer creating a parse error by removing the function close brace. +class Test +{ + public function __construct( + protected int $id + ) { + } +} + +// Prevent fixer conflict with itself. +function foo( + $param1, +) +: \SomeClass { + } + +function foo( + $param1, + $param2 +) : // comment. + \Package\Sub\SomeClass {} diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php index 93a19d1b0d..d12c8c08bc 100644 --- a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php +++ b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php @@ -99,6 +99,9 @@ public function getErrorList($testFile='FunctionDeclarationUnitTest.inc') 371 => 1, 402 => 1, 406 => 1, + 475 => 1, + 483 => 1, + 490 => 2, ]; } else { $errors = [