Merge pull request #1 from nikophil/feat/handle-private-properties

feat: better private properties handling
jolicode · Sep 26, 2023 · ad6a5d6 · ad6a5d6
2 parents baced6f + c883cbd
commit ad6a5d6
Show file tree

Hide file tree

Showing 18 changed files with 213 additions and 98 deletions.
diff --git a/src/AutoMapper.php b/src/AutoMapper.php
@@ -152,7 +152,7 @@ public function bindTransformerFactory(TransformerFactoryInterface $transformerF
     }
 
     public static function create(
-        bool $private = true,
+        bool $mapPrivateProperties = false,
         ClassLoaderInterface $loader = null,
         AdvancedNameConverterInterface $nameConverter = null,
         string $classPrefix = 'Mapper_',
@@ -171,19 +171,9 @@ public static function create(
             ));
         }
 
-        $flags = ReflectionExtractor::ALLOW_PUBLIC;
+        $flags = ReflectionExtractor::ALLOW_PUBLIC | ReflectionExtractor::ALLOW_PROTECTED | ReflectionExtractor::ALLOW_PRIVATE;
 
-        if ($private) {
-            $flags |= ReflectionExtractor::ALLOW_PROTECTED | ReflectionExtractor::ALLOW_PRIVATE;
-        }
-
-        $reflectionExtractor = new ReflectionExtractor(
-            null,
-            null,
-            null,
-            true,
-            $flags
-        );
+        $reflectionExtractor = new ReflectionExtractor(accessFlags: $flags);
 
         $phpDocExtractor = new PhpDocExtractor();
         $propertyInfoExtractor = new PropertyInfoExtractor(
@@ -226,7 +216,8 @@ public static function create(
             $fromTargetMappingExtractor,
             $classPrefix,
             $attributeChecking,
-            $dateTimeFormat
+            $dateTimeFormat,
+            $mapPrivateProperties
         )) : new self($loader, $transformerFactory);
 
         $transformerFactory->addTransformerFactory(new MultipleTransformerFactory($transformerFactory));

diff --git a/src/Extractor/FromSourceMappingExtractor.php b/src/Extractor/FromSourceMappingExtractor.php
@@ -8,6 +8,7 @@
 use AutoMapper\MapperMetadataInterface;
 use AutoMapper\Transformer\TransformerFactoryInterface;
 use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
+use Symfony\Component\PropertyInfo\PropertyReadInfo;
 use Symfony\Component\PropertyInfo\PropertyReadInfoExtractorInterface;
 use Symfony\Component\PropertyInfo\PropertyWriteInfoExtractorInterface;
 use Symfony\Component\PropertyInfo\Type;
@@ -85,7 +86,8 @@ public function getPropertiesMapping(MapperMetadataInterface $mapperMetadata): a
                 $this->getGroups($mapperMetadata->getTarget(), $property),
                 $this->getMaxDepth($mapperMetadata->getSource(), $property),
                 $this->isIgnoredProperty($mapperMetadata->getSource(), $property),
-                $this->isIgnoredProperty($mapperMetadata->getTarget(), $property)
+                $this->isIgnoredProperty($mapperMetadata->getTarget(), $property),
+                PropertyReadInfo::VISIBILITY_PUBLIC === $this->readInfoExtractor->getReadInfo($mapperMetadata->getSource(), $property)?->getVisibility() ?? true,
             );
         }
 

diff --git a/src/Extractor/FromTargetMappingExtractor.php b/src/Extractor/FromTargetMappingExtractor.php
@@ -8,6 +8,7 @@
 use AutoMapper\MapperMetadataInterface;
 use AutoMapper\Transformer\TransformerFactoryInterface;
 use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
+use Symfony\Component\PropertyInfo\PropertyReadInfo;
 use Symfony\Component\PropertyInfo\PropertyReadInfoExtractorInterface;
 use Symfony\Component\PropertyInfo\PropertyWriteInfo;
 use Symfony\Component\PropertyInfo\PropertyWriteInfoExtractorInterface;
