diff --git a/rules-tests/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector/Fixture/skip_short_circuit_dependent_call.php.inc b/rules-tests/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector/Fixture/skip_short_circuit_dependent_call.php.inc new file mode 100644 index 00000000000..20ba892c646 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector/Fixture/skip_short_circuit_dependent_call.php.inc @@ -0,0 +1,23 @@ +getTranslation($locale)) { + return true; + } + + return false; + } +} + +final class SomeTranslations +{ + public function getTranslation(string $locale): ?string + { + return null; + } +} diff --git a/rules-tests/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector/Fixture/yoda_null_on_variables.php.inc b/rules-tests/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector/Fixture/yoda_null_on_variables.php.inc new file mode 100644 index 00000000000..a8fb145c168 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/BooleanAnd/RepeatedAndNotEqualToNotInArrayRector/Fixture/yoda_null_on_variables.php.inc @@ -0,0 +1,35 @@ + +----- + diff --git a/rules-tests/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector/Fixture/skip_short_circuit_dependent_call.php.inc b/rules-tests/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector/Fixture/skip_short_circuit_dependent_call.php.inc new file mode 100644 index 00000000000..1efd4a14ccc --- /dev/null +++ b/rules-tests/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector/Fixture/skip_short_circuit_dependent_call.php.inc @@ -0,0 +1,23 @@ +getTranslation($locale)) { + return true; + } + + return false; + } +} + +final class SomeTranslations +{ + public function getTranslation(string $locale): ?string + { + return null; + } +} diff --git a/rules-tests/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector/Fixture/yoda_null_on_variables.php.inc b/rules-tests/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector/Fixture/yoda_null_on_variables.php.inc new file mode 100644 index 00000000000..ecdaf244961 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/BooleanOr/RepeatedOrEqualToInArrayRector/Fixture/yoda_null_on_variables.php.inc @@ -0,0 +1,35 @@ + +----- + diff --git a/rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php b/rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php index 75d99c556d8..8c275d22a26 100644 --- a/rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php +++ b/rules/CodeQuality/NodeFactory/InArrayFromRepeatedCompareFactory.php @@ -5,7 +5,10 @@ namespace Rector\CodeQuality\NodeFactory; use PhpParser\Node\Arg; +use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Variable; use Rector\CodeQuality\ValueObject\ComparedExprAndValueExpr; +use Rector\NodeAnalyzer\ExprAnalyzer; use Rector\PhpParser\Comparing\NodeComparator; use Rector\PhpParser\Node\NodeFactory; @@ -13,14 +16,15 @@ { public function __construct( private NodeComparator $nodeComparator, - private NodeFactory $nodeFactory + private NodeFactory $nodeFactory, + private ExprAnalyzer $exprAnalyzer ) { } /** * Builds the "$value, [...]" args of an in_array() call from a repeated compare chain, * once all compared expressions are confirmed equal. Returns null when the chain is too - * short or the compared expressions differ. + * short, the compared expressions differ, or a value expression is not safe to evaluate eagerly. * * @param ComparedExprAndValueExpr[] $comparedExprAndValueExprs * @return Arg[]|null @@ -33,7 +37,15 @@ public function createInArrayArgs(array $comparedExprAndValueExprs): ?array $valueExprs = []; foreach ($comparedExprAndValueExprs as $comparedExprAndValueExpr) { - $valueExprs[] = $comparedExprAndValueExpr->getValueExpr(); + $valueExpr = $comparedExprAndValueExpr->getValueExpr(); + + // the array literal evaluates every item up front, while the &&/|| chain stops early, + // e.g. null !== $a && null !== $a->get() would call get() on null + if (! $this->isSafeToEvaluateEagerly($valueExpr)) { + return null; + } + + $valueExprs[] = $valueExpr; } /** @var ComparedExprAndValueExpr $firstComparedExprAndValue */ @@ -53,4 +65,14 @@ public function createInArrayArgs(array $comparedExprAndValueExprs): ?array return $this->nodeFactory->createArgs([$firstComparedExprAndValue->getComparedExpr(), $array]); } + + private function isSafeToEvaluateEagerly(Expr $expr): bool + { + // reading a plain variable has no side effect; $$name could evaluate a call + if ($expr instanceof Variable) { + return is_string($expr->name); + } + + return ! $this->exprAnalyzer->isDynamicExpr($expr); + } }