minor #4908 Remove the documentation comments compilation overhead (fabpot)

This PR was squashed before being merged into the 3.x branch.

Discussion
----------

Remove the documentation comments compilation overhead

Documentation comments are now attached to semantic nodes during parsing, removing the dedicated AST traversal. Token parsers returning placeholder nodes can select the semantic documentation target.

Commits
-------

60eed4ccd5 Remove the documentation comments compilation overhead

doc/documentation_comments.rst

@@ -175,13 +175,16 @@ Reading Documentation from Nodes
 
 Node visitors can read documentation with ``Node::getDocumentation()``. Twig
 automatically attaches documentation before a custom tag to the node returned
-by its token parser. To support inline documentation, custom token parsers can
-pass the corresponding tokens to ``NodeDocumentation::add()``.
+by its token parser. If that node is a placeholder, the token parser can call
+``Parser::setDocumentationTarget()`` once while parsing the tag to select the
+semantic node instead. To support inline documentation, custom token parsers
+can pass the corresponding tokens to ``NodeDocumentation::add()``.
 
 The metadata is stored on the semantic node represented by the source:
 
 * output documentation is stored on the ``PrintNode``;
-* tag documentation is stored on the node produced by the tag;
+* tag documentation is stored on the node produced by the tag or the target
+  selected by its token parser;
 * block documentation is stored on the ``BlockNode``;
 * macro documentation is stored on the ``MacroNode``;
 * type documentation is stored on each ``TypeNode``;

src/Extension/CoreExtension.php

@@ -87,7 +87,6 @@ use Twig\Node\Expression\Unary\PosUnary;
 use Twig\Node\Expression\Unary\SpreadUnary;
 use Twig\Node\Node;
 use Twig\NodeVisitor\CorrectnessNodeVisitor;
-use Twig\NodeVisitor\DocumentationNodeVisitor;
 use Twig\Parser;
 use Twig\Sandbox\SecurityNotAllowedMethodError;
 use Twig\Sandbox\SecurityNotAllowedPropertyError;
@@ -341,7 +340,6 @@ final class CoreExtension extends AbstractExtension
     public function getNodeVisitors(): array
     {
         return [
-            new DocumentationNodeVisitor(),
             new CorrectnessNodeVisitor(),
         ];
     }

src/NodeVisitor/DocumentationNodeVisitor.php

@@ -1,75 +0,0 @@
-<?php
-
-/*
- * This file is part of Twig.
- *
- * (c) Fabien Potencier
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Twig\NodeVisitor;
-
-use Twig\Environment;
-use Twig\Node\BlockReferenceNode;
-use Twig\Node\MacroDeclarationNode;
-use Twig\Node\ModuleNode;
-use Twig\Node\Node;
-use Twig\Node\NodeDocumentation;
-
-/**
- * @internal
- */
-final class DocumentationNodeVisitor implements NodeVisitorInterface
-{
-    /** @var list<ModuleNode> */
-    private array $modules = [];
-    /** @var list<array<string, MacroDeclarationNode>> */
-    private array $macroDeclarations = [];
-
-    public function enterNode(Node $node, Environment $env): Node
-    {
-        if ($node instanceof ModuleNode) {
-            $this->modules[] = $node;
-            $this->macroDeclarations[] = [];
-        }
-
-        $module = end($this->modules);
-        if ($node instanceof BlockReferenceNode && $module->getNode('blocks')->hasNode($name = $node->getAttribute('name'))) {
-            NodeDocumentation::move($node, $module->getNode('blocks')->getNode($name)->getNode('0'));
-        }
-
-        if ($node instanceof MacroDeclarationNode && $module->getNode('macros')->hasNode($name = $node->getAttribute('name'))) {
-            $macro = $module->getNode('macros')->getNode($name);
-            $index = array_key_last($this->macroDeclarations);
-            if ($node->getTemplateLine() === $macro->getTemplateLine()) {
-                if (isset($this->macroDeclarations[$index][$name])) {
-                    $this->macroDeclarations[$index][$name]->setDocumentation(null);
-                }
-                $this->macroDeclarations[$index][$name] = $node;
-            } else {
-                $node->setDocumentation(null);
-            }
-        }
-
-        return $node;
-    }
-
-    public function leaveNode(Node $node, Environment $env): Node
-    {
-        if ($node instanceof ModuleNode) {
-            foreach (array_pop($this->macroDeclarations) as $name => $declaration) {
-                NodeDocumentation::move($declaration, $node->getNode('macros')->getNode($name));
-            }
-            array_pop($this->modules);
-        }
-
-        return $node;
-    }
-
-    public function getPriority(): int
-    {
-        return -512;
-    }
-}

src/Parser.php

@@ -50,6 +50,8 @@ class Parser
     private $expressionParser;
     private $blocks;
     private $blockStack;
+    /** @var list<Node|null> */
+    private array $documentationTargets = [];
     private $macros;
     private $importedSymbols;
     private $traits;
@@ -99,6 +101,7 @@ class Parser
         $this->stream = $stream;
         $this->parent = null;
         $this->blocks = [];
+        $this->documentationTargets = [];
         $this->macros = [];
         $this->traits = [];
         $this->blockStack = [];
@@ -229,12 +232,22 @@ class Parser
                     $this->stream->next();
 
                     $subparser->setParser($this);
