Merge branch 'fix/#882-compat-of-public-typed-properties-and-lazy-loa…

…ding' Fixes #881 Fixes #882
doctrine · Jan 10, 2020 · 2053eaf · 2053eaf
2 parents cb3944b + 6f11a7c
commit 2053eaf
Show file tree

Hide file tree

Showing 7 changed files with 1,066 additions and 38 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -7,10 +7,10 @@ cache:
     - $HOME/.composer/cache
 
 php:
-  - 7.1
-  - 7.2
+  - 7.4
   - 7.3
-  - 7.4snapshot
+  - 7.2
+  - 7.1
 
 before_install:
   - mv ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/xdebug.ini{,.disabled} || echo "xdebug not available"

diff --git a/lib/Doctrine/Common/Proxy/ProxyGenerator.php b/lib/Doctrine/Common/Proxy/ProxyGenerator.php
@@ -5,6 +5,8 @@
 use Doctrine\Common\Proxy\Exception\InvalidArgumentException;
 use Doctrine\Common\Proxy\Exception\UnexpectedValueException;
 use Doctrine\Common\Util\ClassUtils;
+use function array_map;
+use function method_exists;
 
 /**
  * This factory is used to generate proxy classes.
@@ -85,12 +87,16 @@ class <proxyShortClassName> extends \<className> implements \<baseProxyInterface
     public $__isInitialized__ = false;
 
     /**
-     * @var array properties to be lazy loaded, with keys being the property
-     *            names and values being their default values
+     * @var array<string, null> properties to be lazy loaded, indexed by property name
+     */
+    public static $lazyPropertiesNames = <lazyPropertiesNames>;
+
+    /**
+     * @var array<string, mixed> default values of properties to be lazy loaded, with keys being the property names
      *
      * @see \Doctrine\Common\Proxy\Proxy::__getLazyProperties
      */
-    public static $lazyPropertiesDefaults = [<lazyPropertiesDefaults>];
+    public static $lazyPropertiesDefaults = <lazyPropertiesDefaults>;
 
 <additionalProperties>
 
@@ -173,6 +179,7 @@ public function __getCloner()
     /**
      * {@inheritDoc}
      * @internal generated method: use only when explicitly handling proxy specific loading logic
+     * @deprecated no longer in use - generated code now relies on internal components rather than generated public API
      * @static
      */
     public function __getLazyProperties()
@@ -358,16 +365,28 @@ private function generateClassName(ClassMetadata $class)
      *
      * @return string
      */
