prevent ternary sniff from introducing logic errors

Author: Khartir

SlevomatCodingStandard/Sniffs/ControlStructures/RequireTernaryOperatorSniff.php

@@ -14,6 +14,9 @@
 use const T_EQUAL;
 use const T_IF;
 use const T_INLINE_THEN;
+use const T_LOGICAL_AND;
+use const T_LOGICAL_OR;
+use const T_LOGICAL_XOR;
 use const T_RETURN;
 use const T_SEMICOLON;
 use const T_WHITESPACE;
@@ -84,14 +87,15 @@ private function checkIfWithReturns(File $phpcsFile, int $ifPointer, int $elsePo
 	{
 		$ifContainsComment = $this->containsComment($phpcsFile, $ifPointer);
 		$elseContainsComment = $this->containsComment($phpcsFile, $elsePointer);
+		$conditionContainsLogicalOperators = $this->containsLogicalOperators($phpcsFile, $ifPointer);
 
 		$errorParameters = [
 			'Use ternary operator.',
 			$ifPointer,
 			self::CODE_TERNARY_OPERATOR_NOT_USED,
 		];
 
-		if ($ifContainsComment || $elseContainsComment) {
+		if ($ifContainsComment || $elseContainsComment || $conditionContainsLogicalOperators) {
 			$phpcsFile->addError(...$errorParameters);
 			return;
 		}
@@ -174,14 +178,15 @@ private function checkIfWithAssignments(
 
 		$ifContainsComment = $this->containsComment($phpcsFile, $ifPointer);
 		$elseContainsComment = $this->containsComment($phpcsFile, $elsePointer);
+		$conditionContainsLogicalOperators = $this->containsLogicalOperators($phpcsFile, $ifPointer);
 
 		$errorParameters = [
 			'Use ternary operator.',
 			$ifPointer,
 			self::CODE_TERNARY_OPERATOR_NOT_USED,
 		];
 
-		if ($ifContainsComment || $elseContainsComment) {
+		if ($ifContainsComment || $elseContainsComment || $conditionContainsLogicalOperators) {
 			$phpcsFile->addError(...$errorParameters);
 			return;
 		}
@@ -253,4 +258,15 @@ private function containsComment(File $phpcsFile, int $scopeOwnerPointer): bool
 		) !== null;
 	}
 
+	private function containsLogicalOperators(File $phpcsFile, int $scopeOwnerPointer): bool
+	{
+		$tokens = $phpcsFile->getTokens();
+		return TokenHelper::findNext(
+			$phpcsFile,
+			[T_LOGICAL_AND, T_LOGICAL_OR, T_LOGICAL_XOR],
+			$tokens[$scopeOwnerPointer]['parenthesis_opener'] + 1,
+			$tokens[$scopeOwnerPointer]['parenthesis_closer']
+		) !== null;
+	}
+
 }

tests/Sniffs/ControlStructures/RequireTernaryOperatorSniffTest.php

@@ -16,7 +16,7 @@ public function testErrors(): void
 	{
 		$report = self::checkFile(__DIR__ . '/data/requireTernaryOperatorErrors.php');
 
-		self::assertSame(8, $report->getErrorCount());
+		self::assertSame(10, $report->getErrorCount());
 
 		self::assertSniffError($report, 4, RequireTernaryOperatorSniff::CODE_TERNARY_OPERATOR_NOT_USED);
 		self::assertSniffError($report, 12, RequireTernaryOperatorSniff::CODE_TERNARY_OPERATOR_NOT_USED);
@@ -26,6 +26,8 @@ public function testErrors(): void
 		self::assertSniffError($report, 42, RequireTernaryOperatorSniff::CODE_TERNARY_OPERATOR_NOT_USED);
 		self::assertSniffError($report, 54, RequireTernaryOperatorSniff::CODE_TERNARY_OPERATOR_NOT_USED);
 		self::assertSniffError($report, 63, RequireTernaryOperatorSniff::CODE_TERNARY_OPERATOR_NOT_USED);
+		self::assertSniffError($report, 75, RequireTernaryOperatorSniff::CODE_TERNARY_OPERATOR_NOT_USED);
+		self::assertSniffError($report, 82, RequireTernaryOperatorSniff::CODE_TERNARY_OPERATOR_NOT_USED);
 
 		self::assertAllFixedInFile($report);
 	}

tests/Sniffs/ControlStructures/data/requireTernaryOperatorErrors.fixed.php

@@ -55,3 +55,17 @@ public function __construct()
 	&& $association->getInversedBy()
 	&& $association instanceof OneToOneAssociationMetadata
  ? 'a' : 'aa';
+
+if (doAnything() and doNothing()) {
+	$a = 'a';
+} else {
+	$a = 'aa';
+}
+
+function () {
+	if (doAnything() and doNothing()) {
+		return 'a';
+	} else {
+		return 'aa';
+	}
+};

tests/Sniffs/ControlStructures/data/requireTernaryOperatorErrors.php

@@ -71,3 +71,17 @@ public function __construct()
 } else {
 	$a = 'aa';
 }
+
+if (doAnything() and doNothing()) {
+	$a = 'a';
+} else {
+	$a = 'aa';
+}
+
+function () {
+	if (doAnything() and doNothing()) {
+		return 'a';
+	} else {
+		return 'aa';
+	}
+};