feat(generator): allow skipping uninitialized property when skipping …

…null values
jolicode · Mar 8, 2024 · 94cf750 · 94cf750
1 parent f0bb0fe
commit 94cf750
Show file tree

Hide file tree

Showing 10 changed files with 155 additions and 21 deletions.
diff --git a/src/Extractor/ReadAccessor.php b/src/Extractor/ReadAccessor.php
@@ -154,6 +154,64 @@ public function getExpression(Expr\Variable $input): Expr
         throw new CompileException('Invalid accessor for read expression');
     }
 
+    public function getIsNullExpression(Expr\Variable $input): Expr
+    {
+        if (self::TYPE_METHOD === $this->type) {
+            $methodCallExpr = $this->getExpression($input);
+
+            /*
+             * null !== $methodCallExpr
+             */
+            return new Expr\BinaryOp\Identical(
+                new Expr\ConstFetch(new Name('null')),
+                $methodCallExpr,
+            );
+        }
+
+        if (self::TYPE_PROPERTY === $this->type) {
+            if ($this->private) {
+                /*
+                 * When the property is private we use the extract callback that can read this value
+                 *
+                 * @see \AutoMapper\Extractor\ReadAccessor::getExtractIsNullCallback()
+                 *
+                 * $this->extractIsNullCallbacks['property_name']($input)
+                 */
+                return new Expr\FuncCall(
+                    new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractIsNullCallbacks'), new Scalar\String_($this->accessor)),
+                    [
+                        new Arg($input),
+                    ]
+                );
+            }
+
+            /*
+             * Use the property fetch to read the value
+             *
+             * isset($input->property_name)
+             */
+            return new Expr\Isset_([new Expr\PropertyFetch($input, $this->accessor)]);
+        }
+
+        if (self::TYPE_ARRAY_DIMENSION === $this->type) {
+            /*
+             * Use the array dim fetch to read the value
+             *
+             * isset($input['property_name'])
+             */
+            return new Expr\Isset_([new Expr\ArrayDimFetch($input, new Scalar\String_($this->accessor))]);
+        }
+
+        if (self::TYPE_SOURCE === $this->type) {
+            return new Expr\BinaryOp\Identical(
+                new Expr\ConstFetch(new Name('null')),
+                $input,
+            );
+        }
+
+        throw new CompileException('Invalid accessor for read expression');
+    }
+
     /**
      * Get AST expression for binding closure when dealing with a private property.
      */
@@ -193,4 +251,36 @@ public function getExtractCallback(string $className): ?Expr
             new Arg(new Scalar\String_($className)),
         ]);
     }
+
+    /**
+     * Get AST expression for binding closure when dealing with a private property.
+     */
+    public function getExtractIsNullCallback(string $className): ?Expr
+    {
+        if ($this->type !== self::TYPE_PROPERTY || !$this->private) {
+            return null;
+        }
+
+        /*
+         * Create extract is null callback for this accessor
+         *
+         *  \Closure::bind(function ($object) {
+         *      return isset($object->property_name);
+         *  }, null, $className)
+         */
+        return new Expr\StaticCall(new Name\FullyQualified(\Closure::class), 'bind', [
+            new Arg(
+                new Expr\Closure([
+                    'params' => [
+                        new Param(new Expr\Variable('object')),
+                    ],
+                    'stmts' => [
+                        new Stmt\Return_(new Expr\Isset_([new Expr\PropertyFetch(new Expr\Variable('object'), $this->accessor)])),
+                    ],
+                ])
+            ),
+            new Arg(new Expr\ConstFetch(new Name('null'))),
+            new Arg(new Scalar\String_($className)),
+        ]);
+    }
 }
diff --git a/src/GeneratedMapper.php b/src/GeneratedMapper.php
@@ -19,6 +19,9 @@ abstract class GeneratedMapper implements MapperInterface
 
     protected $extractCallbacks = [];
 
+    /** @var callable[] */
+    protected array $extractIsNullCallbacks = [];
+
     protected $cachedTarget;
 
     protected $circularReferenceHandler;

