EarlyExitSniff: fixed fixing of negative logical conditions

slevomat · Feb 7, 2018 · ea5fe67 · ea5fe67
1 parent 4b52495
commit ea5fe67
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 8 deletions.
diff --git a/SlevomatCodingStandard/Sniffs/ControlStructures/EarlyExitSniff.php b/SlevomatCodingStandard/Sniffs/ControlStructures/EarlyExitSniff.php
@@ -230,6 +230,12 @@ private function getNegativeCondition(\PHP_CodeSniffer\Files\File $phpcsFile, in
 
 		$condition = TokenHelper::getContent($phpcsFile, $startPointer, $endPointer);
 
+		$booleanNotPointer = TokenHelper::findNextEffective($phpcsFile, $startPointer);
+		if ($tokens[$booleanNotPointer]['code'] === T_BOOLEAN_NOT) {
+			$negativeCondition = preg_replace('~^!\\s*~', '', $condition);
+			return preg_replace('~^\(\\s*(.+?)\\s*\)\\s*$~', '\\1', $negativeCondition);
+		}
+
 		$booleanPointers = TokenHelper::findNextAll($phpcsFile, \PHP_CodeSniffer\Util\Tokens::$booleanOperators, $startPointer, $endPointer + 1);
 		if (count($booleanPointers) > 0) {
 			$uniqueBooleanOperators = array_unique(array_map(function (int $pointer) use ($tokens) {
@@ -274,12 +280,6 @@ private function getNegativeCondition(\PHP_CodeSniffer\Files\File $phpcsFile, in
 			return $negativeCondition;
 		}
 
-		$booleanNotPointer = TokenHelper::findNextEffective($phpcsFile, $startPointer);
-		if ($tokens[$booleanNotPointer]['code'] === T_BOOLEAN_NOT) {
-			$negativeCondition = preg_replace('~^!\\s*~', '', $condition);
-			return preg_replace('~^\(\\s*(.+?)\\s*\)\\s*~', '\\1', $negativeCondition);
-		}
-
 		if (TokenHelper::findNext($phpcsFile, [T_INSTANCEOF, T_BITWISE_AND], $startPointer, $endPointer + 1) !== null) {
 			return sprintf('!(%s)', $condition);
 		}

diff --git a/tests/Sniffs/ControlStructures/EarlyExitSniffTest.php b/tests/Sniffs/ControlStructures/EarlyExitSniffTest.php
@@ -15,13 +15,13 @@ public function testErrors(): void
 	{
 		$report = self::checkFile(__DIR__ . '/data/earlyExitErrors.php');
 
-		self::assertSame(29, $report->getErrorCount());
+		self::assertSame(31, $report->getErrorCount());
 
 		foreach ([6, 15, 24, 33, 42, 50, 58, 66, 74, 82, 90, 98, 108, 149, 157, 165, 191, 199, 207] as $line) {
 			self::assertSniffError($report, $line, EarlyExitSniff::CODE_EARLY_EXIT_NOT_USED, 'Use early exit instead of else.');
 		}
 
-		foreach ([115, 122, 129, 135, 141, 213, 222, 229] as $line) {
+		foreach ([115, 122, 129, 135, 141, 213, 222, 229, 235, 241] as $line) {
 			self::assertSniffError($report, $line, EarlyExitSniff::CODE_EARLY_EXIT_NOT_USED, 'Use early exit to reduce code nesting.');
 		}
 

diff --git a/tests/Sniffs/ControlStructures/data/earlyExitErrors.fixed.php b/tests/Sniffs/ControlStructures/data/earlyExitErrors.fixed.php
@@ -248,3 +248,19 @@ function binaryAndCondition() {
 
 	$this->eventManager->dispatchEvent($eventName, $event);
 }
+
+function negativeLogicalAndCondition() {
+	if (isset($parameterMappings[$parameterName]) && array_key_exists($parameterName, $parameterMappings)) {
+		return;
+	}
+
+	unset($parameters[$key]);
+}
+
+function negativeLogicalOrCondition() {
+	if (isset($parameterMappings[$parameterName]) || array_key_exists($parameterName, $parameterMappings)) {
+		return;
+	}
+
+	unset($parameters[$key]);
+}
diff --git a/tests/Sniffs/ControlStructures/data/earlyExitErrors.php b/tests/Sniffs/ControlStructures/data/earlyExitErrors.php
@@ -230,3 +230,15 @@ function binaryAndCondition() {
 		$this->eventManager->dispatchEvent($eventName, $event);
 	}
 }
+
+function negativeLogicalAndCondition() {
+	if (! (isset($parameterMappings[$parameterName]) && array_key_exists($parameterName, $parameterMappings))) {
+		unset($parameters[$key]);
+	}
+}
+
+function negativeLogicalOrCondition() {
+	if (! (isset($parameterMappings[$parameterName]) || array_key_exists($parameterName, $parameterMappings))) {
+		unset($parameters[$key]);
+	}
+}