@@ -84,7 +85,8 @@ public function getPropertiesMapping(MapperMetadataInterface $mapperMetadata): a
                 $this->getGroups($mapperMetadata->getTarget(), $property),
                 $this->getMaxDepth($mapperMetadata->getTarget(), $property),
                 $this->isIgnoredProperty($mapperMetadata->getSource(), $property),
-                $this->isIgnoredProperty($mapperMetadata->getTarget(), $property)
+                $this->isIgnoredProperty($mapperMetadata->getTarget(), $property),
+                PropertyReadInfo::VISIBILITY_PUBLIC === $this->readInfoExtractor->getReadInfo($mapperMetadata->getSource(), $property)?->getVisibility() ?? true,
             );
         }
 

diff --git a/src/Extractor/MapToContextPropertyInfoExtractorDecorator.php b/src/Extractor/MapToContextPropertyInfoExtractorDecorator.php
@@ -5,21 +5,31 @@
 namespace AutoMapper\Extractor;
 
 use AutoMapper\Attribute\MapToContext;
+use Symfony\Component\PropertyInfo\Extractor\ConstructorArgumentTypeExtractorInterface;
 use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
 use Symfony\Component\PropertyInfo\PropertyAccessExtractorInterface;
+use Symfony\Component\PropertyInfo\PropertyInitializableExtractorInterface;
+use Symfony\Component\PropertyInfo\PropertyListExtractorInterface;
 use Symfony\Component\PropertyInfo\PropertyReadInfo;
 use Symfony\Component\PropertyInfo\PropertyReadInfoExtractorInterface;
+use Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface;
+use Symfony\Component\PropertyInfo\PropertyWriteInfo;
+use Symfony\Component\PropertyInfo\PropertyWriteInfoExtractorInterface;
 