diff --git a/src/Generator/MapperConstructorGenerator.php b/src/Generator/MapperConstructorGenerator.php
@@ -30,6 +30,7 @@ public function getStatements(MapperGeneratorMetadataInterface $mapperMetadata):
 
         foreach ($mapperMetadata->getPropertiesMapping() as $propertyMapping) {
             $constructStatements[] = $this->extractCallbackForProperty($propertyMapping);
+            $constructStatements[] = $this->extractIsNullCallbackForProperty($propertyMapping);
             $constructStatements[] = $this->hydrateCallbackForProperty($propertyMapping);
         }
 
@@ -62,6 +63,30 @@ private function extractCallbackForProperty(PropertyMapping $propertyMapping): ?
             ));
     }
 
+    /**
+     * Add read callback to the constructor of the generated mapper.
+     *
+     * ```php
+     * $this->extractIsNullCallbacks['propertyName'] = $extractIsNullCallback;
+     * ```
+     */
+    private function extractIsNullCallbackForProperty(PropertyMapping $propertyMapping): ?Stmt\Expression
+    {
+        $mapperMetadata = $propertyMapping->mapperMetadata;
+
+        $extractNullCallback = $propertyMapping->readAccessor?->getExtractIsNullCallback($mapperMetadata->getSource());
+
+        if (!$extractNullCallback) {
+            return null;
+        }
+
+        return new Stmt\Expression(
+            new Expr\Assign(
+                new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractIsNullCallbacks'), new Scalar\String_($propertyMapping->property)),
+                $extractNullCallback
+            ));
+    }
+
     /**
      * Add hydrate callback to the constructor of the generated mapper.
      *

diff --git a/src/Generator/PropertyConditionsGenerator.php b/src/Generator/PropertyConditionsGenerator.php
@@ -112,16 +112,10 @@ private function isAllowedAttribute(PropertyMapping $propertyMapping): ?Expr
 
         $variableRegistry = $mapperMetadata->getVariableRegistry();
 
-        /** Create expression on how to read the value from the source */
-        $sourcePropertyAccessor = new Expr\Assign(
-            $variableRegistry->getFieldValueVariable($propertyMapping),
-            $propertyMapping->readAccessor->getExpression($variableRegistry->getSourceInput())
-        );
-
         return new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'isAllowedAttribute', [
             new Arg($variableRegistry->getContext()),
             new Arg(new Scalar\String_($propertyMapping->property)),
-            new Arg($sourcePropertyAccessor),
+            new Arg($propertyMapping->readAccessor->getIsNullExpression($variableRegistry->getSourceInput())),
         ]);
     }
 

diff --git a/src/Generator/PropertyStatementsGenerator.php b/src/Generator/PropertyStatementsGenerator.php
@@ -35,12 +35,16 @@ public function generate(PropertyMapping $propertyMapping): array
         }
 
         $variableRegistry = $mapperMetadata->getVariableRegistry();
-
         $fieldValueVariable = $variableRegistry->getFieldValueVariable($propertyMapping);
 
