[FEATURE] Introduce ArgumentProcessor API

s2b · s2b · commit f7dd6e0a640d · 2025-05-02T19:09:45.000+02:00
Fluid allows ViewHelpers to specify their API by using
`AbstractViewHelper::registerArgument()`, which is usually called in
`initializeArguments()` of the ViewHelper. Internally, `ArgumentDefinition`
is used to represent the provided argument information and constraints.
Among those is the ability to specify the type of an argument as well
as if the argument should be a required argument.

While the required status is already validated at the parser level, the
argument types are validated by the ViewHelper implementation. However,
in most cases, Fluid's built-in validation method by `AbstractViewHelper`
is used. Unfortunately, this implementation has many flaws:

* it only covers a subset of possible types (e. g. it covers "boolean", but
  not "bool")
* It doesn't ensure the correct argument type for scalar arguments (e. g.
  you might receive an integer in your ViewHelper implementation, even
  if the argument has been defined as "string")
* There are several small inconsistencies

In order to fix this in a future Fluid version without introducing too many
hard-to-debug breaking changes, a new API is introduced which extracts
validation functionality from the `AbstractViewHelper` and allows
alternative implementations in the future. It also adds the possibility
to pre-process arguments before their validity is checked, which will
allow implicit type conversions in the future (e. g. convert "int" to "string").

As a first step, the existing helper methods `isValidType()` and
`getFirstElementOfNonEmpty()` are deprecated and extracted into
a `LenientArgumentProcessor`, which mimicks the current validation
behavior. Like the current implementation, it doesn't modify the argument
types, but rather passes them along unmodified.