-final readonly class MapToContextPropertyInfoExtractorDecorator implements PropertyAccessExtractorInterface, PropertyReadInfoExtractorInterface
+final readonly class MapToContextPropertyInfoExtractorDecorator implements PropertyListExtractorInterface, PropertyTypeExtractorInterface, PropertyAccessExtractorInterface, PropertyInitializableExtractorInterface, PropertyReadInfoExtractorInterface, PropertyWriteInfoExtractorInterface, ConstructorArgumentTypeExtractorInterface
 {
     public function __construct(
-        private PropertyReadInfoExtractorInterface&PropertyAccessExtractorInterface $propertyReadInfoExtractor
+        private ReflectionExtractor $decorated
     ) {
     }
 
     public function getReadInfo(string $class, string $property, array $context = []): ?PropertyReadInfo
     {
-        $readInfo = $this->propertyReadInfoExtractor->getReadInfo($class, $property, $context);
+        $readInfo = $this->decorated->getReadInfo($class, $property, $context);
+
+        if ($class === 'array') {
+            return $readInfo;
+        }
 
         if (null === $readInfo || $readInfo->getType() === PropertyReadInfo::TYPE_PROPERTY && PropertyReadInfo::VISIBILITY_PUBLIC !== $readInfo->getVisibility()) {
             $reflClass = new \ReflectionClass($class);
@@ -56,7 +66,7 @@ public function isReadable(string $class, string $property, array $context = [])
 
     public function isWritable(string $class, string $property, array $context = []): bool
     {
-        return $this->propertyReadInfoExtractor->isWritable($class, $property, $context);
+        return $this->decorated->isWritable($class, $property, $context);
     }
 
     private function camelize(string $string): string
@@ -91,4 +101,29 @@ private function isAllowedProperty(string $class, string $property, bool $writeA
 
         return false;
     }
+
+    public function getTypesFromConstructor(string $class, string $property): ?array
+    {
+        return $this->decorated->getTypesFromConstructor($class, $property);
+    }
+
+    public function isInitializable(string $class, string $property, array $context = []): ?bool
+    {
+        return $this->decorated->isInitializable($class, $property, $context);
+    }
+
+    public function getProperties(string $class, array $context = [])
+    {
+        return $this->decorated->getProperties($class, $context);
+    }
+
+    public function getTypes(string $class, string $property, array $context = [])
+    {
+        return $this->decorated->getTypes($class, $property, $context);
+    }
+
+    public function getWriteInfo(string $class, string $property, array $context = []): ?PropertyWriteInfo
+    {
+        return $this->decorated->getWriteInfo($class, $property, $context);
+    }
 }
diff --git a/src/Extractor/MappingExtractor.php b/src/Extractor/MappingExtractor.php
@@ -22,7 +22,7 @@ abstract class MappingExtractor implements MappingExtractorInterface
 {
     public function __construct(
         protected readonly PropertyInfoExtractorInterface $propertyInfoExtractor,
-        private readonly PropertyReadInfoExtractorInterface $readInfoExtractor,
+        protected readonly PropertyReadInfoExtractorInterface $readInfoExtractor,
         protected readonly PropertyWriteInfoExtractorInterface $writeInfoExtractor,
         protected readonly TransformerFactoryInterface $transformerFactory,
         private readonly ?ClassMetadataFactoryInterface $classMetadataFactory = null,
@@ -47,7 +47,8 @@ public function getReadAccessor(string $source, string $target, string $property
             $type,
             $readInfo->getName(),
             $source,
-            PropertyReadInfo::VISIBILITY_PUBLIC !== $readInfo->getVisibility()
+            PropertyReadInfo::VISIBILITY_PUBLIC !== $readInfo->getVisibility(),
+            $property
         );
     }
 

diff --git a/src/Extractor/PropertyMapping.php b/src/Extractor/PropertyMapping.php
@@ -24,12 +24,15 @@ public function __construct(
         public readonly ?array $targetGroups = null,
         public readonly ?int $maxDepth = null,
         public readonly bool $sourceIgnored = false,
-        public readonly bool $targetIgnored = false
+        public readonly bool $targetIgnored = false,
+        public readonly bool $isPublic = false,
     ) {
     }
 
-    public function shouldIgnoreProperty(): bool
+    public function shouldIgnoreProperty(bool $shouldMapPrivateProperties = true): bool
     {
-        return $this->sourceIgnored || $this->targetIgnored;
+        return $this->sourceIgnored
+            || $this->targetIgnored
+            || !($shouldMapPrivateProperties || $this->isPublic);
     }
 }
diff --git a/src/Extractor/ReadAccessor.php b/src/Extractor/ReadAccessor.php
@@ -28,10 +28,11 @@ final class ReadAccessor
 
     public function __construct(
         private readonly int $type,
-        private readonly string $name,
+        private readonly string $accessor,
         private readonly ?string $sourceClass = null,
-        private readonly bool $private = false)
-    {
+        private readonly bool $private = false,
+        private readonly ?string $name = null, // will be the name of the property if different from accessor
+    ) {
         if (self::TYPE_METHOD === $this->type && null === $this->sourceClass) {
             throw new \InvalidArgumentException('Source class must be provided when using "method" type.');
         }
@@ -48,7 +49,7 @@ public function getExpression(Expr\Variable $input): Expr
             $methodCallArguments = [];
 
             if (\PHP_VERSION_ID >= 80000 && class_exists($this->sourceClass)) {
-                $parameters = (new \ReflectionMethod($this->sourceClass, $this->name))->getParameters();
+                $parameters = (new \ReflectionMethod($this->sourceClass, $this->accessor))->getParameters();
 
                 foreach ($parameters as $parameter) {
                     if ($attribute = ($parameter->getAttributes(MapToContext::class)[0] ?? null)) {
@@ -72,7 +73,7 @@ public function getExpression(Expr\Variable $input): Expr
                                         [
                                             new Arg(
                                                 new Scalar\String_(
-                                                    "Parameter \"\${$parameter->getName()}\" of method \"{$this->sourceClass}\"::\"{$this->name}()\" is configured to be mapped to context but no value was found in the context."
+                                                    "Parameter \"\${$parameter->getName()}\" of method \"{$this->sourceClass}\"::\"{$this->accessor}()\" is configured to be mapped to context but no value was found in the context."
                                                 )
                                             ),
                                         ]
@@ -81,29 +82,38 @@ public function getExpression(Expr\Variable $input): Expr
                             )
                         );
                     } elseif (!$parameter->isDefaultValueAvailable()) {
-                        throw new \InvalidArgumentException("Accessors method \"{$this->sourceClass}\"::\"{$this->name}()\" parameters must have either a default value or the #[MapToContext] attribute.");
+                        throw new \InvalidArgumentException("Accessors method \"{$this->sourceClass}\"::\"{$this->accessor}()\" parameters must have either a default value or the #[MapToContext] attribute.");
                     }
                 }
             }
 
-            return new Expr\MethodCall($input, $this->name, $methodCallArguments);
+            if ($this->private) {
+                return new Expr\FuncCall(
+                    new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractCallbacks'), new Scalar\String_($this->name ?? $this->accessor)),
+                    [
+                        new Arg($input),
+                    ]
+                );
+            }
+
+            return new Expr\MethodCall($input, $this->accessor, $methodCallArguments);
         }
 
         if (self::TYPE_PROPERTY === $this->type) {
             if ($this->private) {
                 return new Expr\FuncCall(
-                    new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractCallbacks'), new Scalar\String_($this->name)),
+                    new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractCallbacks'), new Scalar\String_($this->accessor)),
                     [
                         new Arg($input),
                     ]
                 );
             }
 
-            return new Expr\PropertyFetch($input, $this->name);
+            return new Expr\PropertyFetch($input, $this->accessor);
         }
 
         if (self::TYPE_ARRAY_DIMENSION === $this->type) {
-            return new Expr\ArrayDimFetch($input, new Scalar\String_($this->name));
+            return new Expr\ArrayDimFetch($input, new Scalar\String_($this->accessor));
         }
 
         if (self::TYPE_SOURCE === $this->type) {
@@ -118,19 +128,25 @@ public function getExpression(Expr\Variable $input): Expr
      */
     public function getExtractCallback(string $className): ?Expr
     {
-        if (self::TYPE_PROPERTY !== $this->type || !$this->private) {
+        if (!\in_array($this->type, [self::TYPE_PROPERTY, self::TYPE_METHOD]) || !$this->private) {
             return null;
         }
 
         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\PropertyFetch(new Expr\Variable('object'), $this->name)),
-                ],
-            ])),
+            new Arg(
+                new Expr\Closure([
+                    'params' => [
+                        new Param(new Expr\Variable('object')),
+                    ],
+                    'stmts' => [
+                        new Stmt\Return_(
+                            $this->type === self::TYPE_PROPERTY
+                                ? new Expr\PropertyFetch(new Expr\Variable('object'), $this->accessor)
+                                : new Expr\MethodCall(new Expr\Variable('object'), $this->accessor)
+                        ),
+                    ],
+                ])
+            ),
             new Arg(new Expr\ConstFetch(new Name('null'))),
             new Arg(new Scalar\String_($className)),
         ]);

