Add replacement sanity check in traverser

This commit is contained in:
Nikita Popov 2018-03-03 13:22:36 +01:00
parent 9c18e3db49
commit 6aba7624ed
2 changed files with 36 additions and 5 deletions

View File

@ -115,6 +115,7 @@ class NodeTraverser implements NodeTraverserInterface
$return = $visitor->enterNode($subNode);
if (null !== $return) {
if ($return instanceof Node) {
$this->ensureReplacementReasonable($subNode, $return);
$subNode = $return;
} elseif (self::DONT_TRAVERSE_CHILDREN === $return) {
$traverseChildren = false;
@ -140,6 +141,7 @@ class NodeTraverser implements NodeTraverserInterface
$return = $visitor->leaveNode($subNode);
if (null !== $return) {
if ($return instanceof Node) {
$this->ensureReplacementReasonable($subNode, $return);
$subNode = $return;
} elseif (self::STOP_TRAVERSAL === $return) {
$this->stopTraversal = true;
@ -179,6 +181,7 @@ class NodeTraverser implements NodeTraverserInterface
$return = $visitor->enterNode($node);
if (null !== $return) {
if ($return instanceof Node) {
$this->ensureReplacementReasonable($node, $return);
$node = $return;
} elseif (self::DONT_TRAVERSE_CHILDREN === $return) {
$traverseChildren = false;
@ -204,6 +207,7 @@ class NodeTraverser implements NodeTraverserInterface
$return = $visitor->leaveNode($node);
if (null !== $return) {
if ($return instanceof Node) {
$this->ensureReplacementReasonable($node, $return);
$node = $return;
} elseif (\is_array($return)) {
$doNodes[] = [$i, $return];
@ -239,4 +243,21 @@ class NodeTraverser implements NodeTraverserInterface
return $nodes;
}
private function ensureReplacementReasonable($old, $new) {
if ($old instanceof Node\Stmt && $new instanceof Node\Expr) {
throw new \LogicException(
"Trying to replace statement ({$old->getType()}) " .
"with expression ({$new->getType()}). Are you missing a " .
"Stmt_Expression wrapper?"
);
}
if ($old instanceof Node\Expr && $new instanceof Node\Stmt) {
throw new \LogicException(
"Trying to replace expression ({$old->getType()}) " .
"with statement ({$new->getType()})"
);
}
}
}

View File

@ -258,7 +258,7 @@ class NodeTraverserTest extends TestCase
$this->expectException(\LogicException::class);
$this->expectExceptionMessage($message);
$stmts = [new Node\Expr\UnaryMinus(new Node\Scalar\LNumber(42))];
$stmts = [new Node\Stmt\Expression(new Node\Scalar\LNumber(42))];
$traverser = new NodeTraverser();
$traverser->addVisitor($visitor);
@ -268,19 +268,19 @@ class NodeTraverserTest extends TestCase
public function provideTestInvalidReturn() {
$visitor1 = $this->getMockBuilder(NodeVisitor::class)->getMock();
$visitor1->expects($this->at(1))->method('enterNode')
->will($this->returnValue('foobar'));
->willReturn('foobar');
$visitor2 = $this->getMockBuilder(NodeVisitor::class)->getMock();
$visitor2->expects($this->at(2))->method('enterNode')
->will($this->returnValue('foobar'));
->willReturn('foobar');
$visitor3 = $this->getMockBuilder(NodeVisitor::class)->getMock();
$visitor3->expects($this->at(3))->method('leaveNode')
->will($this->returnValue('foobar'));
->willReturn('foobar');
$visitor4 = $this->getMockBuilder(NodeVisitor::class)->getMock();
$visitor4->expects($this->at(4))->method('leaveNode')
->will($this->returnValue('foobar'));
->willReturn('foobar');
$visitor5 = $this->getMockBuilder(NodeVisitor::class)->getMock();
$visitor5->expects($this->at(3))->method('leaveNode')
@ -290,6 +290,14 @@ class NodeTraverserTest extends TestCase
$visitor6->expects($this->at(4))->method('leaveNode')
->willReturn(false);
$visitor7 = $this->getMockBuilder(NodeVisitor::class)->getMock();
$visitor7->expects($this->at(1))->method('enterNode')
->willReturn(new Node\Scalar\LNumber(42));
$visitor8 = $this->getMockBuilder(NodeVisitor::class)->getMock();
$visitor8->expects($this->at(2))->method('enterNode')
->willReturn(new Node\Stmt\Return_());
return [
[$visitor1, 'enterNode() returned invalid value of type string'],
[$visitor2, 'enterNode() returned invalid value of type string'],
@ -297,6 +305,8 @@ class NodeTraverserTest extends TestCase
[$visitor4, 'leaveNode() returned invalid value of type string'],
[$visitor5, 'leaveNode() may only return an array if the parent structure is an array'],
[$visitor6, 'bool(false) return from leaveNode() no longer supported. Return NodeTraverser::REMOVE_NODE instead'],
[$visitor7, 'Trying to replace statement (Stmt_Expression) with expression (Scalar_LNumber). Are you missing a Stmt_Expression wrapper?'],
[$visitor8, 'Trying to replace expression (Scalar_LNumber) with statement (Stmt_Return)'],
];
}
}