diff --git a/src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php b/src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php index 97007fc458..2a87978a0c 100644 --- a/src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php +++ b/src/Standards/Squiz/Sniffs/WhiteSpace/OperatorSpacingSniff.php @@ -238,7 +238,8 @@ public function process(File $phpcsFile, $stackPtr) ) { // Throw an error for assignments only if enabled using the sniff property // because other standards allow multiple spaces to align assignments. - if ($tokens[($stackPtr - 2)]['line'] !== $tokens[$stackPtr]['line']) { + $prevNonWhitespace = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); + if ($tokens[$prevNonWhitespace]['line'] !== $tokens[$stackPtr]['line']) { $found = 'newline'; } else { $found = $tokens[($stackPtr - 1)]['length']; @@ -253,20 +254,29 @@ public function process(File $phpcsFile, $stackPtr) $operator, $found, ]; - $fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingBefore', $data); - if ($fix === true) { - $phpcsFile->fixer->beginChangeset(); - if ($found === 'newline') { - $i = ($stackPtr - 2); - while ($tokens[$i]['code'] === T_WHITESPACE) { - $phpcsFile->fixer->replaceToken($i, ''); - $i--; + + if (isset(Tokens::$commentTokens[$tokens[$prevNonWhitespace]['code']]) === true) { + // Throw a non-fixable error if the token on the previous line is a comment token, + // as in that case it's not for the sniff to decide where the comment should be moved to + // and it would get us into unfixable situations as the new line char is included + // in the contents of the comment token. + $phpcsFile->addError($error, $stackPtr, 'SpacingBefore', $data); + } else { + $fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingBefore', $data); + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + if ($found === 'newline') { + $i = ($stackPtr - 2); + while ($tokens[$i]['code'] === T_WHITESPACE) { + $phpcsFile->fixer->replaceToken($i, ''); + $i--; + } } - } - $phpcsFile->fixer->replaceToken(($stackPtr - 1), ' '); - $phpcsFile->fixer->endChangeset(); - } + $phpcsFile->fixer->replaceToken(($stackPtr - 1), ' '); + $phpcsFile->fixer->endChangeset(); + } + }//end if }//end if }//end if diff --git a/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc b/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc index 765e7ab717..29acf308a6 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc +++ b/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc @@ -483,5 +483,28 @@ match ($a) { default => -3, }; -/* Intentional parse error. This has to be the last test in the file. */ -$a = 10 + +$foo = $var + ? 10 + : true; + +// Safeguard that a non-fixable error is thrown when there is a new line before the operator, +// but the last non-whitespace token before the operator is a comment token. +$foo = $var // Comment + ? false /* Comment */ + : true; + +$foo = $var // phpcs: ignore Stnd.Cat.Sniff -- for reasons. + + + ? $something /** + * Don't ask, but someone might have a docblock between the lines. It's valid PHP after all. + */ + + + : true; + +// phpcs:set Squiz.WhiteSpace.OperatorSpacing ignoreNewlines true +$foo = $var // Comment + ? false // Comment + : true; +// phpcs:set Squiz.WhiteSpace.OperatorSpacing ignoreNewlines false diff --git a/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc.fixed index ada77fa859..5c94e3658b 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.1.inc.fixed @@ -477,5 +477,26 @@ match ($a) { default => -3, }; -/* Intentional parse error. This has to be the last test in the file. */ -$a = 10 + +$foo = $var ? 10 : true; + +// Safeguard that a non-fixable error is thrown when there is a new line before the operator, +// but the last non-whitespace token before the operator is a comment token. +$foo = $var // Comment + ? false /* Comment */ + : true; + +$foo = $var // phpcs: ignore Stnd.Cat.Sniff -- for reasons. + + + ? $something /** + * Don't ask, but someone might have a docblock between the lines. It's valid PHP after all. + */ + + + : true; + +// phpcs:set Squiz.WhiteSpace.OperatorSpacing ignoreNewlines true +$foo = $var // Comment + ? false // Comment + : true; +// phpcs:set Squiz.WhiteSpace.OperatorSpacing ignoreNewlines false diff --git a/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.3.inc b/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.3.inc new file mode 100644 index 0000000000..8d6cf19e4c --- /dev/null +++ b/src/Standards/Squiz/Tests/WhiteSpace/OperatorSpacingUnitTest.3.inc @@ -0,0 +1,6 @@ + 2, 266 => 2, 271 => 2, + 487 => 1, + 488 => 1, + 493 => 1, + 494 => 1, + 499 => 1, + 504 => 1, ]; case 'OperatorSpacingUnitTest.js':