[BUGFIX] Ensure correct ViewHelperNode in ViewHelper

Fluid have been designed without DependencyInjection
and therefore shared classes, for example ViewHelper,
in mind.

Using systems can implement custom ViewHelperResolvers
to determine how ViewHelper are resolved and created,
using custom configuration. For example, the TYPO3 CMS
implements such a custom ViewHelperResolver providing
the ability to instanciate them throug Symfony DI.

This allows shared ViewHelper instances now and may
become more likely, especial for custom ViewHelper
using the `CompileWithRenderStatic` trait.

ViewHelper and ViewHelperNodes are connected through
cycling references. This has not been a problem until
the execution code for `convert()` have been moved
from ViewHelperNode to the AbstractViewHelper (#787)
to avoid creating unnessesary children codes, which may
be duplicated and not needed. However, that enforced
the code to work with the `ViewHelperNode` set in the
`ViewHelper`.

Using DI and shared `ViewHelper` now leads to the state,
that the wrong `ViewHelperNode` instance is referenced
in the shared `ViewHelper`. That compiled template code
retrieving the wrong argument in some circumstances, for
example in a for-loop when the viewhelper are additionaly
used after the for-loop.

Moving the `convert()` code from the node to the ViewHelper
is valid, but `shared ViewHelper instances` are valid too.

This change now updates the ViewHelperNode reference in
the ViewHelper before dispatching to the ViewHelper.
We do not have any evidence yet, but the same thing is
done in the `ViewHelperNode->evaluate()` method.

This is baked by a regression test. A cross check is
added, but skipped for now as it makes the test green
even without the fix. That points to another issue,
test setup related or other, and have to be tackled
in a dedicated change. The skipped test is added to
the phpstan baseline.

Used command(s):

```shell
composer phpstan:generate-baseline
```

Resolves #804
Related: #787 1f1dc5a

phpstan-baseline.neon

@@ -114,3 +114,8 @@ parameters:
 			message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertSame\\(\\) with TYPO3Fluid\\\\Fluid\\\\Tests\\\\Functional\\\\Fixtures\\\\Various\\\\UserWithoutToString and string will always evaluate to false\\.$#"
 			count: 1
 			path: tests/Functional/ViewHelpers/Format/HtmlspecialcharsViewHelperTest.php
+
+		-
+			message: "#^Unreachable statement \\- code above always terminates\\.$#"
+			count: 1
+			path: tests/Functional/ViewHelpers/StaticCacheable/NotSharedStaticCompilableViewHelperTest.php

src/Core/Parser/SyntaxTree/ViewHelperNode.php

@@ -122,18 +122,42 @@ public function addChildNode(NodeInterface $childNode)
      * If the view helper implements \TYPO3Fluid\Fluid\Core\ViewHelper\ChildNodeAccessInterface,
      * it calls setChildNodes(array childNodes) on the view helper.
      *
-     * Afterwards, checks that the view helper did not leave a variable lying around.
+     * Afterward, checks that the view helper did not leave a variable lying around.
      *
      * @param RenderingContextInterface $renderingContext
      * @return string evaluated node after the view helper has been called.
      */
     public function evaluate(RenderingContextInterface $renderingContext)
     {
+        // This is added as a safe-off, currently no evidence that we need this here like in convert().
+        // See: https://github.com/TYPO3/Fluid/issues/804
+        $this->updateViewHelperNodeInViewHelper();
         return $renderingContext->getViewHelperInvoker()->invoke($this->uninitializedViewHelper, $this->arguments, $renderingContext);
     }
 
     public function convert(TemplateCompiler $templateCompiler): array
     {
+        // We need this here to avoid https://github.com/TYPO3/Fluid/issues/804.
+        $this->updateViewHelperNodeInViewHelper();
         return $this->uninitializedViewHelper->convert($templateCompiler);
     }
+
+    /**
+     * Ensure correct ViewHelperNode (this) reference in the uninitialized ViewHelper instance.
+     */
+    protected function updateViewHelperNodeInViewHelper(): void
+    {
+        // Custom ViewHelperResolver can and are implemented providing the ability to instantiate ViewHelpers through
+        // a DependencyInjection system like Symfony DI, for example done by TYPO3. Due to the nature, instances may be
+        // set as shared, which means that changes to property reflects the latest set state. Therefore, we need to set
+        // the current ViewHelperNode to a viewhelper instance to ensure correct context.
+        // See https://github.com/TYPO3/Fluid/issues/804
+        // @todo We should evaluate if we can get rid of this state and better pass it around.
+        // @todo The ViewHelperInterface does not contain the setViewHelperNode() method. Most likely ViewHelper are
+        //       created using the AbstractViewHelper class as base, which contains this method. However, we need
+        //       to check for method to exists before calling it.
+        if (method_exists($this->uninitializedViewHelper, 'setViewHelperNode')) {
+            $this->uninitializedViewHelper->setViewHelperNode($this);
+        }
+    }
 }

tests/Functional/ViewHelpers/StaticCacheable/Fixtures/ExpectedOutput.html

