optimize detection for incompatible filters

Author: wol-soft

src/Model/Validator/FilterValidator.php

@@ -61,11 +61,11 @@ public function __construct(
                 'isTransformingFilter' => $filter instanceof TransformingFilterInterface,
                 // check if the given value has a type matched by the filter
                 'typeCheck' => !empty($filter->getAcceptedTypes())
-                    ? '($value !== null && (' .
+                    ? '(' .
                         implode(' && ', array_map(function (string $type) use ($property): string {
                             return ReflectionTypeCheckValidator::fromType($type, $property)->getCheck();
                         }, $this->mapDataTypes($filter->getAcceptedTypes()))) .
-                        '))'
+                      ')'
                     : '',
                 'filterClass' => $filter->getFilter()[0],
                 'filterMethod' => $filter->getFilter()[1],
@@ -136,10 +136,17 @@ public function addTransformedCheck(TransformingFilterInterface $filter, Propert
      */
     private function validateFilterCompatibilityWithBaseType(FilterInterface $filter, PropertyInterface $property)
     {
-        if (!empty($filter->getAcceptedTypes()) &&
-            $property->getType() &&
-            $property->getType()->getName() &&
-            !in_array($property->getType()->getName(), $this->mapDataTypes($filter->getAcceptedTypes()))
+        if (empty($filter->getAcceptedTypes()) || !$property->getType()) {
+            return;
+        }
+
+        if (
+            (
+                $property->getType()->getName() &&
+                !in_array($property->getType()->getName(), $this->mapDataTypes($filter->getAcceptedTypes()))
+            ) || (
+                $property->getType()->isNullable() && !in_array('null', $filter->getAcceptedTypes())
+            )
         ) {
             throw new SchemaException(
                 sprintf(
@@ -174,13 +181,18 @@ private function validateFilterCompatibilityWithTransformedType(
         ))->getReturnType();
 
         if (!empty($filter->getAcceptedTypes()) &&
-            !in_array($transformedType->getName(), $this->mapDataTypes($filter->getAcceptedTypes()))
+            (
+                !in_array($transformedType->getName(), $this->mapDataTypes($filter->getAcceptedTypes())) ||
+                ($transformedType->allowsNull() && !in_array('null', $filter->getAcceptedTypes()))
+            )
         ) {
             throw new SchemaException(
                 sprintf(
                     'Filter %s is not compatible with transformed property type %s for property %s in file %s',
                     $filter->getToken(),
-                    $transformedType->getName(),
+                    $transformedType->allowsNull()
+                        ? "[null, {$transformedType->getName()}]"
+                        : $transformedType->getName(),
                     $property->getName(),
                     $property->getJsonSchema()->getFile()
                 )

tests/Basic/FilterTest.php

@@ -80,7 +80,7 @@ public function testNonExistingFilterThrowsAnException(): void
     protected function getCustomFilter(
         array $customFilter,
         string $token = 'customFilter',
-        array $acceptedTypes = ['string']
+        array $acceptedTypes = ['string', 'null']
     ): FilterInterface {
         return new class ($customFilter, $token, $acceptedTypes) implements FilterInterface {
             private $customFilter;
@@ -532,7 +532,7 @@ public function testMultipleTransformingFiltersAppliedToOnePropertyThrowsAnExcep
                 new class () extends DateTimeFilter {
                     public function getAcceptedTypes(): array
                     {
-                        return [DateTime::class];
+                        return [DateTime::class, 'null'];
                     }
 
                     public function getToken(): string
@@ -590,11 +590,12 @@ public static function exceptionFilter(string $value): void
     }
 
     /**
-     * @dataProvider namespaceDataProvider
+     * @dataProvider implicitNullNamespaceDataProvider
      *
+     * @param bool $implicitNull
      * @param string $namespace
      */
-    public function testTransformingToScalarType(string $namespace)
+    public function testTransformingToScalarType(bool $implicitNull, string $namespace)
     {
         $className = $this->generateClassFromFile(
             'TransformingScalarFilter.json',
@@ -609,7 +610,9 @@ public function testTransformingToScalarType(string $namespace)
                         'binary',
                         ['integer']
                     )
-                )
+                ),
+            false,
+            $implicitNull
         );
 
         $fqcn = $namespace . $className;
@@ -621,6 +624,15 @@ public function testTransformingToScalarType(string $namespace)
 
         $this->assertSame(['value' => 11], $object->toArray());
         $this->assertSame('{"value":11}', $object->toJSON());
+
+        $this->expectException(ErrorRegistryException::class);
+        $this->expectExceptionMessage(
+            $implicitNull
+                ? 'Filter binary is not compatible with property type NULL for property value'
+                : 'Invalid type for value. Requires [string, int], got NULL'
+        );
+
+        new $fqcn(['value' => null]);
     }
 
     public static function filterIntToBinary(int $value): string
@@ -637,7 +649,8 @@ public function testInvalidFilterChainWithTransformingFilterThrowsAnException():
     {
         $this->expectException(SchemaException::class);
         $this->expectExceptionMessage(
-            'Filter trim is not compatible with transformed property type DateTime for property filteredProperty'
+            'Filter trim is not compatible with transformed property type ' .
+            '[null, DateTime] for property filteredProperty'
         );
 
         $this->generateClassFromFileTemplate('FilterChain.json', ['["dateTime", "trim"]'], null, false);
@@ -654,7 +667,7 @@ public function testFilterChainWithTransformingFilter(): void
                     $this->getCustomFilter(
                         [self::class, 'stripTimeFilter'],
                         'stripTime',
-                        [DateTime::class]
+                        [DateTime::class, 'null']
                     )
                 ),
             false
@@ -692,7 +705,7 @@ public function testFilterChainWithTransformingFilterOnMultiTypeProperty(
                     $this->getCustomFilter(
                         [self::class, 'stripTimeFilter'],
                         'stripTime',
-                        [DateTime::class]
+                        [DateTime::class, 'null']
                     )
                 ),
             false,
@@ -722,6 +735,27 @@ public function implicitNullNamespaceDataProvider(): array
         );
     }
 
+    public function testFilterChainWithIncompatibleFilterAfterTransformingFilterOnMultiTypeProperty(): void
+    {
+        $this->expectException(SchemaException::class);
+        $this->expectExceptionMessage(
+            'Filter stripTime is not compatible with transformed ' .
+            'property type [null, DateTime] for property filteredProperty'
+        );
+
+        $this->generateClassFromFile(
+            'FilterChainMultiType.json',
+            (new GeneratorConfiguration())
+                ->addFilter(
+                    $this->getCustomFilter(
+                        [self::class, 'stripTimeFilterStrict'],
+                        'stripTime',
+                        [DateTime::class]
+                    )
+                )
+        );
+    }
+
     public function testFilterAfterTransformingFilterIsSkippedIfTransformingFilterFails(): void
     {
         $this->expectException(ErrorRegistryException::class);
@@ -736,7 +770,7 @@ public function testFilterAfterTransformingFilterIsSkippedIfTransformingFilterFa
                     $this->getCustomFilter(
                         [self::class, 'exceptionFilter'],
                         'stripTime',
-                        [DateTime::class]
+                        [DateTime::class, 'null']
                     )
                 ),
             false
@@ -766,7 +800,7 @@ public function testFilterWhichAppliesToMultiTypePropertyPartiallyThrowsAnExcept
                     $this->getCustomFilter(
                         [self::class, 'stripTimeFilter'],
                         'stripTime',
-                        [DateTime::class]
+                        [DateTime::class, 'null']
                     )
                 ),
             false
@@ -778,6 +812,11 @@ public static function stripTimeFilter(?DateTime $value): ?DateTime
         return $value !== null ? $value->setTime(0, 0) : null;
     }
 
+    public static function stripTimeFilterStrict(DateTime $value): DateTime
+    {
+        return $value->setTime(0, 0);
+    }
+
     /**
      * @dataProvider arrayFilterDataProvider
      *