-                    $node = $subparser->parse($token);
+                    $documentationTargetIndex = \count($this->documentationTargets);
+                    $this->documentationTargets[] = null;
+                    try {
+                        $node = $subparser->parse($token);
+                        $documentationTarget = $this->documentationTargets[$documentationTargetIndex];
+                    } finally {
+                        array_pop($this->documentationTargets);
+                    }
                     if (!$node) {
                         trigger_deprecation('twig/twig', '3.12', 'Returning "null" from "%s" is deprecated and forbidden by "TokenParserInterface".', $subparser::class);
                     } else {
                         $node->setNodeTag($subparser->getTag());
                         NodeDocumentation::add($node, $startToken);
+                        if (null !== $documentationTarget && $node !== $documentationTarget) {
+                            NodeDocumentation::move($node, $documentationTarget);
+                        }
                         $rv[] = $node;
                     }
                     break;
@@ -306,6 +319,18 @@ class Parser
         return isset($this->macros[$name]);
     }
 
+    public function setDocumentationTarget(Node $node): void
+    {
+        if (null === $index = array_key_last($this->documentationTargets)) {
+            throw new \LogicException('A documentation target can only be set while parsing a tag.');
+        }
+        if (null !== $this->documentationTargets[$index]) {
+            throw new \LogicException('The documentation target for a tag can only be set once.');
+        }
+
+        $this->documentationTargets[$index] = $node;
+    }
+
     public function setMacro(string $name, MacroNode $node): void
     {
         if (isset($this->macros[$name])) {
@@ -564,11 +589,6 @@ class Parser
     private function cleanupBodyForChildTemplates(Node $body): Node
     {
         if ($body instanceof BlockReferenceNode) {
-            $name = $body->getAttribute('name');
-            if (isset($this->blocks[$name])) {
-                NodeDocumentation::move($body, $this->blocks[$name]->getNode('0'));
-            }
-
             return new EmptyNode();
         }
         if ($body instanceof TextNode && $body->isBlank()) {
@@ -578,10 +598,6 @@ class Parser
         foreach ($body as $k => $node) {
             if ($node instanceof BlockReferenceNode) {
                 // as it has a parent, the block reference won't be used
-                $name = $node->getAttribute('name');
-                if (isset($this->blocks[$name])) {
-                    NodeDocumentation::move($node, $this->blocks[$name]->getNode('0'));
-                }
                 $body->removeNode($k);
             } elseif ($node instanceof TextNode && $node->isBlank()) {
                 // remove nodes considered as "empty"

src/TokenParser/BlockTokenParser.php

@@ -39,6 +39,7 @@ final class BlockTokenParser extends AbstractTokenParser
         $stream = $this->parser->getStream();
         $name = $stream->expect(Token::NAME_TYPE)->getValue();
         $this->parser->setBlock($name, $block = new BlockNode($name, new EmptyNode(), $lineno));
+        $this->parser->setDocumentationTarget($block);
         $this->parser->pushLocalScope();
         $this->parser->pushBlockStack($name);
 

src/TokenParser/MacroTokenParser.php

@@ -56,7 +56,8 @@ final class MacroTokenParser extends AbstractTokenParser
         $this->parser->popLocalScope();
         $stream->expect(Token::BLOCK_END_TYPE);
 
-        $this->parser->setMacro($name, new MacroNode($name, new BodyNode([$body]), $arguments, $lineno, $variadicName));
+        $this->parser->setMacro($name, $macro = new MacroNode($name, new BodyNode([$body]), $arguments, $lineno, $variadicName));
+        $this->parser->setDocumentationTarget($macro);
 
         return new MacroDeclarationNode($name, $lineno);
     }

tests/ParserTest.php

@@ -41,6 +41,7 @@ use Twig\Node\IfNode;
 use Twig\Node\MacroDeclarationNode;
 use Twig\Node\MacroNode;
 use Twig\Node\Node;
+use Twig\Node\Nodes;
 use Twig\Node\PrintNode;
 use Twig\Node\TextNode;
 use Twig\NodeVisitor\NodeVisitorInterface;
@@ -384,6 +385,18 @@ TWIG, 'index')));
         ], $visitor->documentation);
     }
 
+    public function testCustomTokenParserCanSetDocumentationTarget(): void
+    {
+        $twig = new Environment(new ArrayLoader());
+        $twig->addTokenParser(new DocumentationTargetTokenParser());
+
+        $module = $twig->parse($twig->tokenize(new Source('{## Target documentation #}{% documentation_target %}', 'index')));
+        $node = $module->getNode('body')->getNode('0');
+
+        $this->assertNull($node->getDocumentation());
+        $this->assertSame('Target documentation', $node->getNode('target')->getDocumentation());
+    }
+
     public function testDocumentationIsRemovedFromUnsupportedOptimizedNodes(): void
     {
         $twig = new Environment(new ArrayLoader(), ['autoescape' => false]);
@@ -552,7 +565,24 @@ class DocumentationReadingNodeVisitor implements NodeVisitorInterface
 
     public function getPriority(): int
     {
-        return 0;
+        return -1024;
+    }
+}
+
+class DocumentationTargetTokenParser extends AbstractTokenParser
+{
+    public function parse(Token $token): Node
+    {
+        $this->parser->getStream()->expect(Token::BLOCK_END_TYPE);
+        $target = new EmptyNode($token->getLine());
+        $this->parser->setDocumentationTarget($target);
+
+        return new Nodes(['target' => $target], $token->getLine());
+    }
+
+    public function getTag(): string
+    {
+        return 'documentation_target';
     }
 }