diff --git a/src/Extractor/SourceTargetMappingExtractor.php b/src/Extractor/SourceTargetMappingExtractor.php
@@ -5,6 +5,7 @@
 namespace AutoMapper\Extractor;
 
 use AutoMapper\MapperMetadataInterface;
+use Symfony\Component\PropertyInfo\PropertyReadInfo;
 
 /**
  * Extracts mapping between two objects, only gives properties that have the same name.
@@ -77,7 +78,8 @@ public function getPropertiesMapping(MapperMetadataInterface $mapperMetadata): a
                     $this->getGroups($mapperMetadata->getTarget(), $property),
                     $maxDepth,
                     $this->isIgnoredProperty($mapperMetadata->getSource(), $property),
-                    $this->isIgnoredProperty($mapperMetadata->getTarget(), $property)
+                    $this->isIgnoredProperty($mapperMetadata->getTarget(), $property),
+                    PropertyReadInfo::VISIBILITY_PUBLIC === $this->readInfoExtractor->getReadInfo($mapperMetadata->getSource(), $property)?->getVisibility() ?? true,
                 );
             }
         }

diff --git a/src/Generator/Generator.php b/src/Generator/Generator.php
@@ -154,7 +154,7 @@ public function generate(MapperGeneratorMetadataInterface $mapperGeneratorMetada
         $duplicatedStatements = [];
         $setterStatements = [];
         foreach ($propertiesMapping as $propertyMapping) {
-            if ($propertyMapping->shouldIgnoreProperty()) {
+            if ($propertyMapping->shouldIgnoreProperty($mapperGeneratorMetadata->shouldMapPrivateProperties())) {
                 continue;
             }
 

diff --git a/src/MapperGeneratorMetadataFactory.php b/src/MapperGeneratorMetadataFactory.php
@@ -22,6 +22,7 @@ public function __construct(
         private string $classPrefix = 'Mapper_',
         private bool $attributeChecking = true,
         private string $dateTimeFormat = \DateTime::RFC3339,
+        private bool $mapPrivateProperties = true,
     ) {
     }
 
@@ -40,7 +41,7 @@ public function create(MapperGeneratorMetadataRegistryInterface $autoMapperRegis
             $extractor = $this->fromSourcePropertiesMappingExtractor;
         }
 
-        $mapperMetadata = new MapperMetadata($autoMapperRegister, $extractor, $source, $target, $this->isReadOnly($target), $this->classPrefix);
+        $mapperMetadata = new MapperMetadata($autoMapperRegister, $extractor, $source, $target, $this->isReadOnly($target), $this->mapPrivateProperties, $this->classPrefix);
         $mapperMetadata->setAttributeChecking($this->attributeChecking);
         $mapperMetadata->setDateTimeFormat($this->dateTimeFormat);
 

diff --git a/src/MapperGeneratorMetadataInterface.php b/src/MapperGeneratorMetadataInterface.php
@@ -54,4 +54,9 @@ public function isTargetCloneable(): bool;
      * If not the case, allow to not generate code about circular references
      */
     public function canHaveCircularReference(): bool;
