Skip to content

Fix nested array access evaluation for null coalescing #1175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ydah wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix nested array access evaluation for null coalescing #1175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions lib/PhpParser/ConstExprEvaluator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public function evaluateDirectly(Expr $expr) {
}

/** @return mixed */
private function evaluate(Expr $expr) {
private function evaluate(Expr $expr, bool $issetContext = false) {
if ($expr instanceof Scalar\Int_
|| $expr instanceof Scalar\Float_
|| $expr instanceof Scalar\String_
Expand Down Expand Up @@ -138,6 +138,10 @@ private function evaluate(Expr $expr) {
}

if ($expr instanceof Expr\ArrayDimFetch && null !== $expr->dim) {
if ($issetContext) {
// This needs to be special cased to respect BP_VAR_IS fetch semantics
return $this->evaluate($expr->var, true)[$this->evaluate($expr->dim)] ?? null;
}
return $this->evaluate($expr->var)[$this->evaluate($expr->dim)];
}

Expand Down Expand Up @@ -175,14 +179,6 @@ private function evaluateTernary(Expr\Ternary $expr) {

/** @return mixed */
private function evaluateBinaryOp(Expr\BinaryOp $expr) {
if ($expr instanceof Expr\BinaryOp\Coalesce
&& $expr->left instanceof Expr\ArrayDimFetch
) {
// This needs to be special cased to respect BP_VAR_IS fetch semantics
return $this->evaluate($expr->left->var)[$this->evaluate($expr->left->dim)]
?? $this->evaluate($expr->right);
}

// The evaluate() calls are repeated in each branch, because some of the operators are
// short-circuiting and evaluating the RHS in advance may be illegal in that case
$l = $expr->left;
Expand All @@ -193,7 +189,7 @@ private function evaluateBinaryOp(Expr\BinaryOp $expr) {
case '^': return $this->evaluate($l) ^ $this->evaluate($r);
case '&&': return $this->evaluate($l) && $this->evaluate($r);
case '||': return $this->evaluate($l) || $this->evaluate($r);
case '??': return $this->evaluate($l) ?? $this->evaluate($r);
case '??': return $this->evaluate($l, true) ?? $this->evaluate($r);
case '.': return $this->evaluate($l) . $this->evaluate($r);
case '/': return $this->evaluate($l) / $this->evaluate($r);
case '==': return $this->evaluate($l) == $this->evaluate($r);
Expand Down
22 changes: 22 additions & 0 deletions test/PhpParser/ConstExprEvaluatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public function testEvaluate($exprString, $expected): void {
$expr = $parser->parse('<?php ' . $exprString . ';')[0]->expr;
$evaluator = new ConstExprEvaluator();
$this->assertSame($expected, $evaluator->evaluateDirectly($expr));
$this->assertSame($expected, $evaluator->evaluateSilently($expr));
}

public static function provideTestEvaluate() {
Expand Down Expand Up @@ -41,6 +42,14 @@ public static function provideTestEvaluate() {
['null ?? 42', 42],
['[0][0] ?? 42', 0],
['[][0] ?? 42', 42],
['["a" => 1]["b"]["c"] ?? 2', 2],
['null["a"]["b"] ?? 3', 3],
['["a" => null]["a"]["b"] ?? 3', 3],
['["a" => 1]["a"]["b"] ?? 2', 2],
['[][0][1][2] ?? 42', 42],
['["a" => ["b" => null]]["a"]["b"] ?? 2', 2],
['["a" => ["b" => 0]]["a"]["b"] ?? (1 / 0)', 0],
['[["foo"]][0][0][1] ?? 42', 'o'],
['0b11 & 0b10', 0b10],
['0b11 | 0b10', 0b11],
['0b11 ^ 0b10', 0b01],
Expand Down Expand Up @@ -81,6 +90,19 @@ public function testEvaluateFails(): void {
$evaluator->evaluateDirectly(new Expr\Variable('a'));
}

public function testCoalesceEvaluatesOffsets(): void {
$parser = (new ParserFactory())->createForNewestSupportedVersion();
$expr = $parser->parse('<?php null["a"][1 % 0] ?? 3;')[0]->expr;
$evaluator = new ConstExprEvaluator();
try {
$evaluator->evaluateSilently($expr);
$this->fail('Expected an exception from the array offset expression');
} catch (ConstExprEvaluationException $e) {
$this->assertInstanceOf(\Error::class, $e->getPrevious());
$this->assertSame('Modulo by zero', $e->getPrevious()->getMessage());
}
}

public function testEvaluateFallbackMagicConst(): void {
$evaluator = new ConstExprEvaluator(function (Expr $expr) {
if ($expr instanceof Scalar\MagicConst\Line) {
Expand Down