The new API is then called for the pre-processing step in
`ViewHelperInvoker` as well as the validation step in `AbstractViewHelper`.
In this first implementation, it isn't possible yet to replace the default
implementation with an alternative.
diff --git a/src/Core/ViewHelper/AbstractViewHelper.php b/src/Core/ViewHelper/AbstractViewHelper.php
@@ -333,20 +333,20 @@ public function prepareArguments()
      */
     public function validateArguments()
     {
+        // @todo make configurable with Fluid v5
+        $argumentProcessor = new LenientArgumentProcessor();
         $argumentDefinitions = $this->prepareArguments();
         foreach ($argumentDefinitions as $argumentName => $registeredArgument) {
+            // Note: This relies on the TemplateParser to check for missing required arguments
             if ($this->hasArgument($argumentName)) {
                 $value = $this->arguments[$argumentName];
-                $type = $registeredArgument->getType();
-                if ($value !== $registeredArgument->getDefaultValue() && $type !== 'mixed') {
+                if (!$argumentProcessor->isValid($value, $registeredArgument)) {
                     $givenType = is_object($value) ? get_class($value) : gettype($value);
-                    if (!$this->isValidType($type, $value)) {
-                        throw new \InvalidArgumentException(
-                            'The argument "' . $argumentName . '" was registered with type "' . $type . '", but is of type "' .
-                            $givenType . '" in view helper "' . get_class($this) . '".',
-                            1256475113,
-                        );
-                    }
+                    throw new \InvalidArgumentException(
+                        'The argument "' . $argumentName . '" was registered with type "' . $registeredArgument->getType() . '", but is of type "' .
+                        $givenType . '" in view helper "' . get_class($this) . '".',
+                        1256475113,
+                    );
                 }
             }
         }
@@ -358,9 +358,11 @@ public function validateArguments()
      * @param string $type
      * @param mixed $value
      * @return bool
+     * @deprecated Will be removed in v5. Use the new argument processing API to validate ViewHelper arguments.
      */
     protected function isValidType($type, $value)
     {
+        trigger_error('AbstractViewHelper::isValidType() has been deprecated and will be removed in Fluid v5.', E_USER_DEPRECATED);
         if ($type === 'object') {
             if (!is_object($value)) {
                 return false;
@@ -396,9 +398,11 @@ protected function isValidType($type, $value)
      *
      * @param mixed $value
      * @return mixed
+     * @deprecated Will be removed in v5. Use the new argument processing API to validate ViewHelper arguments.
      */
     protected function getFirstElementOfNonEmpty($value)
     {
+        trigger_error('AbstractViewHelper::getFirstElementOfNonEmpty() has been deprecated and will be removed in Fluid v5.', E_USER_DEPRECATED);
         if (is_array($value)) {
             return reset($value);
         }
diff --git a/src/Core/ViewHelper/ArgumentProcessorInterface.php b/src/Core/ViewHelper/ArgumentProcessorInterface.php
@@ -0,0 +1,16 @@
+<?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\Core\ViewHelper;
+
+interface ArgumentProcessorInterface
+{
+    public function process(mixed $value, ArgumentDefinition $definition): mixed;
+    public function isValid(mixed $value, ArgumentDefinition $definition): bool;
+}
diff --git a/src/Core/ViewHelper/LenientArgumentProcessor.php b/src/Core/ViewHelper/LenientArgumentProcessor.php
@@ -0,0 +1,105 @@
+<?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\Core\ViewHelper;
+
+use ArrayAccess;
+use Traversable;
+
+/**
+ * The LenientArgumentProcessor mimicks the ViewHelper argument validation
+ * logic used by Fluid up to (and including) 4.x:
+ *
+ * 1. Argument types are validated against the isValidateType() implementation
+ *    which previously was part of AbstractViewHelper. For clear violations
+ *    (e. g. a string instead of an array), validation fails.
+ * 2. Valid types are NOT guaranteed to ViewHelpers in all cases, since scalar
+ *    types are not converted implicitly (meaning: You might receive an integer,
+ *    even if the ViewHelper specifies string as argument type)
+ * 3. ArrayAccess and Traversible are valid array arguments
+ * 4. Objects implementing Stringable are valid string arguments
+ * 5. In addition to PHP's internal types, class names can be specified as well
+ * 6. Collection types (e. g. "string[]") are possible. Only the first item
+ *    of a collection is validated.
+ */
+final class LenientArgumentProcessor implements ArgumentProcessorInterface
+{
+    public function process(mixed $value, ArgumentDefinition $definition): mixed
+    {
+        // No processing, argument values are passed on unmodified
+        return $value;
+    }
+
+    public function isValid(mixed $value, ArgumentDefinition $definition): bool
+    {
+        // Allow everything for mixed type
+        if ($definition->getType() === 'mixed') {
+            return true;
+        }
+
+        // Always allow default value if argument is not required
+        if (!$definition->isRequired() && $value === $definition->getDefaultValue()) {
+            return true;
+        }
+
+        // Perform type validation
+        return $this->isValidType($definition->getType(), $value);
+    }
+
+    /**
+     * Check whether the defined type matches the value type
+     */
+    private function isValidType(string $type, mixed $value): bool
+    {
+        if ($type === 'object') {
+            if (!is_object($value)) {
+                return false;
+            }
+        } elseif ($type === 'array' || substr($type, -2) === '[]') {
+            if (!is_array($value) && !$value instanceof ArrayAccess && !$value instanceof Traversable && !empty($value)) {
+                return false;
+            }
+            if (substr($type, -2) === '[]') {
+                $firstElement = $this->getFirstElementOfNonEmpty($value);
+                if ($firstElement === null) {
+                    return true;
+                }
+                return $this->isValidType(substr($type, 0, -2), $firstElement);
+            }
+        } elseif ($type === 'string') {
+            if (is_object($value) && !method_exists($value, '__toString')) {
+                return false;
+            }
+        } elseif ($type === 'boolean' && !is_bool($value)) {
+            return false;
+        } elseif (class_exists($type) && $value !== null && !$value instanceof $type) {
+            return false;
+        } elseif (is_object($value) && !is_a($value, $type, true)) {
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Return the first element of the given array, ArrayAccess or Traversable
+     * that is not empty
+     */
+    private function getFirstElementOfNonEmpty(mixed $value): mixed
+    {
+        if (is_array($value)) {
+            return reset($value);
+        }
+        if ($value instanceof Traversable) {
+            foreach ($value as $element) {
+                return $element;
+            }
+        }
+        return null;
+    }
+}
diff --git a/src/Core/ViewHelper/ViewHelperInvoker.php b/src/Core/ViewHelper/ViewHelperInvoker.php
@@ -46,6 +46,8 @@ public function invoke(string|ViewHelperInterface $viewHelperClassNameOrInstance
         }
         $argumentDefinitions = $viewHelperResolver->getArgumentDefinitionsForViewHelper($viewHelper);
 
+        // @todo make configurable with Fluid v5
+        $argumentProcessor = new LenientArgumentProcessor();
         try {
             // Convert nodes to actual values (in uncached context)
             $arguments = array_map(
@@ -56,7 +58,9 @@ public function invoke(string|ViewHelperInterface $viewHelperClassNameOrInstance
             // Determine arguments defined by the ViewHelper API
             $registeredArguments = [];
             foreach ($argumentDefinitions as $argumentName => $argumentDefinition) {
-                $registeredArguments[$argumentName] = $arguments[$argumentName] ?? $argumentDefinition->getDefaultValue();
+                $registeredArguments[$argumentName] = isset($arguments[$argumentName])
+                    ? $argumentProcessor->process($arguments[$argumentName], $argumentDefinition)
+                    : $argumentDefinition->getDefaultValue();
                 unset($arguments[$argumentName]);
             }
 
diff --git a/tests/Functional/Core/ViewHelper/ViewHelperArgumentTypesTest.php b/tests/Functional/Core/ViewHelper/ViewHelperArgumentTypesTest.php
@@ -17,6 +17,40 @@
 
 final class ViewHelperArgumentTypesTest extends AbstractFunctionalTestCase
 {
+    #[Test]
+    public function invalidArgumentTypeUncached(): void
+    {
+        $source = '<f:count subject="test" />';
+
+        self::expectExceptionCode(1256475113);
+
+        $view = new TemplateView();
+        $view->getRenderingContext()->setCache(self::$cache);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($source);
+        $view->render();
+    }
+
+    #[Test]
+    public function invalidArgumentTypeCached(): void
+    {
+        $source = '<f:count subject="test" />';
+
+        self::expectExceptionCode(1256475113);
+
+        try {
+            $view = new TemplateView();
+            $view->getRenderingContext()->setCache(self::$cache);
+            $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($source);
+            $view->render();
+        } catch (\Exception) {
+        }
+
+        $view = new TemplateView();
+        $view->getRenderingContext()->setCache(self::$cache);
+        $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($source);
+        $view->render();
+    }
+
     public static function scalarArgumentsDataProvider(): array
     {
         return [
diff --git a/tests/Unit/Core/ViewHelper/AbstractViewHelperTest.php b/tests/Unit/Core/ViewHelper/AbstractViewHelperTest.php
@@ -46,6 +46,7 @@ public static function getFirstElementOfNonEmptyTestValues(): array
 
     #[DataProvider('getFirstElementOfNonEmptyTestValues')]
     #[Test]
+    #[IgnoreDeprecations]
     public function getFirstElementOfNonEmptyReturnsExpectedValue(mixed $input, ?string $expected): void
     {
         $subject = $this->getMockBuilder(AbstractViewHelper::class)->onlyMethods([])->getMock();
@@ -66,15 +67,6 @@ public function registeringTheSameArgumentNameAgainOverridesArgument(): void
         self::assertEquals($expected, $subject->prepareArguments());
     }
 
-    #[Test]
-    public function validateArgumentsAcceptsAllObjectsImplementingArrayAccessAsAnArray(): void
-    {
-        $subject = $this->getMockBuilder(AbstractViewHelper::class)->onlyMethods(['prepareArguments'])->getMock();
-        $subject->setArguments(['test' => new \ArrayObject()]);
-        $subject->expects(self::once())->method('prepareArguments')->willReturn(['test' => new ArgumentDefinition('test', 'array', 'documentation', false)]);
-        $subject->validateArguments();
-    }
-
     #[Test]
     public function setRenderingContextShouldSetInnerVariables(): void
     {
@@ -102,31 +94,6 @@ public function renderChildrenCallsRenderChildrenClosureIfSet(): void
         self::assertEquals('foobar', $result);
     }
 
-    public static function validateArgumentsErrorsDataProvider(): array
-    {
-        return [
-            [new ArgumentDefinition('test', 'boolean', '', true), ['bad']],
-            [new ArgumentDefinition('test', 'string', '', true), new \ArrayIterator(['bar'])],
-            [new ArgumentDefinition('test', 'DateTime', '', true), new \ArrayIterator(['bar'])],
-            [new ArgumentDefinition('test', 'DateTime', '', true), 'test'],
-            [new ArgumentDefinition('test', 'integer', '', true), new \ArrayIterator(['bar'])],
-            [new ArgumentDefinition('test', 'object', '', true), 'test'],
-            [new ArgumentDefinition('test', 'string[]', '', true), [new \DateTime('now'), 'test']],
-        ];
-    }
-
-    #[DataProvider('validateArgumentsErrorsDataProvider')]
-    #[Test]
-    public function validateArgumentsErrors(ArgumentDefinition $argument, array|string|object $value): void
-    {
-        $this->expectException(\InvalidArgumentException::class);
-        $subject = $this->getMockBuilder(AbstractViewHelper::class)->onlyMethods(['hasArgument', 'prepareArguments'])->getMock();
-        $subject->expects(self::once())->method('prepareArguments')->willReturn([$argument->getName() => $argument]);
-        $subject->expects(self::once())->method('hasArgument')->with($argument->getName())->willReturn(true);
-        $subject->setArguments([$argument->getName() => $value]);
-        $subject->validateArguments();
-    }
-
     #[Test]
     public function validateAdditionalArgumentsThrowsExceptionIfNotEmpty(): void
     {
diff --git a/tests/Unit/Core/ViewHelper/LenientArgumentProcessorTest.php b/tests/Unit/Core/ViewHelper/LenientArgumentProcessorTest.php