@@ -0,0 +1,15 @@
+<ul class="pagination-test">
+    <li class="page-item">1- 1</li>
+
+    <li class="page-item">1 - 1</li>
+
+    <li class="page-item">2 - 2</li>
+
+    <li class="page-item">3 - 3</li>
+
+    <li class="page-item">4 - 4</li>
+
+    <li class="page-item">5 - 5</li>
+
+    <li class="page-item">3 - 3</li>
+</ul>

tests/Functional/ViewHelpers/StaticCacheable/Fixtures/Partials/Result/Pagination.html

@@ -0,0 +1,7 @@
+<ul class="pagination-test">
+    <li class="page-item">{s:compilable(page: previousPageNumber)}- {previousPageNumber}</li>
+<f:for each="{allPageNumbers}" as="page">
+    <li class="page-item">{s:compilable(page: '{page}')} - {page}</li>
+</f:for>
+    <li class="page-item">{s:compilable(page: '{nextPageNumber}')} - {nextPageNumber}</li>
+</ul>

tests/Functional/ViewHelpers/StaticCacheable/Fixtures/Templates/Results.html

@@ -0,0 +1 @@
+<f:render partial="Result/Pagination" arguments="{allPageNumbers: '{1:1, 2:2, 3:3, 4:4, 5:5}', previousPageNumber: 1, nextPageNumber: 3}"/>

tests/Functional/ViewHelpers/StaticCacheable/Fixtures/ViewHelpers/CompilableViewHelper.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file belongs to the package "TYPO3 Fluid".
+ * See LICENSE.txt that was shipped with this package.
+ */
+
+namespace TYPO3Fluid\Fluid\Tests\Functional\ViewHelpers\StaticCacheable\Fixtures\ViewHelpers;
+
+use TYPO3Fluid\Fluid\Core\Rendering\RenderingContextInterface;
+use TYPO3Fluid\Fluid\Core\ViewHelper\AbstractViewHelper;
+use TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithRenderStatic;
+
+final class CompilableViewHelper extends AbstractViewHelper
+{
+    use CompileWithRenderStatic;
+
+    /**
+     * @inheritdoc
+     */
+    public function initializeArguments(): void
+    {
+        parent::initializeArguments();
+        $this->registerArgument('page', 'int', 'The page', false);
+    }
+
+    public static function renderStatic(
+        array $arguments,
+        \Closure $renderChildrenClosure,
+        RenderingContextInterface $renderingContext,
+    ) {
+        $page = $arguments['page'] ?? null;
+        return (string)$page;
+    }
+}

tests/Functional/ViewHelpers/StaticCacheable/NotSharedStaticCompilableViewHelperTest.php

