From 9d036f84fb206942bbd8d4c1c0718b0e069fb32b Mon Sep 17 00:00:00 2001 From: Greg Sherwood Date: Mon, 28 Sep 2020 08:46:15 +1000 Subject: [PATCH] Fixed bug #3075 : PSR12.ControlStructures.BooleanOperatorPlacement false positive when operator is the only content on line --- package.xml | 1 + .../BooleanOperatorPlacementSniff.php | 51 +++++++++++++------ .../BooleanOperatorPlacementUnitTest.inc | 10 ++++ ...BooleanOperatorPlacementUnitTest.inc.fixed | 10 ++++ 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/package.xml b/package.xml index f95be96af9..0cd2574fcd 100644 --- a/package.xml +++ b/package.xml @@ -61,6 +61,7 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Thanks to Sergei Morozov for the patch - Fixed bug #3066 : No support for namespace operator used in type declarations -- Thanks to Juliette Reinders Folmer for the patch + - Fixed bug #3075 : PSR12.ControlStructures.BooleanOperatorPlacement false positive when operator is the only content on line - Fixed bug #3099 : Squiz.WhiteSpace.OperatorSpacing false positive when exiting with negative number -- Thanks to Sergei Morozov for the patch - Fixed bug #3124 : PSR-12 not reporting error for empty lines with only whitespace diff --git a/src/Standards/PSR12/Sniffs/ControlStructures/BooleanOperatorPlacementSniff.php b/src/Standards/PSR12/Sniffs/ControlStructures/BooleanOperatorPlacementSniff.php index ad663db986..f101d399c9 100644 --- a/src/Standards/PSR12/Sniffs/ControlStructures/BooleanOperatorPlacementSniff.php +++ b/src/Standards/PSR12/Sniffs/ControlStructures/BooleanOperatorPlacementSniff.php @@ -90,35 +90,56 @@ public function process(File $phpcsFile, $stackPtr) break; } - $operators[] = $operator; - $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($operator - 1), $parenOpener, true); if ($prev === false) { // Parse error. return; } + $next = $phpcsFile->findNext(T_WHITESPACE, ($operator + 1), $parenCloser, true); + if ($next === false) { + // Parse error. + return; + } + + $firstOnLine = false; + $lastOnLine = false; + if ($tokens[$prev]['line'] < $tokens[$operator]['line']) { // The boolean operator is the first content on the line. - if ($position === null) { - $position = 'first'; - } + $firstOnLine = true; + } - if ($position !== 'first') { - $error = true; - } + if ($tokens[$next]['line'] > $tokens[$operator]['line']) { + // The boolean operator is the last content on the line. + $lastOnLine = true; + } + if ($firstOnLine === true && $lastOnLine === true) { + // The operator is the only content on the line. + // Don't record it because we can't determine + // placement information from looking at it. continue; } - $next = $phpcsFile->findNext(T_WHITESPACE, ($operator + 1), $parenCloser, true); - if ($next === false) { - // Parse error. - return; + $operators[] = $operator; + + if ($firstOnLine === false && $lastOnLine === false) { + // It's in the middle of content, so we can't determine + // placement information from looking at it, but we may + // still need to process it. + continue; } - if ($tokens[$next]['line'] > $tokens[$operator]['line']) { - // The boolean operator is the last content on the line. + if ($firstOnLine === true) { + if ($position === null) { + $position = 'first'; + } + + if ($position !== 'first') { + $error = true; + } + } else { if ($position === null) { $position = 'last'; } @@ -126,8 +147,6 @@ public function process(File $phpcsFile, $stackPtr) if ($position !== 'last') { $error = true; } - - continue; } } while ($operator !== false); diff --git a/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc b/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc index 3289f7eee7..cc2ae92d78 100644 --- a/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc +++ b/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc @@ -108,3 +108,13 @@ if ( ) { // elseif body } + +if ( + ($value == 1 || + $value == 2) + && + ($value == 3 || + $value == 4) +) { + return 5; +} diff --git a/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc.fixed b/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc.fixed index 5e8f0c3f9b..19792a7642 100644 --- a/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc.fixed +++ b/src/Standards/PSR12/Tests/ControlStructures/BooleanOperatorPlacementUnitTest.inc.fixed @@ -118,3 +118,13 @@ if ( ) { // elseif body } + +if ( + ($value == 1 || + $value == 2) + && + ($value == 3 || + $value == 4) +) { + return 5; +}