-    private function generateLazyPropertiesDefaults(ClassMetadata $class)
+    private function generateLazyPropertiesNames(ClassMetadata $class)
     {
-        $lazyPublicProperties = $this->getLazyLoadedPublicProperties($class);
+        $lazyPublicProperties = $this->getLazyLoadedPublicPropertiesNames($class);
         $values               = [];
 
-        foreach ($lazyPublicProperties as $key => $value) {
-            $values[] = var_export($key, true) . ' => ' . var_export($value, true);
+        foreach ($lazyPublicProperties as $name) {
+            $values[$name] = null;
         }
 
-        return implode(', ', $values);
+        return var_export($values, true);
+    }
+
+    /**
+     * Generates the array representation of lazy loaded public properties names.
+     *
+     * @param \Doctrine\Common\Persistence\Mapping\ClassMetadata $class
+     *
+     * @return string
+     */
+    private function generateLazyPropertiesDefaults(ClassMetadata $class)
+    {
+        return var_export($this->getLazyLoadedPublicProperties($class), true);
     }
 
     /**
@@ -380,21 +399,16 @@ private function generateLazyPropertiesDefaults(ClassMetadata $class)
     private function generateConstructorImpl(ClassMetadata $class)
     {
         $constructorImpl = <<<'EOT'
-    /**
-     * @param \Closure $initializer
-     * @param \Closure $cloner
-     */
-    public function __construct($initializer = null, $cloner = null)
+    public function __construct(?\Closure $initializer = null, ?\Closure $cloner = null)
     {
 
 EOT;
-        $toUnset         = [];
 
-        foreach ($this->getLazyLoadedPublicProperties($class) as $lazyPublicProperty => $unused) {
-            $toUnset[] = '$this->' . $lazyPublicProperty;
-        }
+        $toUnset = array_map(static function (string $name) : string {
+            return '$this->' . $name;
+        }, $this->getLazyLoadedPublicPropertiesNames($class));
 
-        $constructorImpl .= (empty($toUnset) ? '' : '        unset(' . implode(', ', $toUnset) . ");\n")
+        $constructorImpl .= ($toUnset === [] ? '' : '        unset(' . implode(', ', $toUnset) . ");\n")
             . <<<'EOT'
 
         $this->__initializer__ = $initializer;
@@ -414,7 +428,7 @@ public function __construct($initializer = null, $cloner = null)
      */
     private function generateMagicGet(ClassMetadata $class)
     {
-        $lazyPublicProperties = array_keys($this->getLazyLoadedPublicProperties($class));
+        $lazyPublicProperties = $this->getLazyLoadedPublicPropertiesNames($class);
         $reflectionClass      = $class->getReflectionClass();
         $hasParentGet         = false;
         $returnReference      = '';
@@ -445,7 +459,7 @@ public function {$returnReference}__get(\$name)
 
         if ( ! empty($lazyPublicProperties)) {
             $magicGet .= <<<'EOT'
-        if (array_key_exists($name, $this->__getLazyProperties())) {
+        if (\array_key_exists($name, self::$lazyPropertiesNames)) {
             $this->__initializer__ && $this->__initializer__->__invoke($this, '__get', [$name]);
 
             return $this->$name;
@@ -483,7 +497,7 @@ public function {$returnReference}__get(\$name)
      */
     private function generateMagicSet(ClassMetadata $class)
     {
-        $lazyPublicProperties = $this->getLazyLoadedPublicProperties($class);
+        $lazyPublicProperties = $this->getLazyLoadedPublicPropertiesNames($class);
         $hasParentSet         = $class->getReflectionClass()->hasMethod('__set');
 
         if (empty($lazyPublicProperties) && ! $hasParentSet) {
@@ -504,7 +518,7 @@ public function __set(\$name, \$value)
 
         if ( ! empty($lazyPublicProperties)) {
             $magicSet .= <<<'EOT'
-        if (array_key_exists($name, $this->__getLazyProperties())) {
+        if (\array_key_exists($name, self::$lazyPropertiesNames)) {
             $this->__initializer__ && $this->__initializer__->__invoke($this, '__set', [$name, $value]);
 
             $this->$name = $value;
@@ -540,7 +554,7 @@ public function __set(\$name, \$value)
      */
     private function generateMagicIsset(ClassMetadata $class)
     {
-        $lazyPublicProperties = array_keys($this->getLazyLoadedPublicProperties($class));
+        $lazyPublicProperties = $this->getLazyLoadedPublicPropertiesNames($class);
         $hasParentIsset       = $class->getReflectionClass()->hasMethod('__isset');
 
         if (empty($lazyPublicProperties) && ! $hasParentIsset) {
@@ -561,7 +575,7 @@ public function __isset(\$name)
 
         if ( ! empty($lazyPublicProperties)) {
             $magicIsset .= <<<'EOT'
-        if (array_key_exists($name, $this->__getLazyProperties())) {
+        if (\array_key_exists($name, self::$lazyPropertiesNames)) {
             $this->__initializer__ && $this->__initializer__->__invoke($this, '__isset', [$name]);
 
             return isset($this->$name);
@@ -611,7 +625,7 @@ public function __sleep()
         $properties = array_merge(['__isInitialized__'], parent::__sleep());
 
         if ($this->__isInitialized__) {
-            $properties = array_diff($properties, array_keys($this->__getLazyProperties()));
+            $properties = array_diff($properties, array_keys(self::$lazyPropertiesNames));
         }
 
         return $properties;
@@ -632,7 +646,7 @@ public function __sleep()
                 : $prop->getName();
         }
 
-        $lazyPublicProperties = array_keys($this->getLazyLoadedPublicProperties($class));
+        $lazyPublicProperties = $this->getLazyLoadedPublicPropertiesNames($class);
         $protectedProperties  = array_diff($allProperties, $lazyPublicProperties);
 
         foreach ($allProperties as &$property) {
@@ -668,7 +682,7 @@ private function generateWakeupImpl(ClassMetadata $class)
         $unsetPublicProperties = [];
         $hasWakeup             = $class->getReflectionClass()->hasMethod('__wakeup');
 
-        foreach (array_keys($this->getLazyLoadedPublicProperties($class)) as $lazyPublicProperty) {
+        foreach ($this->getLazyLoadedPublicPropertiesNames($class) as $lazyPublicProperty) {
             $unsetPublicProperties[] = '$this->' . $lazyPublicProperty;
         }
 
@@ -687,7 +701,7 @@ public function __wakeup()
 
                 \$existingProperties = get_object_vars(\$proxy);
 
-                foreach (\$proxy->__getLazyProperties() as \$property => \$defaultValue) {
+                foreach (\$proxy::\$lazyPropertiesDefaults as \$property => \$defaultValue) {
                     if ( ! array_key_exists(\$property, \$existingProperties)) {
                         \$proxy->\$property = \$defaultValue;
                     }
@@ -870,28 +884,60 @@ private function isShortIdentifierGetter($method, ClassMetadata $class)
     }
 
     /**
-     * Generates the list of public properties to be lazy loaded, with their default values.
+     * Generates the list of public properties to be lazy loaded.
      *
      * @param \Doctrine\Common\Persistence\Mapping\ClassMetadata $class
      *
-     * @return mixed[]
+     * @return array<int, string>
      */
-    private function getLazyLoadedPublicProperties(ClassMetadata $class)
+    private function getLazyLoadedPublicPropertiesNames(ClassMetadata $class) : array
     {
-        $defaultProperties = $class->getReflectionClass()->getDefaultProperties();
-        $properties        = [];
+        $properties = [];
 
         foreach ($class->getReflectionClass()->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) {
             $name = $property->getName();
 
             if (($class->hasField($name) || $class->hasAssociation($name)) && ! $class->isIdentifier($name)) {
-                $properties[$name] = $defaultProperties[$name];
+                $properties[] = $name;
             }
         }
 
         return $properties;
     }
 
+    /**
+     * Generates the list of default values of public properties.
+     *
+     * @param \Doctrine\Common\Persistence\Mapping\ClassMetadata $class
+     *
+     * @return mixed[]
+     */
+    private function getLazyLoadedPublicProperties(ClassMetadata $class)
+    {
+        $defaultProperties          = $class->getReflectionClass()->getDefaultProperties();
+        $lazyLoadedPublicProperties = $this->getLazyLoadedPublicPropertiesNames($class);
+        $defaultValues              = [];
+
+        foreach ($class->getReflectionClass()->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) {
+            $name = $property->getName();
+
+            if ( ! in_array($name, $lazyLoadedPublicProperties, true)) {
+                continue;
+            }
+
+            if (array_key_exists($name, $defaultProperties)) {
+                $defaultValues[$name] = $defaultProperties[$name];
+            } elseif (method_exists($property, 'getType')) {
+                $propertyType = $property->getType();
+                if (null !== $propertyType && $propertyType->allowsNull()) {
+                    $defaultValues[$name] = null;
+                }
+            }
+        }
+
+        return $defaultValues;
+    }
+
     /**
      * @param \ReflectionParameter[] $parameters
      *

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -11,6 +11,8 @@ parameters:
         - %currentWorkingDirectory%/tests/Doctrine/Tests/Common/Proxy/InvalidTypeHintClass.php
         - %currentWorkingDirectory%/tests/Doctrine/Tests/Common/Proxy/SerializedClass.php
         - %currentWorkingDirectory%/tests/Doctrine/Tests/Common/Proxy/VariadicTypeHintClass.php
+        - %currentWorkingDirectory%/tests/Doctrine/Tests/Common/Proxy/ProxyLogicTypedPropertiesTest.php
+        - %currentWorkingDirectory%/tests/Doctrine/Tests/Common/Proxy/LazyLoadableObjectWithTypedProperties.php
     ignoreErrors:
         - '#Access to an undefined property Doctrine\\Common\\Proxy\\Proxy::\$publicField#'
         -

diff --git a/tests/Doctrine/Tests/Common/Proxy/LazyLoadableObjectWithTypedProperties.php b/tests/Doctrine/Tests/Common/Proxy/LazyLoadableObjectWithTypedProperties.php
@@ -0,0 +1,67 @@
+<?php
+namespace Doctrine\Tests\Common\Proxy;
+
+/**
+ * Test asset representing a lazy loadable object with typed properties
+ */
+class LazyLoadableObjectWithTypedProperties
+{
+    public string $publicIdentifierField;
+
+    protected string $protectedIdentifierField;
+
+    public string $publicTransientField = 'publicTransientFieldValue';
+
+    protected string $protectedTransientField = 'protectedTransientFieldValue';
+
+    public ?string $publicPersistentField = 'publicPersistentFieldValue';
+
+    protected string $protectedPersistentField = 'protectedPersistentFieldValue';
+
+    public string $publicAssociation = 'publicAssociationValue';
+
+    protected string $protectedAssociation = 'protectedAssociationValue';
+
+    /**
+     * @return string
+     */
+    public function getProtectedIdentifierField()
+    {
+        return $this->protectedIdentifierField;
+    }
+
+    /**
+     * @return string
+     */
+    public function testInitializationTriggeringMethod()
+    {
+        return 'testInitializationTriggeringMethod';
+    }
+
+    /**
+     * @return string
+     */
+    public function getProtectedAssociation()
+    {
+        return $this->protectedAssociation;
+    }
+
+    /**
+     * @param \stdClass $param
+     */
+    public function publicTypeHintedMethod(\stdClass $param)
+    {
+    }
+
+    public function &byRefMethod()
+    {
+    }
+
+    /**
+     * @param mixed $thisIsNotByRef
+     * @param mixed $thisIsByRef
+     */
+    public function byRefParamMethod($thisIsNotByRef, &$thisIsByRef)
+    {
+    }
+}