From 263278b503270ce46551b3a8eb6f20585912e43f Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Sat, 31 Oct 2026 18:49:36 +0100 Subject: [PATCH 1/3] Report ternary operators that always give one of their branches The rule is behind the unnecessaryTernary toggle, which bleeding edge turns on. It covers the short ?: form too. Co-Authored-By: Claude Opus 5.5 (1M context) --- conf/bleedingEdge.neon | 1 + conf/config.neon | 1 + conf/parametersSchema.neon | 1 + .../UnnecessaryTernaryOperatorRule.php | 292 ++++++++++++++++++ .../UnnecessaryTernaryOperatorRuleTest.php | 166 ++++++++++ .../Comparison/data/unnecessary-ternary.php | 221 +++++++++++++ 6 files changed, 682 insertions(+) create mode 100644 src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php create mode 100644 tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php create mode 100644 tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 7098877fb44..01e20e44486 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -23,6 +23,7 @@ parameters: unusedLabel: true newOnNonObject: true unnecessaryNullCoalesce: true + unnecessaryTernary: true finiteTypesInHaystack: true switchConditionAlwaysFalse: true checkImportedClassNameCase: true diff --git a/conf/config.neon b/conf/config.neon index 0eb6c0c764b..3930d28fad1 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -55,6 +55,7 @@ parameters: unusedLabel: false newOnNonObject: false unnecessaryNullCoalesce: false + unnecessaryTernary: false finiteTypesInHaystack: false switchConditionAlwaysFalse: false checkImportedClassNameCase: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 62dab9d9c3b..945e9b25e0c 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -52,6 +52,7 @@ parametersSchema: unusedLabel: bool() newOnNonObject: bool() unnecessaryNullCoalesce: bool() + unnecessaryTernary: bool() finiteTypesInHaystack: bool() switchConditionAlwaysFalse: bool() checkImportedClassNameCase: bool() diff --git a/src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php b/src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php new file mode 100644 index 00000000000..f16b831c052 --- /dev/null +++ b/src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php @@ -0,0 +1,292 @@ + + */ +#[RegisteredRule(level: 4)] +final class UnnecessaryTernaryOperatorRule implements Rule +{ + + public function __construct( + private ConstantConditionRuleHelper $helper, + private ConstantConditionInTraitHelper $constantConditionInTraitHelper, + private ReflectionProvider $reflectionProvider, + private ExprPrinter $exprPrinter, + #[AutowiredParameter(ref: '%featureToggles.unnecessaryTernary%')] + private bool $unnecessaryTernary, + #[AutowiredParameter] + private bool $treatPhpDocTypesAsCertain, + #[AutowiredParameter(ref: '%tips.treatPhpDocTypesAsCertain%')] + private bool $treatPhpDocTypesAsCertainTip, + ) + { + } + + public function getNodeType(): string + { + return Node\Expr\Ternary::class; + } + + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array + { + if (!$this->unnecessaryTernary) { + return []; + } + + $error = $this->check($node, $scope); + if ($error === null) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $node); + return []; + } + + if ($scope->isInTrait()) { + $this->constantConditionInTraitHelper->emitError(self::class, $scope, $node, true, $error); + return []; + } + + return [$error]; + } + + private function check(Node\Expr\Ternary $node, Scope $scope): ?IdentifierRuleError + { + // TernaryOperatorConstantConditionRule already reports these + $conditionType = $this->helper->getBooleanType($scope, $node->cond); + if ($conditionType->isTrue()->yes() || $conditionType->isFalse()->yes()) { + return null; + } + + $result = $this->findResult($node, $scope, !$this->treatPhpDocTypesAsCertain); + if ($result === null) { + return null; + } + + $ruleErrorBuilder = RuleErrorBuilder::message(sprintf( + $node->if === null + ? 'Short ternary operator ?: is unnecessary because it always results in %s.' + : 'Ternary operator is unnecessary because it always results in %s.', + $this->exprPrinter->printExpr($result), + ))->identifier('ternary.unnecessary'); + + if ( + $this->treatPhpDocTypesAsCertain + && $this->treatPhpDocTypesAsCertainTip + && $this->findResult($node, $scope, true) === null + ) { + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + } + + return $ruleErrorBuilder->build(); + } + + /** + * The branch the ternary always results in, or null. + */ + private function findResult(Node\Expr\Ternary $node, Scope $scope, bool $native): ?Expr + { + // Without the ternary, the result is evaluated every time, and the + // condition and the other branch are not evaluated at all. + if (!$this->isPure($node->else, $scope)) { + return null; + } + + if (!$this->isTruthinessNarrowedExactly($node->cond, $scope, $native)) { + return null; + } + + if ($node->if === null) { + if (!$this->isAlwaysResult($scope->filterByFalseyValue($node->cond), $node->cond, $scope, $node->else, $native)) { + return null; + } + + return $node->cond; + } + + if (!$this->isPure($node->cond, $scope) || !$this->isPure($node->if, $scope)) { + return null; + } + + // The result then also runs in the scope of the other branch, where a + // function call can get an argument it does not accept, for example + // `$s === false ? 0 : strlen($s)`. + if (!$this->containsCall($node->if) && $this->isAlwaysResult($scope->filterByFalseyValue($node->cond), $node->if, $scope, $node->else, $native)) { + return $node->if; + } + if (!$this->containsCall($node->else) && $this->isAlwaysResult($scope->filterByTruthyValue($node->cond), $node->else, $scope, $node->if, $native)) { + return $node->else; + } + + return null; + } + + /** + * Whether $result always has the single value of $other in $branchScope, + * the scope in which the ternary picks $other. + */ + private function isAlwaysResult(Scope $branchScope, Expr $result, Scope $scope, Expr $other, bool $native): bool + { + // a fresh copy, because the walked node keeps the type it had in its own branch + $result = (new NodeTraverser(new CloningVisitor()))->traverse([$result])[0]; + if (!$result instanceof Expr) { + return false; + } + + if ($native) { + $resultType = $branchScope->getNativeType($result); + $otherType = $scope->getNativeType($other); + } else { + $resultType = $branchScope->getType($result); + $otherType = $scope->getType($other); + } + + // enum cases are not constant values, but each of them is a single value + if (!$otherType->isConstantValue()->yes() && count($otherType->getFiniteTypes()) !== 1) { + return false; + } + if (!$resultType->equals($otherType)) { + return false; + } + + // 0.0 === -0.0, so `$f ?: 0.0` turns -0.0 into 0.0 + return $otherType->isFloat()->no(); + } + + /** + * Narrowing by truthiness treats every object as truthy, but an empty + * SimpleXMLElement or a zero GMP is falsy. Then the scope of the other + * branch does not hold all values that reach it. + */ + private function isTruthinessNarrowedExactly(Expr $cond, Scope $scope, bool $native): bool + { + if ($cond instanceof Expr\BooleanNot) { + return $this->isTruthinessNarrowedExactly($cond->expr, $scope, $native); + } + + if ( + $cond instanceof Expr\BinaryOp\BooleanAnd + || $cond instanceof Expr\BinaryOp\BooleanOr + || $cond instanceof Expr\BinaryOp\LogicalAnd + || $cond instanceof Expr\BinaryOp\LogicalOr + ) { + return $this->isTruthinessNarrowedExactly($cond->left, $scope, $native) && $this->isTruthinessNarrowedExactly($cond->right, $scope, $native); + } + + $type = $native ? $scope->getNativeType($cond) : $scope->getType($cond); + $objectType = new ObjectWithoutClassType(); + if ($objectType->isSuperTypeOf($type)->no()) { + return true; + } + + return TypeCombinator::intersect($type, $objectType)->toBoolean()->isTrue()->yes(); + } + + private function containsCall(Expr $expr): bool + { + return (new NodeFinder())->findFirstInstanceOf([$expr], Expr\FuncCall::class) !== null; + } + + /** + * Whether evaluating the expression can only produce its value, so that + * dropping it or evaluating it once more does not change anything. + */ + private function isPure(Expr $expr, Scope $scope): bool + { + if ($expr instanceof Expr\Variable) { + return is_string($expr->name); + } + + if ($expr instanceof Node\Scalar\Int_ || $expr instanceof Node\Scalar\Float_ || $expr instanceof Node\Scalar\String_) { + return true; + } + + if ($expr instanceof Expr\ConstFetch) { + return true; + } + + if ($expr instanceof Expr\ClassConstFetch) { + return $expr->class instanceof Node\Name && $expr->name instanceof Node\Identifier; + } + + if ($expr instanceof Expr\Array_) { + foreach ($expr->items as $item) { + if ($item->unpack || $item->byRef) { + return false; + } + if ($item->key !== null && !$this->isPure($item->key, $scope)) { + return false; + } + if (!$this->isPure($item->value, $scope)) { + return false; + } + } + + return true; + } + + if ( + $expr instanceof Expr\BinaryOp\Identical + || $expr instanceof Expr\BinaryOp\NotIdentical + || $expr instanceof Expr\BinaryOp\BooleanAnd + || $expr instanceof Expr\BinaryOp\BooleanOr + || $expr instanceof Expr\BinaryOp\LogicalAnd + || $expr instanceof Expr\BinaryOp\LogicalOr + ) { + return $this->isPure($expr->left, $scope) && $this->isPure($expr->right, $scope); + } + + if ($expr instanceof Expr\BooleanNot) { + return $this->isPure($expr->expr, $scope); + } + + if ($expr instanceof Expr\Instanceof_) { + return $expr->class instanceof Node\Name && $this->isPure($expr->expr, $scope); + } + + if ($expr instanceof Expr\FuncCall) { + if (!$expr->name instanceof Node\Name || $expr->isFirstClassCallable()) { + return false; + } + if (!$this->reflectionProvider->hasFunction($expr->name, $scope)) { + return false; + } + if (!$this->reflectionProvider->getFunction($expr->name, $scope)->hasSideEffects()->no()) { + return false; + } + foreach ($expr->getArgs() as $arg) { + if ($arg->unpack || $arg->byRef || !$this->isPure($arg->value, $scope)) { + return false; + } + } + + return true; + } + + return false; + } + +} diff --git a/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php b/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php new file mode 100644 index 00000000000..1a45beaecb0 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php @@ -0,0 +1,166 @@ + + */ +class UnnecessaryTernaryOperatorRuleTest extends RuleTestCase +{ + + private bool $unnecessaryTernary = true; + + private bool $treatPhpDocTypesAsCertain = true; + + protected function getRule(): Rule + { + // @phpstan-ignore argument.type + return new CompositeRule([ + new UnnecessaryTernaryOperatorRule( + self::getContainer()->getByType(ConstantConditionRuleHelper::class), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + self::createReflectionProvider(), + self::getContainer()->getByType(ExprPrinter::class), + $this->unnecessaryTernary, + $this->treatPhpDocTypesAsCertain, + true, + ), + new ConstantConditionInTraitRule(), + ]); + } + + protected function shouldTreatPhpDocTypesAsCertain(): bool + { + return $this->treatPhpDocTypesAsCertain; + } + + #[RequiresPhp('>= 8.1')] + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/unnecessary-ternary.php'], [ + [ + 'Ternary operator is unnecessary because it always results in $i.', + 21, + ], + [ + 'Ternary operator is unnecessary because it always results in $i.', + 31, + ], + [ + 'Ternary operator is unnecessary because it always results in $i.', + 36, + ], + [ + 'Ternary operator is unnecessary because it always results in $foo.', + 41, + ], + [ + 'Ternary operator is unnecessary because it always results in $i.', + 46, + ], + [ + 'Ternary operator is unnecessary because it always results in $foo.', + 54, + 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $foo.', + 59, + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $b.', + 64, + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $a.', + 73, + ], + [ + 'Ternary operator is unnecessary because it always results in $s.', + 78, + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $foo.', + 151, + ], + [ + 'Ternary operator is unnecessary because it always results in $s.', + 190, + ], + [ + 'Ternary operator is unnecessary because it always results in $foo.', + 195, + ], + ]); + } + + #[RequiresPhp('>= 8.1')] + public function testRuleWithoutTreatingPhpDocTypesAsCertain(): void + { + $this->treatPhpDocTypesAsCertain = false; + $this->analyse([__DIR__ . '/data/unnecessary-ternary.php'], [ + [ + 'Ternary operator is unnecessary because it always results in $i.', + 21, + ], + [ + 'Ternary operator is unnecessary because it always results in $i.', + 31, + ], + [ + 'Ternary operator is unnecessary because it always results in $i.', + 36, + ], + [ + 'Ternary operator is unnecessary because it always results in $foo.', + 41, + ], + [ + 'Ternary operator is unnecessary because it always results in $i.', + 46, + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $foo.', + 59, + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $b.', + 64, + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $a.', + 73, + ], + [ + 'Ternary operator is unnecessary because it always results in $s.', + 78, + ], + [ + 'Short ternary operator ?: is unnecessary because it always results in $foo.', + 151, + ], + [ + 'Ternary operator is unnecessary because it always results in $s.', + 190, + ], + [ + 'Ternary operator is unnecessary because it always results in $foo.', + 195, + ], + ]); + } + + #[RequiresPhp('>= 8.1')] + public function testRuleDisabled(): void + { + $this->unnecessaryTernary = false; + $this->analyse([__DIR__ . '/data/unnecessary-ternary.php'], []); + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php b/tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php new file mode 100644 index 00000000000..2307a0a3e11 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php @@ -0,0 +1,221 @@ += 8.1 + +namespace UnnecessaryTernary; + +class Foo +{ + +} + +enum Suit +{ + case Hearts; + case Spades; +} + +class HelloWorld +{ + + public function getIntOrNull(?int $i): ?int + { + return $i !== null ? $i : null; + } + + public function getIntOrNullCoalesce(?int $i): ?int + { + return $i ?? null; + } + + public function identical(?int $i): ?int + { + return $i === null ? null : $i; + } + + public function nullFirst(?int $i): ?int + { + return null !== $i ? $i : null; + } + + public function instanceofCheck(?Foo $foo): ?Foo + { + return $foo instanceof Foo ? $foo : null; + } + + public function isInt(?int $i): ?int + { + return is_int($i) ? $i : null; + } + + /** + * @param Foo|null $foo + */ + public function phpDoc($foo): ?Foo + { + return $foo instanceof Foo ? $foo : null; + } + + public function shortObject(?Foo $foo): ?Foo + { + return $foo ?: null; + } + + public function shortBool(bool $b): bool + { + return $b ?: false; + } + + /** + * @param array $a + * @return array + */ + public function shortArray(array $a): array + { + return $a ?: []; + } + + public function enumCase(Suit $s): Suit + { + return $s !== Suit::Hearts ? $s : Suit::Hearts; + } + + public function shortInt(?int $i): ?int + { + // falsy $i is 0|null + return $i ?: null; + } + + public function shortString(string $s): string + { + // falsy $s is ''|'0' + return $s ?: ''; + } + + public function otherValue(?int $i): ?int + { + return $i !== null ? $i : 0; + } + + public function float(float $f): float + { + return $f !== 0.0 ? $f : 0.0; + } + + public function shortFloat(?float $f): ?float + { + return $f ?: null; + } + + public function impureCondition(): ?int + { + return $this->next() !== null ? $this->next() : null; + } + + public function impureElse(?Foo $foo): ?Foo + { + return $foo ?: $this->nothing(); + } + + public function otherVariable(?int $i, ?int $j): ?int + { + return $j !== null ? $i : null; + } + + public function next(): ?int + { + return rand(0, 1) === 1 ? 1 : null; + } + + public function nothing(): ?Foo + { + echo 'x'; + return null; + } + +} + +function strlenOfFalse(string|false $blob): int +{ + return $blob === false ? 0 : strlen($blob); +} + +function strlenOfString(string|false $blob): int +{ + return $blob !== false ? strlen($blob) : 0; +} + +trait FooTrait +{ + + public function shortObject(?Foo $foo): ?Foo + { + return $foo ?: null; + } + +} + +class UsesFooTrait +{ + + use FooTrait; + +} + +class Issets +{ + + public function maybeUndefined(): ?int + { + if (rand(0, 1)) { + $a = 1; + } + + return isset($a) ? $a : null; + } + + public function property(Bar $bar): ?int + { + return $bar->p !== null ? $bar->p : null; + } + + public function sameVariables(?int $a, ?int $b): ?int + { + return $a !== $b ? $a : $b; + } + + /** + * @param 'a'|'b' $s + */ + public function literal(string $s): string + { + return $s === 'a' ? 'a' : $s; + } + + public function booleanNot(?Foo $foo): ?Foo + { + return !$foo ? null : $foo; + } + +} + +class Bar +{ + + public ?int $p = null; + +} + +class FalsyObjects +{ + + public function simpleXml(?\SimpleXMLElement $xml): ?\SimpleXMLElement + { + // an empty SimpleXMLElement is falsy + return $xml ?: null; + } + + public function simpleXmlNot(?\SimpleXMLElement $xml): ?\SimpleXMLElement + { + return !$xml ? null : $xml; + } + +} From a8d0c59224592406230a09d1bf4ece18b6c836d6 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Sat, 31 Oct 2026 20:45:34 +0100 Subject: [PATCH 2/3] Use a full version in the RequiresPhp attributes Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php b/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php index 1a45beaecb0..3bfcb595120 100644 --- a/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php @@ -40,7 +40,7 @@ protected function shouldTreatPhpDocTypesAsCertain(): bool return $this->treatPhpDocTypesAsCertain; } - #[RequiresPhp('>= 8.1')] + #[RequiresPhp('>= 8.1.0')] public function testRule(): void { $this->analyse([__DIR__ . '/data/unnecessary-ternary.php'], [ @@ -100,7 +100,7 @@ public function testRule(): void ]); } - #[RequiresPhp('>= 8.1')] + #[RequiresPhp('>= 8.1.0')] public function testRuleWithoutTreatingPhpDocTypesAsCertain(): void { $this->treatPhpDocTypesAsCertain = false; @@ -156,7 +156,7 @@ public function testRuleWithoutTreatingPhpDocTypesAsCertain(): void ]); } - #[RequiresPhp('>= 8.1')] + #[RequiresPhp('>= 8.1.0')] public function testRuleDisabled(): void { $this->unnecessaryTernary = false; From 453d4035ba800eb12852821b4a8492182b506ce0 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Sat, 31 Oct 2026 21:03:28 +0100 Subject: [PATCH 3/3] Test the constant condition and side effect checks of the ternary rule The constant condition and float checks now read the constant values, and the object check uses isObject(), so a mutant of them changes the result. New test cases cover a constant condition and a function that can have side effects. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../UnnecessaryTernaryOperatorRule.php | 16 +++++++++---- .../UnnecessaryTernaryOperatorRuleTest.php | 12 +++++----- .../Comparison/data/unnecessary-ternary.php | 23 +++++++++++++++++++ 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php b/src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php index f16b831c052..6f2a4e69a1f 100644 --- a/src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php +++ b/src/Rules/Comparison/UnnecessaryTernaryOperatorRule.php @@ -20,6 +20,7 @@ use PHPStan\Type\ObjectWithoutClassType; use PHPStan\Type\TypeCombinator; use function count; +use function is_float; use function is_string; use function sprintf; @@ -77,7 +78,7 @@ private function check(Node\Expr\Ternary $node, Scope $scope): ?IdentifierRuleEr { // TernaryOperatorConstantConditionRule already reports these $conditionType = $this->helper->getBooleanType($scope, $node->cond); - if ($conditionType->isTrue()->yes() || $conditionType->isFalse()->yes()) { + if (count($conditionType->getConstantScalarValues()) === 1) { return null; } @@ -173,7 +174,13 @@ private function isAlwaysResult(Scope $branchScope, Expr $result, Scope $scope, } // 0.0 === -0.0, so `$f ?: 0.0` turns -0.0 into 0.0 - return $otherType->isFloat()->no(); + foreach ($otherType->getConstantScalarValues() as $value) { + if (is_float($value)) { + return false; + } + } + + return true; } /** @@ -197,12 +204,11 @@ private function isTruthinessNarrowedExactly(Expr $cond, Scope $scope, bool $nat } $type = $native ? $scope->getNativeType($cond) : $scope->getType($cond); - $objectType = new ObjectWithoutClassType(); - if ($objectType->isSuperTypeOf($type)->no()) { + if ($type->isObject()->no()) { return true; } - return TypeCombinator::intersect($type, $objectType)->toBoolean()->isTrue()->yes(); + return TypeCombinator::intersect($type, new ObjectWithoutClassType())->toBoolean()->isTrue()->yes(); } private function containsCall(Expr $expr): bool diff --git a/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php b/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php index 3bfcb595120..45f16eb2654 100644 --- a/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/UnnecessaryTernaryOperatorRuleTest.php @@ -87,15 +87,15 @@ public function testRule(): void ], [ 'Short ternary operator ?: is unnecessary because it always results in $foo.', - 151, + 162, ], [ 'Ternary operator is unnecessary because it always results in $s.', - 190, + 201, ], [ 'Ternary operator is unnecessary because it always results in $foo.', - 195, + 206, ], ]); } @@ -143,15 +143,15 @@ public function testRuleWithoutTreatingPhpDocTypesAsCertain(): void ], [ 'Short ternary operator ?: is unnecessary because it always results in $foo.', - 151, + 162, ], [ 'Ternary operator is unnecessary because it always results in $s.', - 190, + 201, ], [ 'Ternary operator is unnecessary because it always results in $foo.', - 195, + 206, ], ]); } diff --git a/tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php b/tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php index 2307a0a3e11..6734681f76b 100644 --- a/tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php +++ b/tests/PHPStan/Rules/Comparison/data/unnecessary-ternary.php @@ -143,6 +143,17 @@ function strlenOfString(string|false $blob): int return $blob !== false ? strlen($blob) : 0; } +/** @return null */ +function maybeSideEffects() +{ + return null; +} + +function callWithMaybeSideEffects(?Foo $foo): ?Foo +{ + return $foo ?: maybeSideEffects(); +} + trait FooTrait { @@ -219,3 +230,15 @@ public function simpleXmlNot(?\SimpleXMLElement $xml): ?\SimpleXMLElement } } + +class ConstantCondition +{ + + public function alwaysFalse(): int + { + // TernaryOperatorConstantConditionRule reports this + $i = 0; + return $i ?: 0; + } + +}