Support WeakReferences in NodeConnectingVisitor (nikic#1057)

Add a constructor argument which enables the use of WeakReferences.
The attributes are called weak_parent, weak_next and weak_previous
in that case.

Author: staabm

Makefile

@@ -8,3 +8,6 @@ phpstan: tools/vendor
 
 php-cs-fixer: tools/vendor
 	php tools/vendor/bin/php-cs-fixer fix
+
+tests:
+	php vendor/bin/phpunit

doc/component/FAQ.markdown

@@ -51,3 +51,17 @@ obtained through `$node->getAttribute('next')`.
 
 `ParentConnectingVisitor` and `NodeConnectingVisitor` should not be used at the same time. The latter
 includes the functionality of the former.
+
+
+How can I limit the impact of cyclic references in the AST?
+-----
+
+NodeConnectingVisitor adds a parent reference, which introduces a cycle. This means that the AST can now only be collected by the cycle garbage collector.
+This in turn can lead to performance and/or memory issues. 
+
+To break the cyclic references between AST nodes `NodeConnectingVisitor` supports a boolean `$weakReferences` constructor parameter.
+When set to `true`, all attributes added by `NodeConnectingVisitor` will be wrapped into a `WeakReference` object.
+
+After enabling this parameter, the parent node can be obtained through `$node->getAttribute('weak_parent')`,
+the previous node can be obtained through `$node->getAttribute('weak_previous')`, and the next node can be
+obtained through `$node->getAttribute('weak_next')`.

lib/PhpParser/NodeVisitor/NodeConnectingVisitor.php

@@ -9,10 +9,12 @@
  * Visitor that connects a child node to its parent node
  * as well as its sibling nodes.
  *
- * On the child node, the parent node can be accessed through
+ * With <code>$weakReferences=false</code> on the child node, the parent node can be accessed through
  * <code>$node->getAttribute('parent')</code>, the previous
  * node can be accessed through <code>$node->getAttribute('previous')</code>,
  * and the next node can be accessed through <code>$node->getAttribute('next')</code>.
+ *
+ * With <code>$weakReferences=true</code> attribute names are prefixed by "weak_", e.g. "weak_parent".
  */
 final class NodeConnectingVisitor extends NodeVisitorAbstract {
     /**
@@ -25,19 +27,39 @@ final class NodeConnectingVisitor extends NodeVisitorAbstract {
      */
     private $previous;
 
+    private bool $weakReferences;
+
+    public function __construct(bool $weakReferences = false) {
+        $this->weakReferences = $weakReferences;
+    }
+
     public function beforeTraverse(array $nodes) {
         $this->stack    = [];
         $this->previous = null;
     }
 
     public function enterNode(Node $node) {
         if (!empty($this->stack)) {
-            $node->setAttribute('parent', $this->stack[count($this->stack) - 1]);
+            $parent = $this->stack[count($this->stack) - 1];
+            if ($this->weakReferences) {
+                $node->setAttribute('weak_parent', \WeakReference::create($parent));
+            } else {
+                $node->setAttribute('parent', $parent);
+            }
         }
 
-        if ($this->previous !== null && $this->previous->getAttribute('parent') === $node->getAttribute('parent')) {
-            $node->setAttribute('previous', $this->previous);
-            $this->previous->setAttribute('next', $node);
+        if ($this->previous !== null) {
+            if (
+                $this->weakReferences
+            ) {
+                if ($this->previous->getAttribute('weak_parent') === $node->getAttribute('weak_parent')) {
+                    $node->setAttribute('weak_previous', \WeakReference::create($this->previous));
+                    $this->previous->setAttribute('weak_next', \WeakReference::create($node));
+                }
+            } elseif ($this->previous->getAttribute('parent') === $node->getAttribute('parent')) {
+                $node->setAttribute('previous', $this->previous);
+                $this->previous->setAttribute('next', $node);
+            }
         }
 
         $this->stack[] = $node;

lib/PhpParser/NodeVisitor/ParentConnectingVisitor.php

@@ -11,22 +11,35 @@
 /**
  * Visitor that connects a child node to its parent node.
  *
- * On the child node, the parent node can be accessed through
+ * With <code>$weakReferences=false</code> on the child node, the parent node can be accessed through
  * <code>$node->getAttribute('parent')</code>.
+ *
+ * With <code>$weakReferences=true</code> the attribute name is "weak_parent" instead.
  */
 final class ParentConnectingVisitor extends NodeVisitorAbstract {
     /**
      * @var Node[]
      */
     private array $stack = [];
 
+    private bool $weakReferences;
+
+    public function __construct(bool $weakReferences = false) {
+        $this->weakReferences = $weakReferences;
+    }
+
     public function beforeTraverse(array $nodes) {
         $this->stack = [];
     }
 
     public function enterNode(Node $node) {
         if (!empty($this->stack)) {
-            $node->setAttribute('parent', $this->stack[count($this->stack) - 1]);
+            $parent = $this->stack[count($this->stack) - 1];
+            if ($this->weakReferences) {
+                $node->setAttribute('weak_parent', \WeakReference::create($parent));
+            } else {
+                $node->setAttribute('parent', $parent);
+            }
         }
 
         $this->stack[] = $node;

test/PhpParser/NodeVisitor/NodeConnectingVisitorTest.php

@@ -30,4 +30,28 @@ public function testConnectsNodeToItsParentNodeAndItsSiblingNodes(): void {
 
         $this->assertSame(Else_::class, get_class($node->getAttribute('next')));
     }
+
+    public function testWeakReferences(): void {
+        $ast = (new ParserFactory())->createForNewestSupportedVersion()->parse(
+            '<?php if (true) {} else {}'
+        );
+
+        $traverser = new NodeTraverser();
+
+        $traverser->addVisitor(new NodeConnectingVisitor(true));
+
+        $ast = $traverser->traverse($ast);
+
+        $node = (new NodeFinder())->findFirstInstanceof($ast, Else_::class);
+
+        $this->assertInstanceOf(\WeakReference::class, $node->getAttribute('weak_parent'));
+        $this->assertSame(If_::class, get_class($node->getAttribute('weak_parent')->get()));
+        $this->assertInstanceOf(\WeakReference::class, $node->getAttribute('weak_previous'));
+        $this->assertSame(ConstFetch::class, get_class($node->getAttribute('weak_previous')->get()));
+
+        $node = (new NodeFinder())->findFirstInstanceof($ast, ConstFetch::class);
+
+        $this->assertInstanceOf(\WeakReference::class, $node->getAttribute('weak_next'));
+        $this->assertSame(Else_::class, get_class($node->getAttribute('weak_next')->get()));
+    }
 }

test/PhpParser/NodeVisitor/ParentConnectingVisitorTest.php

@@ -23,4 +23,22 @@ public function testConnectsChildNodeToParentNode(): void {
 
         $this->assertSame('C', $node->getAttribute('parent')->name->toString());
     }
+
+    public function testWeakReferences(): void {
+        $ast = (new ParserFactory())->createForNewestSupportedVersion()->parse(
+            '<?php class C { public function m() {} }'
+        );
+
+        $traverser = new NodeTraverser();
+
+        $traverser->addVisitor(new ParentConnectingVisitor(true));
+
+        $ast = $traverser->traverse($ast);
+
+        $node = (new NodeFinder())->findFirstInstanceof($ast, ClassMethod::class);
+
+        $weakReference = $node->getAttribute('weak_parent');
+        $this->assertInstanceOf(\WeakReference::class, $weakReference);
+        $this->assertSame('C', $weakReference->get()->name->toString());
+    }
 }