From 7d9062c6df456b2ce8bf215fd0b3a9b9b66a3c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Hansl=C3=ADk?= Date: Sat, 6 May 2023 12:02:05 +0200 Subject: [PATCH] SlevomatCodingStandard.ControlStructures.RequireNullSafeObjectOperator: Fixed false positive --- .../RequireNullSafeObjectOperatorSniff.php | 113 +++++++++++++----- .../requireNullSafeObjectOperatorNoErrors.php | 7 ++ 2 files changed, 92 insertions(+), 28 deletions(-) diff --git a/SlevomatCodingStandard/Sniffs/ControlStructures/RequireNullSafeObjectOperatorSniff.php b/SlevomatCodingStandard/Sniffs/ControlStructures/RequireNullSafeObjectOperatorSniff.php index 85d41793e..f71f9972f 100644 --- a/SlevomatCodingStandard/Sniffs/ControlStructures/RequireNullSafeObjectOperatorSniff.php +++ b/SlevomatCodingStandard/Sniffs/ControlStructures/RequireNullSafeObjectOperatorSniff.php @@ -69,36 +69,23 @@ public function process(File $phpcsFile, $identicalPointer): int $tokens = $phpcsFile->getTokens(); - $pointerBeforeIdentical = TokenHelper::findPreviousEffective($phpcsFile, $identicalPointer - 1); - $pointerAfterIdentical = TokenHelper::findNextEffective($phpcsFile, $identicalPointer + 1); + [$pointerBeforeIdentical, $pointerAfterIdentical] = $this->getIndenticalData($phpcsFile, $identicalPointer); if ($tokens[$pointerBeforeIdentical]['code'] !== T_NULL && $tokens[$pointerAfterIdentical]['code'] !== T_NULL) { return $identicalPointer + 1; } - $isYoda = $tokens[$pointerBeforeIdentical]['code'] === T_NULL; - - if ($isYoda) { - $identificatorStartPointer = $pointerAfterIdentical; - $identificatorEndPointer = $this->findIdentificatorEnd($phpcsFile, $identificatorStartPointer); - - if ($identificatorEndPointer === null) { - return $pointerAfterIdentical + 1; - } - - $conditionStartPointer = $pointerBeforeIdentical; - - } else { - $identificatorEndPointer = $pointerBeforeIdentical; - $identificatorStartPointer = $this->findIdentificatorStart($phpcsFile, $identificatorEndPointer); - - if ($identificatorStartPointer === null) { - return $identificatorEndPointer + 1; - } + [$identificatorStartPointer, $identificatorEndPointer, $conditionStartPointer] = $this->getConditionData( + $phpcsFile, + $pointerBeforeIdentical, + $pointerAfterIdentical + ); - $conditionStartPointer = $identificatorStartPointer; + if ($identificatorStartPointer === null || $identificatorEndPointer === null) { + return $identicalPointer + 1; } + $isYoda = $tokens[$pointerBeforeIdentical]['code'] === T_NULL; $identificator = IdentificatorHelper::getContent($phpcsFile, $identificatorStartPointer, $identificatorEndPointer); $pointerAfterCondition = TokenHelper::findNextEffective( @@ -159,6 +146,40 @@ private function checkTernaryOperator( } while (true); + $pointerBeforeCondition = TokenHelper::findPreviousEffective($phpcsFile, $conditionStartPointer - 1); + if (in_array($tokens[$pointerBeforeCondition]['code'], [T_BOOLEAN_AND, T_BOOLEAN_OR], true)) { + $previousIdenticalPointer = TokenHelper::findPreviousLocal( + $phpcsFile, + [T_IS_IDENTICAL, T_IS_NOT_IDENTICAL], + $pointerBeforeCondition + ); + + if ($previousIdenticalPointer !== null) { + [$pointerBeforePreviousIdentical, $pointerAfterPreviousIdentical] = $this->getIndenticalData( + $phpcsFile, + $previousIdenticalPointer + ); + + [$previousIdentificatorStartPointer, $previousIdentificatorEndPointer] = $this->getConditionData( + $phpcsFile, + $pointerBeforePreviousIdentical, + $pointerAfterPreviousIdentical + ); + + if ($previousIdentificatorStartPointer !== null && $previousIdentificatorEndPointer !== null) { + $previousIdentificator = IdentificatorHelper::getContent( + $phpcsFile, + $previousIdentificatorStartPointer, + $previousIdentificatorEndPointer + ); + + if (!self::areIdentificatorsCompatible($previousIdentificator, $identificator)) { + return; + } + } + } + } + $defaultInElse = $tokens[$identicalPointer]['code'] === T_IS_NOT_IDENTICAL; $inlineElsePointer = TernaryOperatorHelper::getElsePointer($phpcsFile, $inlineThenPointer); $inlineElseEndPointer = TernaryOperatorHelper::getEndPointer($phpcsFile, $inlineThenPointer, $inlineElsePointer); @@ -274,7 +295,7 @@ private function checkNextCondition( $nextIdentificator = IdentificatorHelper::getContent($phpcsFile, $nextIdentificatorStartPointer, $nextIdentificatorEndPointer); if (!$this->areIdentificatorsCompatible($identificator, $nextIdentificator)) { - return $nextConditionBooleanPointer; + return $nextIdentificatorEndPointer; } $pointerAfterNexIdentificator = TokenHelper::findNextEffective($phpcsFile, $nextIdentificatorEndPointer + 1); @@ -285,16 +306,16 @@ private function checkNextCondition( $tokens[$pointerAfterNexIdentificator]['code'] !== $tokens[$identicalPointer]['code'] && !in_array($tokens[$pointerAfterNexIdentificator]['code'], [T_INLINE_THEN, T_SEMICOLON], true) ) { - return $nextConditionBooleanPointer; + return $pointerAfterNexIdentificator; } if (!in_array($tokens[$pointerAfterNexIdentificator]['code'], [T_IS_IDENTICAL, T_IS_NOT_IDENTICAL], true)) { - return $nextConditionBooleanPointer; + return $pointerAfterNexIdentificator; } $pointerAfterIdentical = TokenHelper::findNextEffective($phpcsFile, $pointerAfterNexIdentificator + 1); if ($tokens[$pointerAfterIdentical]['code'] !== T_NULL) { - return $nextConditionBooleanPointer; + return $pointerAfterNexIdentificator; } $identificatorDifference = $this->getIdentificatorDifference( @@ -307,7 +328,7 @@ private function checkNextCondition( $fix = $phpcsFile->addFixableError('Operator ?-> is required.', $identicalPointer, self::CODE_REQUIRED_NULL_SAFE_OBJECT_OPERATOR); if (!$fix) { - return $nextConditionBooleanPointer; + return $pointerAfterNexIdentificator; } $isConditionOfTernaryOperator = TernaryOperatorHelper::isConditionOfTernaryOperator($phpcsFile, $identicalPointer); @@ -324,7 +345,7 @@ private function checkNextCondition( return TokenHelper::findNext($phpcsFile, T_INLINE_THEN, $identicalPointer + 1); } - return $nextConditionBooleanPointer; + return $pointerAfterNexIdentificator; } /** @@ -451,4 +472,40 @@ private function getIdentificatorDifference( return TokenHelper::getContent($phpcsFile, $differencePointer, $nextIdentificatorEndPointer); } + /** + * @return array{0: int, 1: int} + */ + private function getIndenticalData(File $phpcsFile, int $identicalPointer): array + { + /** @var int $pointerBeforeIdentical */ + $pointerBeforeIdentical = TokenHelper::findPreviousEffective($phpcsFile, $identicalPointer - 1); + /** @var int $pointerAfterIdentical */ + $pointerAfterIdentical = TokenHelper::findNextEffective($phpcsFile, $identicalPointer + 1); + + return [$pointerBeforeIdentical, $pointerAfterIdentical]; + } + + /** + * @return array{0: int|null, 1: int|null, 2: int|null} + */ + private function getConditionData(File $phpcsFile, int $pointerBeforeIdentical, int $pointerAfterIdentical): array + { + $tokens = $phpcsFile->getTokens(); + + $isYoda = $tokens[$pointerBeforeIdentical]['code'] === T_NULL; + + if ($isYoda) { + $identificatorStartPointer = $pointerAfterIdentical; + $identificatorEndPointer = $this->findIdentificatorEnd($phpcsFile, $identificatorStartPointer); + $conditionStartPointer = $pointerBeforeIdentical; + + } else { + $identificatorEndPointer = $pointerBeforeIdentical; + $identificatorStartPointer = $this->findIdentificatorStart($phpcsFile, $identificatorEndPointer); + $conditionStartPointer = $identificatorStartPointer; + } + + return [$identificatorStartPointer, $identificatorEndPointer, $conditionStartPointer]; + } + } diff --git a/tests/Sniffs/ControlStructures/data/requireNullSafeObjectOperatorNoErrors.php b/tests/Sniffs/ControlStructures/data/requireNullSafeObjectOperatorNoErrors.php index 436ef2346..589a286ed 100644 --- a/tests/Sniffs/ControlStructures/data/requireNullSafeObjectOperatorNoErrors.php +++ b/tests/Sniffs/ControlStructures/data/requireNullSafeObjectOperatorNoErrors.php @@ -54,3 +54,10 @@ function ($something): bool { if ($a === null || $a->size() === 0) { } + +function format($format, $date): ?string +{ + return $format !== null && $date !== null + ? $date->format($format) + : null; +}