+        if ($propertyMapping->readAccessor) {
+            $fieldValueVariable = $propertyMapping->readAccessor->getExpression($variableRegistry->getSourceInput());
+        }
+
         if ($propertyMapping->hasCustomTransformer()) {
-            $output = $this->customTransformerExtractor->extract($propertyMapping->transformer, $fieldValueVariable, $variableRegistry->getSourceInput());
             $propStatements = [];
+
+            $output = $this->customTransformerExtractor->extract($propertyMapping->transformer, $fieldValueVariable, $variableRegistry->getSourceInput());
         } else {
             /* Create expression to transform the read value into the wanted written value, depending on the transform it may add new statements to get the correct value */
             [$output, $propStatements] = $propertyMapping->transformer->transform(

diff --git a/src/MapperContext.php b/src/MapperContext.php
@@ -175,9 +175,9 @@ public static function withReference(array $context, string $reference, &$object
     /**
      * Check whether an attribute is allowed to be mapped.
      */
-    public static function isAllowedAttribute(array $context, string $attribute, $value): bool
+    public static function isAllowedAttribute(array $context, string $attribute, bool $valueIsNotNullOrNotUndefined): bool
     {
-        if (($context[self::SKIP_NULL_VALUES] ?? false) && null === $value) {
+        if (($context[self::SKIP_NULL_VALUES] ?? false) && !$valueIsNotNullOrNotUndefined) {
             return false;
         }
 

diff --git a/tests/AutoMapperTest.php b/tests/AutoMapperTest.php
@@ -31,6 +31,7 @@
 use AutoMapper\Tests\Fixtures\Order;
 use AutoMapper\Tests\Fixtures\PetOwner;
 use AutoMapper\Tests\Fixtures\Transformer\MoneyTransformerFactory;
+use AutoMapper\Tests\Fixtures\Uninitialized;
 use Symfony\Component\HttpKernel\Kernel;
 use Symfony\Component\Serializer\NameConverter\AdvancedNameConverterInterface;
 use Symfony\Component\Uid\Ulid;
@@ -1266,4 +1267,14 @@ public function testItCanMapFromArrayWithMissingNullableProperty(): void
             $this->autoMapper->map(['foo' => 1], ClassWithNullablePropertyInConstructor::class)
         );
     }
+
+    public function testNoErrorWithUninitializedProperty(): void
+    {
+        $this->buildAutoMapper(mapPrivatePropertiesAndMethod: true);
+
+        self::assertSame(
+            ['bar' => 'bar'],
+            $this->autoMapper->map(new Uninitialized(), 'array', ['skip_null_values' => true])
+        );
+    }
 }
diff --git a/tests/Fixtures/Uninitialized.php b/tests/Fixtures/Uninitialized.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace AutoMapper\Tests\Fixtures;
+
+class Uninitialized
+{
+    public string $foo;
+
+    public string $bar = 'bar';
+}
diff --git a/tests/MapperContextTest.php b/tests/MapperContextTest.php
@@ -19,9 +19,9 @@ public function testIsAllowedAttribute(): void
         $context->setAllowedAttributes(['id', 'age']);
         $context->setIgnoredAttributes(['age']);
 
-        self::assertTrue(MapperContext::isAllowedAttribute($context->toArray(), 'id', 1));
-        self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'age', 29));
-        self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'name', 'Baptiste'));
+        self::assertTrue(MapperContext::isAllowedAttribute($context->toArray(), 'id', true));
+        self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'age', true));
+        self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'name', true));
     }
 
     public function testCircularReferenceLimit(): void
@@ -127,7 +127,7 @@ public function testWithNewContextAllowedAttributesNested(): void
             ],
         ];
 
-        self::assertTrue(MapperContext::isAllowedAttribute($context, 'foo', 1));
+        self::assertTrue(MapperContext::isAllowedAttribute($context, 'foo', true));
         $newContext = MapperContext::withNewContext($context, 'foo');
 
         self::assertEquals(['bar'], $newContext[MapperContext::ALLOWED_ATTRIBUTES]);
@@ -136,6 +136,6 @@ public function testWithNewContextAllowedAttributesNested(): void
     public function testSkipNullValues(): void
     {
         $context = [MapperContext::SKIP_NULL_VALUES => true];
-        self::assertFalse(MapperContext::isAllowedAttribute($context, 'id', null));
+        self::assertFalse(MapperContext::isAllowedAttribute($context, 'id', false));
     }
 }
diff --git a/tools/phpstan/phpstan-baseline.neon b/tools/phpstan/phpstan-baseline.neon
@@ -345,11 +345,6 @@ parameters:
 			count: 1
 			path: ../../src/MapperContext.php
 
-		-
-			message: "#^Method AutoMapper\\\\MapperContext\\:\\:isAllowedAttribute\\(\\) has parameter \\$value with no type specified\\.$#"
-			count: 1
-			path: ../../src/MapperContext.php
-
 		-
 			message: "#^Method AutoMapper\\\\MapperContext\\:\\:setAllowedAttributes\\(\\) has parameter \\$allowedAttributes with no value type specified in iterable type array\\.$#"
 			count: 1