+
+    /**
+     * Whether we should map private properties and methods.
+     */
+    public function shouldMapPrivateProperties(): bool;
 }
diff --git a/src/MapperMetadata.php b/src/MapperMetadata.php
@@ -35,6 +35,7 @@ public function __construct(
         private readonly string $source,
         private readonly string $target,
         private readonly bool $isTargetReadOnlyClass,
+        private readonly bool $mapPrivateProperties,
         private readonly string $classPrefix = 'Mapper_',
     ) {
         $this->isConstructorAllowed = true;
@@ -266,4 +267,9 @@ public function isTargetReadOnlyClass(): bool
     {
         return $this->isTargetReadOnlyClass;
     }
+
+    public function shouldMapPrivateProperties(): bool
+    {
+        return $this->mapPrivateProperties;
+    }
 }
diff --git a/tests/AutoMapperBaseTest.php b/tests/AutoMapperBaseTest.php
@@ -33,7 +33,7 @@ protected function setUp(): void
         $this->buildAutoMapper();
     }
 
-    protected function buildAutoMapper(bool $allowReadOnlyTargetToPopulate = false): AutoMapper
+    protected function buildAutoMapper(bool $allowReadOnlyTargetToPopulate = false, bool $mapPrivatePropertiesAndMethod = false, string $classPrefix = 'Mapper_'): AutoMapper
     {
         $fs = new Filesystem();
         $fs->remove(__DIR__ . '/cache/');
@@ -45,6 +45,6 @@ protected function buildAutoMapper(bool $allowReadOnlyTargetToPopulate = false):
             $allowReadOnlyTargetToPopulate
         ), __DIR__ . '/cache');
 
-        return $this->autoMapper = AutoMapper::create(true, $this->loader);
+        return $this->autoMapper = AutoMapper::create($mapPrivatePropertiesAndMethod, $this->loader, classPrefix: $classPrefix);
     }
 }