RequireNullSafeObjectOperatorSniff: Fixed false positive

Author: kukulich

SlevomatCodingStandard/Helpers/TernaryOperatorHelper.php

@@ -15,6 +15,7 @@
 use const T_DOUBLE_ARROW;
 use const T_EQUAL;
 use const T_INLINE_ELSE;
+use const T_INLINE_THEN;
 use const T_OPEN_PARENTHESIS;
 use const T_OPEN_SHORT_ARRAY;
 use const T_OPEN_SQUARE_BRACKET;
@@ -30,6 +31,19 @@
 class TernaryOperatorHelper
 {
 
+	public static function isConditionOfTernaryOperator(File $phpcsFile, int $pointer): bool
+	{
+		$inlineThenPointer = TokenHelper::findNext($phpcsFile, T_INLINE_THEN, $pointer + 1);
+
+		if ($inlineThenPointer === null) {
+			return false;
+		}
+
+		$startPointer = self::getStartPointer($phpcsFile, $inlineThenPointer);
+
+		return $startPointer <= $pointer;
+	}
+
 	public static function getElsePointer(File $phpcsFile, int $inlineThenPointer): int
 	{
 		$tokens = $phpcsFile->getTokens();

SlevomatCodingStandard/Sniffs/ControlStructures/RequireNullSafeObjectOperatorSniff.php

@@ -58,13 +58,14 @@ public function register(): array
 	 * @phpcsSuppress SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingNativeTypeHint
 	 * @param File $phpcsFile
 	 * @param int $identicalPointer
+	 * @return int
 	 */
-	public function process(File $phpcsFile, $identicalPointer): void
+	public function process(File $phpcsFile, $identicalPointer): int
 	{
 		$this->enable = SniffSettingsHelper::isEnabledByPhpVersion($this->enable, 80000);
 
 		if (!$this->enable) {
-			return;
+			return $identicalPointer + 1;
 		}
 
 		$tokens = $phpcsFile->getTokens();
@@ -73,7 +74,7 @@ public function process(File $phpcsFile, $identicalPointer): void
 		$pointerAfterIdentical = TokenHelper::findNextEffective($phpcsFile, $identicalPointer + 1);
 
 		if ($tokens[$pointerBeforeIdentical]['code'] !== T_NULL && $tokens[$pointerAfterIdentical]['code'] !== T_NULL) {
-			return;
+			return $identicalPointer + 1;
 		}
 
 		$isYoda = $tokens[$pointerBeforeIdentical]['code'] === T_NULL;
@@ -83,7 +84,7 @@ public function process(File $phpcsFile, $identicalPointer): void
 			$identificatorEndPointer = $this->findIdentificatorEnd($phpcsFile, $identificatorStartPointer);
 
 			if ($identificatorEndPointer === null) {
-				return;
+				return $pointerAfterIdentical + 1;
 			}
 
 			$conditionStartPointer = $pointerBeforeIdentical;
@@ -93,7 +94,7 @@ public function process(File $phpcsFile, $identicalPointer): void
 			$identificatorStartPointer = $this->findIdentificatorStart($phpcsFile, $identificatorEndPointer);
 
 			if ($identificatorStartPointer === null) {
-				return;
+				return $identificatorEndPointer + 1;
 			}
 
 			$conditionStartPointer = $identificatorStartPointer;
@@ -108,10 +109,15 @@ public function process(File $phpcsFile, $identicalPointer): void
 
 		$allowedBooleanCondition = $tokens[$identicalPointer]['code'] === T_IS_NOT_IDENTICAL ? T_BOOLEAN_AND : T_BOOLEAN_OR;
 		if ($tokens[$pointerAfterCondition]['code'] === $allowedBooleanCondition) {
-			$this->checkNextCondition($phpcsFile, $identicalPointer, $conditionStartPointer, $identificator, $pointerAfterCondition);
-		} elseif ($tokens[$pointerAfterCondition]['code'] === T_INLINE_THEN) {
+			return $this->checkNextCondition($phpcsFile, $identicalPointer, $conditionStartPointer, $identificator, $pointerAfterCondition);
+		}
+
+		if ($tokens[$pointerAfterCondition]['code'] === T_INLINE_THEN) {
 			$this->checkTernaryOperator($phpcsFile, $identicalPointer, $conditionStartPointer, $identificator, $pointerAfterCondition);
+			return $pointerAfterCondition + 1;
 		}
+
+		return $identicalPointer + 1;
 	}
 
 	private function checkTernaryOperator(
@@ -258,20 +264,20 @@ private function checkNextCondition(
 		int $conditionStartPointer,
 		string $identificator,
 		int $nextConditionBooleanPointer
-	): void
+	): int
 	{
 		$nextIdentificatorPointers = $this->getNextIdentificator($phpcsFile, $nextConditionBooleanPointer);
 
 		if ($nextIdentificatorPointers === null) {
-			return;
+			return $nextConditionBooleanPointer;
 		}
 
 		[$nextIdentificatorStartPointer, $nextIdentificatorEndPointer] = $nextIdentificatorPointers;
 
 		$nextIdentificator = IdentificatorHelper::getContent($phpcsFile, $nextIdentificatorStartPointer, $nextIdentificatorEndPointer);
 
 		if (!$this->areIdentificatorsCompatible($identificator, $nextIdentificator)) {
-			return;
+			return $nextConditionBooleanPointer;
 		}
 
 		$pointerAfterNexIdentificator = TokenHelper::findNextEffective($phpcsFile, $nextIdentificatorEndPointer + 1);
@@ -282,13 +288,17 @@ private function checkNextCondition(
 			$tokens[$pointerAfterNexIdentificator]['code'] !== $tokens[$identicalPointer]['code']
 			&& !in_array($tokens[$pointerAfterNexIdentificator]['code'], [T_INLINE_THEN, T_SEMICOLON], true)
 		) {
-			return;
+			return $nextConditionBooleanPointer;
+		}
+
+		if (!in_array($tokens[$pointerAfterNexIdentificator]['code'], [T_IS_IDENTICAL, T_IS_NOT_IDENTICAL], true)) {
+			return $nextConditionBooleanPointer;
 		}
 
 		if ($tokens[$pointerAfterNexIdentificator]['code'] === T_IS_NOT_IDENTICAL) {
 			$pointerAfterNotIdentical = TokenHelper::findNextEffective($phpcsFile, $pointerAfterNexIdentificator + 1);
 			if ($tokens[$pointerAfterNotIdentical]['code'] !== T_NULL) {
-				return;
+				return $nextConditionBooleanPointer;
 			}
 		}
 
@@ -302,9 +312,11 @@ private function checkNextCondition(
 		$fix = $phpcsFile->addFixableError('Operator ?-> is required.', $identicalPointer, self::CODE_REQUIRED_NULL_SAFE_OBJECT_OPERATOR);
 
 		if (!$fix) {
-			return;
+			return $nextConditionBooleanPointer;
 		}
 
+		$isConditionOfTernaryOperator = TernaryOperatorHelper::isConditionOfTernaryOperator($phpcsFile, $identicalPointer);
+
 		$phpcsFile->fixer->beginChangeset();
 
 		$phpcsFile->fixer->replaceToken($conditionStartPointer, sprintf('%s?%s', $identificator, $identificatorDifference));
@@ -314,6 +326,12 @@ private function checkNextCondition(
 		}
 
 		$phpcsFile->fixer->endChangeset();
+
+		if ($isConditionOfTernaryOperator) {
+			return TokenHelper::findNext($phpcsFile, T_INLINE_THEN, $identicalPointer + 1);
+		}
+
+		return $nextConditionBooleanPointer;
 	}
 
 	/**

tests/Sniffs/ControlStructures/RequireNullSafeObjectOperatorSniffTest.php

@@ -21,9 +21,9 @@ public function testErrors(): void
 			'enable' => true,
 		]);
 
-		self::assertSame(15, $report->getErrorCount());
+		self::assertSame(16, $report->getErrorCount());
 
-		foreach ([3, 4, 6, 7, 9, 10, 12, 13, 15, 18, 27] as $line) {
+		foreach ([3, 4, 6, 7, 9, 10, 12, 13, 15, 18, 27, 34] as $line) {
 			self::assertSniffError($report, $line, RequireNullSafeObjectOperatorSniff::CODE_REQUIRED_NULL_SAFE_OBJECT_OPERATOR);
 		}
 

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

@@ -27,3 +27,8 @@ function ($gatewayData, $response) {
 		$this->getSerializedResponse($response),
 	);
 };
+
+// Must be last
+if ($b->getC()?->getD() !== null) {
+
+}

tests/Sniffs/ControlStructures/data/requireNullSafeObjectOperatorErrors.php

@@ -29,3 +29,8 @@ function ($gatewayData, $response) {
 		$this->getSerializedResponse($response),
 	);
 };
+
+// Must be last
+if ($b->getC() !== null && $b->getC()->getD() !== null) {
+
+}

tests/Sniffs/ControlStructures/data/requireNullSafeObjectOperatorNoErrors.php

@@ -44,3 +44,7 @@
 $a = $b && $c->getD() !== null ? $c->getD()->getValue() : null;
 
 $a = $b !== false && $c->getD() !== null ? $c->getD()->getValue() : null;
+
+function ($something): bool {
+	return $something->getA() !== null && $something->getA()->isBoolean();
+};