@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file belongs to the package "TYPO3 Fluid".
+ * See LICENSE.txt that was shipped with this package.
+ */
+
+namespace TYPO3Fluid\Fluid\Tests\Functional\ViewHelpers\StaticCacheable;
+
+use TYPO3Fluid\Fluid\Core\ViewHelper\ViewHelperResolver;
+use TYPO3Fluid\Fluid\Tests\Functional\AbstractFunctionalTestCase;
+use TYPO3Fluid\Fluid\View\TemplateView;
+
+/**
+ * Regression test for https://github.com/TYPO3/Fluid/issues/804.
+ */
+final class NotSharedStaticCompilableViewHelperTest extends AbstractFunctionalTestCase
+{
+    /**
+     * @test
+     */
+    public function renderWithNotSharedCompilableViewHelper(): void
+    {
+        // @todo If the test with the mocked ViewHelperResolver is executed standalone, it fails and shows the broken
+        //       state. As soon as it is run next to other tests, it succeeds - which is currently wrong. We need to
+        //       tackle this first, so we can have proper test coverage for this regression - and before covering or
+        //       fixing the regression. If the skip is removed, the other test is green (which should be red right now).
+        // Note: To investigate this, the method code in ViewHelperNode->updateViewHelperNodeInViewHelper() should be
+        //       commented out and full test suite run vs the single concrete testcase to see if the single-failure vs
+        //       succeed with full suite can be solved.
+        //       Single TestCase: tests/Functional/ViewHelpers/StaticCacheable/SharedStaticCompilableViewHelperTest.php
+        self::markTestSkipped('Interfering with StaticCompilableViewHelperTest::renderWithSharedCompilableViewHelper');
+
+        $template = __DIR__ . '/Fixtures/Templates/Results.html';
+        $expectedMarkup = trim(file_get_contents(__DIR__ . '/Fixtures/ExpectedOutput.html'));
+
+        $view = new TemplateView();
+        $view->assignMultiple([]);
+        $view->getRenderingContext()->setCache(self::$cache);
+        $view->getRenderingContext()->getViewHelperResolver()->addNamespace('s', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\ViewHelpers\\StaticCacheable\\Fixtures\\ViewHelpers');
+        $view->getRenderingContext()->getTemplatePaths()->setPartialRootPaths([__DIR__ . '/Fixtures/Partials/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplateRootPaths([__DIR__ . '/Fixtures/Templates/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplatePathAndFilename($template);
+        self::assertSame($expectedMarkup, trim($view->render()), 'Uncached (#1) run returned unexpected output');
+
+        $view = new TemplateView();
+        $view->assignMultiple([]);
+        $view->getRenderingContext()->setCache(self::$cache);
+        $view->getRenderingContext()->getViewHelperResolver()->addNamespace('s', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\ViewHelpers\\StaticCacheable\\Fixtures\\ViewHelpers');
+        $view->getRenderingContext()->getTemplatePaths()->setPartialRootPaths([__DIR__ . '/Fixtures/Partials/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplateRootPaths([__DIR__ . '/Fixtures/Templates/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplatePathAndFilename($template);
+        self::assertSame($expectedMarkup, trim($view->render()), 'Cached (#2) run returned unexpected output');
+    }
+}

tests/Functional/ViewHelpers/StaticCacheable/SharedStaticCompilableViewHelperTest.php

@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file belongs to the package "TYPO3 Fluid".
+ * See LICENSE.txt that was shipped with this package.
+ */
+
+namespace TYPO3Fluid\Fluid\Tests\Functional\ViewHelpers\StaticCacheable;
+
+use TYPO3Fluid\Fluid\Core\ViewHelper\ViewHelperResolver;
+use TYPO3Fluid\Fluid\Tests\Functional\AbstractFunctionalTestCase;
+use TYPO3Fluid\Fluid\Tests\Functional\ViewHelpers\StaticCacheable\Fixtures\ViewHelpers\CompilableViewHelper;
+use TYPO3Fluid\Fluid\View\TemplateView;
+
+/**
+ * Regression test for https://github.com/TYPO3/Fluid/issues/804.
+ */
+final class SharedStaticCompilableViewHelperTest extends AbstractFunctionalTestCase
+{
+    /**
+     * @test
+     */
+    public function renderWithSharedCompilableViewHelper(): void
+    {
+        // TYPO3 implements a custom ViewHelperResolver to provide DI-able ViewHelper instances. This allows
+        // developers to use shared ViewHelpers by mark them as `shared` in the configuration. The following
+        // mocked ViewHelperResolver simulates a simplified version of the TYPO3 implementation. We use this
+        // to tests with a reused (shared) ViewHelper. See https://github.com/TYPO3/Fluid/issues/804.
+        // @todo Consider to convert this into a fixture class to test other scenario's with shared ViewHelpers.
+        $viewHelperResolver = new class () extends ViewHelperResolver {
+            public array $sharedInstances = [];
+            protected array $classesFlaggedAsShared = [
+                CompilableViewHelper::class,
+            ];
+            public array $called = [];
+
+            public function createViewHelperInstanceFromClassName($viewHelperClassName)
+            {
+                $this->called[$viewHelperClassName] ??= 0;
+                $this->called[$viewHelperClassName]++;
+
+                if ($this->sharedInstances[$viewHelperClassName] ?? false) {
+                    return $this->sharedInstances[$viewHelperClassName];
+                }
+                if (!in_array($viewHelperClassName, $this->classesFlaggedAsShared, true)) {
+                    return parent::createViewHelperInstanceFromClassName($viewHelperClassName);
+                }
+
+                $this->sharedInstances[$viewHelperClassName] = parent::createViewHelperInstanceFromClassName($viewHelperClassName);
+                return $this->sharedInstances[$viewHelperClassName];
+            }
+        };
+
+        $template = __DIR__ . '/Fixtures/Templates/Results.html';
+        $expectedMarkup = trim(file_get_contents(__DIR__ . '/Fixtures/ExpectedOutput.html'));
+
+        $view = new TemplateView();
+        $view->assignMultiple([]);
+        $view->getRenderingContext()->setCache(self::$cache);
+        $view->getRenderingContext()->setViewHelperResolver($viewHelperResolver);
+        $view->getRenderingContext()->getViewHelperResolver()->addNamespace('s', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\ViewHelpers\\StaticCacheable\\Fixtures\\ViewHelpers');
+        $view->getRenderingContext()->getTemplatePaths()->setPartialRootPaths([__DIR__ . '/Fixtures/Partials/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplateRootPaths([__DIR__ . '/Fixtures/Templates/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplatePathAndFilename($template);
+        self::assertSame($expectedMarkup, trim($view->render()), 'Uncached (#1) run returned unexpected output');
+
+        $view = new TemplateView();
+        $view->assignMultiple([]);
+        $view->getRenderingContext()->setCache(self::$cache);
+        $view->getRenderingContext()->setViewHelperResolver($viewHelperResolver);
+        $view->getRenderingContext()->getViewHelperResolver()->addNamespace('s', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\ViewHelpers\\StaticCacheable\\Fixtures\\ViewHelpers');
+        $view->getRenderingContext()->getTemplatePaths()->setPartialRootPaths([__DIR__ . '/Fixtures/Partials/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplateRootPaths([__DIR__ . '/Fixtures/Templates/']);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplatePathAndFilename($template);
+        self::assertSame($expectedMarkup, trim($view->render()), 'Cached (#2) run returned unexpected output');
+    }
+}