[TASK] Use render() as primary ViewHelper method (#983)

Until now it was best practice to differentiate between object-based
ViewHelpers, using the object method `render()` as their primary render
method, and static ViewHelpers, using the static method `renderStatic()`.
This distinction has been introduced to Fluid mainly for performance reasons:
With earlier PHP versions it was quite expensive performance-wise to create a
new ViewHelper object for each ViewHelper call. However, PHP's performance
characteristics for objects have changed quite a bit with newer versions,
which questions the usefulness of this performance optimization nowadays.

Our performance tests have shown that the performance gains are still existent
today if you compare `renderStatic()` to `render()` ViewHelpers, especially
if no further optimizations are applied to Fluid's core implementation.
However, the impact is quite small and almost not measurable in real-world
scenarios.

In contrast to that, we see meaningful opportunities for the further development
and optimization for Fluid if we streamline and simplify Fluid's APIs. It
improves the developer experience if there's only one correct way to
implement ViewHelpers and if all ViewHelpers use the same API. For Fluid
internally, this opens up new opportunities of optimization and refactoring,
which are currently not feasible to tackle.

Ultimately, we have taken the decision to deprecate `renderStatic()` with Fluid v4.
ViewHelpers should use `render()` as their primary render method from now on.
This patch deprecates both the trait `CompileWithRenderStatic` and the usage
of `renderStatic()` without any trait applied to the ViewHelper (which is probably
only an edge case). Instead, the `ViewHelperInvoker` is called both for uncached
and cached templates to initiate and perform each ViewHelper call.

We will backport the `@deprecated` annotation on `CompileWithRenderStatic` to
Fluid v2.15 to let users know in advance of this change.

`AbstractConditionViewHelper` will be adjusted to this change in a follow-up patch.
There will probably also be further code optimization patches that are made possible
because of this decision, either still in Fluid v4 or later in Fluid v5, when
`renderStatic()` will be removed completely.

Documentation/Changelog/2.x.rst

@@ -9,6 +9,9 @@ Changelog 2.x
 2.15
 ----
 
+* Deprecation: Trait :php:`TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithRenderStatic`
+  has been marked as deprecated. It will log a deprecation level error message when called in
+  Fluid v4. It will be removed in Fluid v5.
 * Deprecation: Trait :php:`TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithContentArgumentAndRenderStatic`
   has been marked as deprecated. It will log a deprecation level error message when called in
   Fluid v4. It will be removed in Fluid v5.

Documentation/Changelog/4.x.rst

@@ -25,8 +25,13 @@ Changelog 4.x
   :php:`TYPO3Fluid\Fluid\View\AbstractTemplateView::RENDERING_LAYOUT`
 * Breaking: Careful addition of method and property type hints throughout the system.
   This should be only mildly breaking and projects should be able to adapt easily.
+* Deprecation: Trait :php:`TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithRenderStatic`
+  now emits a E_USER_DEPRECATED level error.
 * Deprecation: Trait :php:`TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithContentArgumentAndRenderStatic`
   now emits a E_USER_DEPRECATED level error.
+* Deprecation: Static method :php:`renderStatic()` on ViewHelpers that don't use :php:`TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithRenderStatic`
+  or :php:`TYPO3Fluid\Fluid\Core\ViewHelper\Traits\CompileWithContentArgumentAndRenderStatic` now emits a
+  E_USER_DEPRECATED level error.
 * Deprecation: Method :php:`TYPO3Fluid\Fluid\Core\ViewHelper\AbstractViewHelper->overrideArgument()`
   now emits a E_USER_DEPRECATED level error.
 * Deprecation: Calling method :php:`TYPO3Fluid\Fluid\Core\ViewHelper\AbstractTagBasedViewHelper->registerUniversalTagAttributes()`

src/Core/ViewHelper/AbstractViewHelper.php

@@ -258,33 +258,7 @@ public function initializeArgumentsAndRender()
         $this->validateArguments();
         $this->initialize();
 
-        return $this->callRenderMethod();
-    }
-
-    /**
-     * Call the render() method and handle errors.
-     *
-     * @return string the rendered ViewHelper
-     * @throws Exception
-     */
-    protected function callRenderMethod()
-    {
-        if (method_exists($this, 'render')) {
-            return $this->render();
-        }
-        if ((new \ReflectionMethod($this, 'renderStatic'))->getDeclaringClass()->getName() !== AbstractViewHelper::class) {
-            // Method is safe to call - will not recurse through ViewHelperInvoker via the default
-            // implementation of renderStatic() on this class.
-            return static::renderStatic($this->arguments, $this->buildRenderChildrenClosure(), $this->renderingContext);
-        }
-        throw new Exception(
-            sprintf(
-                'ViewHelper class "%s" does not declare a "render()" method and inherits the default "renderStatic". ' .
-                'Executing this ViewHelper would cause infinite recursion - please either implement "render()" or ' .
-                '"renderStatic()" on your ViewHelper class',
-                get_class($this),
-            ),
-        );
+        return $this->render();
     }
 
     /**
@@ -309,8 +283,9 @@ public function renderChildren()
     }
 
     /**
-     * Helper which is mostly needed when calling renderStatic() from within
-     * render().
+     * Creates a closure that renders a view helper's child nodes. It also takes
+     * into account the contentArgumentName, which if defined leads to that argument
+     * being rendered instead.
      *
      * No public API yet.
      *
@@ -491,6 +466,36 @@ public function validateAdditionalArguments(array $arguments)
         }
     }
 
+    /**
+     * Main render method of the ViewHelper. Every modern ViewHelper implementation
+     * must implement this method.
+     *
+     * @todo Remove fallback implementation for renderStatic() and declare as abstract with Fluid v5
+     *
+     * @return mixed
+     */
+    public function render()
+    {
+        if (!method_exists(static::class, 'renderStatic')) {
+            throw new Exception(
+                sprintf(
+                    'ViewHelper class "%s" does not declare a "render()" method. Also, no implementation of "renderStatic"' .
+                    'could be found to use as fallback. Please implement "render()" on your ViewHelper class',
+                    static::class,
+                ),
+            );
+        }
+
+        // This covers the edge case where a ViewHelper implements neither CompileWithRenderStatic nor
+        // CompileWithContentArgumentAndRenderStatic, but still uses renderStatic().
+        trigger_error('renderStatic() has been deprecated and will be removed in Fluid v5.', E_USER_DEPRECATED);
+        return static::renderStatic(
+            $this->arguments,
+            $this->buildRenderChildrenClosure(),
+            $this->renderingContext,
+        );
+    }
+
     /**
      * You only should override this method *when you absolutely know what you
      * are doing*, and really want to influence the generated PHP code during
@@ -528,7 +533,7 @@ public function validateAdditionalArguments(array $arguments)
     public function compile($argumentsName, $closureName, &$initializationPhpCode, ViewHelperNode $node, TemplateCompiler $compiler)
     {
         $execution = sprintf(
-            '%s::renderStatic(%s, %s, $renderingContext)',
+            '$renderingContext->getViewHelperInvoker()->invoke(%s::class, %s, $renderingContext, %s)',
             static::class,
             $argumentsName,
             $closureName,
@@ -551,22 +556,6 @@ public function compile($argumentsName, $closureName, &$initializationPhpCode, V
         return $execution;
     }
 
-    /**
-     * Default implementation of static rendering; useful API method if your ViewHelper
-     * when compiled is able to render itself statically to increase performance. This
-     * default implementation will simply delegate to the ViewHelperInvoker.
-     *
-     * @param array<string, mixed> $arguments
-     * @param \Closure $renderChildrenClosure
-     * @param RenderingContextInterface $renderingContext
-     * @return mixed
-     */
-    public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext)
-    {
-        $viewHelperClassName = get_called_class();
-        return $renderingContext->getViewHelperInvoker()->invoke($viewHelperClassName, $arguments, $renderingContext, $renderChildrenClosure);
-    }
-
     /**
      * Save the associated ViewHelper node in a static public class variable.
      * called directly after the ViewHelper was built.

src/Core/ViewHelper/Traits/CompileWithRenderStatic.php

@@ -14,7 +14,8 @@
  * any ViewHelper that conforms to the `renderStatic`
  * method pattern.
  *
- * @todo add missing types with Fluid v5
+ * @deprecated Will be removed in v5. The non-static render() method
+ *             should be used instead
  */
 trait CompileWithRenderStatic
 {
@@ -27,6 +28,7 @@ trait CompileWithRenderStatic
      */
     public function render()
     {
+        trigger_error('CompileWithRenderStatic has been deprecated and will be removed in Fluid v5.', E_USER_DEPRECATED);
         return static::renderStatic(
             $this->arguments,
             $this->buildRenderChildrenClosure(),

src/Core/ViewHelper/ViewHelperInterface.php

@@ -77,27 +77,6 @@ public function handleAdditionalArguments(array $arguments);
      */
     public function validateAdditionalArguments(array $arguments);
 
-    /**
-     * Here follows a more detailed description of the arguments of this function:
-     *
-     * $arguments contains a plain array of all arguments this ViewHelper has received,
-     * including the default argument values if an argument has not been specified
-     * in the ViewHelper invocation.
-     *
-     * $renderChildrenClosure is a closure you can execute instead of $this->renderChildren().
-     * It returns the rendered child nodes, so you can simply do $renderChildrenClosure() to execute
-     * it. It does not take any parameters.
-     *
-     * $renderingContext contains references to the VariableProvider and the
-     * ViewHelperVariableContainer.
-     *
-     * @param array<string, mixed> $arguments
-     * @param \Closure $renderChildrenClosure
-     * @param RenderingContextInterface $renderingContext
-     * @return mixed the resulting value from the ViewHelper
-     */
-    public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext);
-
     /**
      * Called when being inside a cached template.
      *

tests/Functional/ViewHelpers/GroupedForViewHelperTest.php

@@ -19,7 +19,7 @@
 final class GroupedForViewHelperTest extends AbstractFunctionalTestCase
 {
     #[Test]
-    public function renderStaticThrowsExceptionWhenEachIsNotTraversable(): void
+    public function renderThrowsExceptionWhenEachIsNotTraversable(): void
     {
         $this->expectException(Exception::class);
         $this->expectExceptionCode(1253108907);
@@ -33,7 +33,7 @@ public function renderStaticThrowsExceptionWhenEachIsNotTraversable(): void
     }
 
     #[Test]
-    public function renderStaticThrowsExceptionWhenEachIsOneDimensionalArray(): void
+    public function renderThrowsExceptionWhenEachIsOneDimensionalArray(): void
     {
         $this->expectException(Exception::class);
         $this->expectExceptionCode(1253120365);
@@ -47,7 +47,7 @@ public function renderStaticThrowsExceptionWhenEachIsOneDimensionalArray(): void
     }
 
     #[Test]
-    public function renderStaticReturnsEmptyStringWhenEachIsNull(): void
+    public function renderReturnsEmptyStringWhenEachIsNull(): void
     {
         $source = '<f:groupedFor each="{items}" as="group" groupBy="by"></f:groupedFor>';
 

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

@@ -15,6 +15,7 @@
 
 final class CompilableViewHelper extends AbstractViewHelper
 {
+    // We leave this here as a test case for the deprecated feature
     use CompileWithRenderStatic;
 
     /**

tests/Functional/ViewHelpers/StaticCacheable/SharedStaticCompilableViewHelperTest.php

@@ -9,6 +9,7 @@
 
 namespace TYPO3Fluid\Fluid\Tests\Functional\ViewHelpers\StaticCacheable;
 
+use PHPUnit\Framework\Attributes\IgnoreDeprecations;
 use PHPUnit\Framework\Attributes\Test;
 use TYPO3Fluid\Fluid\Core\ViewHelper\ViewHelperInterface;
 use TYPO3Fluid\Fluid\Core\ViewHelper\ViewHelperResolver;
@@ -22,6 +23,7 @@
 final class SharedStaticCompilableViewHelperTest extends AbstractFunctionalTestCase
 {
     #[Test]
+    #[IgnoreDeprecations]
     public function renderWithSharedCompilableViewHelper(): void
     {
         // TYPO3 implements a custom ViewHelperResolver to provide DI-able ViewHelper instances. This allows

tests/Unit/Core/ViewHelper/AbstractViewHelperTest.php

@@ -10,6 +10,7 @@
 namespace TYPO3Fluid\Fluid\Tests\Unit\Core\ViewHelper;
 
 use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\IgnoreDeprecations;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\TestCase;
 use TYPO3Fluid\Fluid\Core\Compiler\TemplateCompiler;
@@ -144,14 +145,15 @@ public function testCompileReturnsAndAssignsExpectedPhpCode(): void
         $subject = $this->getMockBuilder(AbstractViewHelper::class)->onlyMethods([])->getMock();
         $result = $subject->compile('foobar', 'baz', $init, $node, new TemplateCompiler());
         self::assertEmpty($init);
-        self::assertEquals(get_class($subject) . '::renderStatic(foobar, baz, $renderingContext)', $result);
+        self::assertEquals('$renderingContext->getViewHelperInvoker()->invoke(' . get_class($subject) . '::class, foobar, $renderingContext, baz)', $result);
     }
 
     #[Test]
+    #[IgnoreDeprecations]
     public function testCallRenderMethodCanRenderViewHelperWithoutRenderMethodAndCallsRenderStatic(): void
     {
         $subject = new RenderMethodFreeViewHelper();
-        $method = new \ReflectionMethod($subject, 'callRenderMethod');
+        $method = new \ReflectionMethod($subject, 'render');
         $subject->setRenderingContext(new RenderingContext());
         $result = $method->invoke($subject);
         self::assertSame('I was rendered', $result);
@@ -162,7 +164,7 @@ public function testCallRenderMethodOnViewHelperWithoutRenderMethodWithDefaultRe
     {
         $this->expectException(Exception::class);
         $subject = new RenderMethodFreeDefaultRenderStaticViewHelper();
-        $method = new \ReflectionMethod($subject, 'callRenderMethod');
+        $method = new \ReflectionMethod($subject, 'render');
         $subject->setRenderingContext(new RenderingContext());
         $method->invoke($subject);
     }