Skip to content

Clear stale node connections when reconnecting an edited AST - #1180

Open
sergioperezcheco wants to merge 1 commit into
nikic:masterfrom
sergioperezcheco:fix/reconnect-stale-node-links-20261019
Open

sergioperezcheco wants to merge 1 commit into
nikic:masterfrom
sergioperezcheco:fix/reconnect-stale-node-links-20261019

Conversation

@sergioperezcheco

Copy link
Copy Markdown

Reconnecting an edited AST

NodeConnectingVisitor adds the links needed by the current traversal, but does not remove links that no longer have a replacement. Running it again after editing an AST can therefore leave a parent on a node promoted to the root, a previous link on the new first sibling, or a next link on the new last sibling.

For example, this prints false before the change and true afterwards:

$ast = (new PhpParser\ParserFactory())->createForNewestSupportedVersion()
    ->parse('<?php echo 1; echo 2; echo 3;');
$traverser = new PhpParser\NodeTraverser(
    new PhpParser\NodeVisitor\NodeConnectingVisitor()
);
$traverser->traverse($ast);
$ast = [$ast[2], $ast[1], $ast[0]];
$traverser->traverse($ast);
var_dump($ast[0]->getAttribute('previous') === null);

Clear the current reference mode's three connection attributes on entry, then let the existing traversal rebuild them. Other attributes, including source positions and the inactive reference namespace, are preserved. This only updates visited nodes; it does not clean up detached nodes or change the separate-traversal usage described in #697.

The regressions cover reordered siblings, a promoted child, and removed siblings in both strong and weak reference modes.

Verification and cost

With the final tests and original production code, the full suite has six failures. With this change:

  • PHP 8.3.32: full PHPUnit passes, 1,946 tests and 2,835 assertions; complete PHPStan and PHP-CS-Fixer checks pass.
  • PHP 8.0.30: full PHPUnit passes with the existing PHP-8.1-only enum test skipped; 1,946 tests and 2,834 assertions.
  • PHP 8.6.0RC3: full PHPUnit passes.
  • Real parser/traverser checks cover reconnecting an edited tree, cloned nodes, repeated traversals, custom attributes, and retaining the inactive namespace.

There is a performance trade-off: getAttributes() followed by modification and setAttributes() copies the PHP attribute array. In a local PHP 8.3 benchmark of 72,000 parsed nodes, three independent processes per configuration and 13 repeated traversals per process, median visitor time changed from 45.9 to 55.6 ms for strong references and 48.1 to 60.3 ms for weak references. With 32 extra attributes per node, it changed from 58.5 to 78.0 ms and 64.7 to 84.4 ms respectively. These are visitor-only measurements, not whole-application results. The patch keeps the existing node API rather than adding an attribute-removal method.

Native PHP 7.4, coverage, the php-src integration jobs, and the remaining platform/version matrix were not run locally and remain for CI.

This contribution was produced and reviewed by autonomous AI agents; no human review is claimed.

Preserve non-connection attributes and the inactive reference namespace while rebuilding links after AST edits.

Assisted-by: Hermes Agent (Nous Research)
Signed-off-by: sergioperezcheco <checo520@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant