Merge pull request #37 from doctrine/GH-36-BugfixTypedNullable

[GH-36] Bugfix: TypedNoDefaultReflectionProperty::setValue NULL when null allowed

.doctrine-project.json

@@ -0,0 +1,30 @@
+{
+    "active": true,
+    "name": "Reflection",
+    "slug": "reflection",
+    "docsSlug": "doctrine-reflection",
+    "versions": [
+        {
+            "name": "1.2",
+            "branchName": "master",
+            "slug": "latest",
+            "upcoming": true
+        },
+        {
+            "name": "1.1",
+            "branchName": "1.1.x",
+            "slug": "1.1",
+            "current": true,
+            "aliases": [
+                "current",
+                "stable"
+            ]
+        },
+        {
+            "name": "1.0",
+            "branchName": "1.0",
+            "slug": "1.0",
+            "maintained": false
+        }
+    ]
+}

.gitattributes

@@ -1,3 +1,4 @@
+/docs export-ignore
 /tests export-ignore
 /.gitattributes export-ignore
 /.gitignore export-ignore

.scrutinizer.yml

@@ -16,7 +16,7 @@ build:
                     - phpcs-run
     dependencies:
         override:
-            - composer install -noa
+            - COMPOSER_ROOT_VERSION=1.2 composer install -noa
 
 tools:
     external_code_coverage:

.travis.yml

@@ -2,6 +2,10 @@ dist: trusty
 sudo: false
 language: php
 
+env:
+  global:
+    - COMPOSER_ROOT_VERSION=1.2
+
 cache:
   directories:
     - $HOME/.composer/cache
@@ -9,6 +13,8 @@ cache:
 php:
   - 7.1
   - 7.2
+  - 7.3
+  - 7.4
   - nightly
 
 before_install:
@@ -42,4 +48,5 @@ jobs:
         - ./vendor/bin/phpcs
 
     - stage: Lint
-      script: vendor/bin/phpstan analyse -l 3 -c phpstan.neon lib tests
+      php: 7.4
+      script: vendor/bin/phpstan analyse

composer.json

@@ -1,8 +1,11 @@
 {
     "name": "doctrine/reflection",
     "type": "library",
-    "description": "Doctrine Reflection component",
-    "keywords": ["reflection"],
+    "description": "The Doctrine Reflection project is a simple library used by the various Doctrine projects which adds some additional functionality on top of the reflection functionality that comes with PHP. It allows you to get the reflection information about classes, methods and properties statically.",
+    "keywords": [
+        "reflection",
+        "static"
+    ],
     "homepage": "https://www.doctrine-project.org/projects/reflection.html",
     "license": "MIT",
     "authors": [
@@ -19,12 +22,14 @@
         "doctrine/annotations": "^1.0"
     },
     "require-dev": {
-        "phpstan/phpstan": "^0.9.2",
-        "phpstan/phpstan-phpunit": "^0.9.4",
+        "phpstan/phpstan": "^0.11.0",
+        "phpstan/phpstan-phpunit": "^0.11.0",
         "phpunit/phpunit": "^7.0",
-        "doctrine/coding-standard": "^4.0",
-        "doctrine/common": "^2.8",
-        "squizlabs/php_codesniffer": "^3.0"
+        "doctrine/coding-standard": "^5.0",
+        "doctrine/common": "^2.10"
+    },
+    "conflict": {
+        "doctrine/common": "<2.9"
     },
     "autoload": {
         "psr-4": {
@@ -33,12 +38,13 @@
     },
     "autoload-dev": {
         "psr-4": {
-            "Doctrine\\Tests\\": "tests/Doctrine/Tests"
+            "Doctrine\\Tests\\": "tests/Doctrine/Tests",
+            "Doctrine\\Tests_PHP74\\": "tests/Doctrine/Tests_PHP74"
         }
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "1.0.x-dev"
+            "dev-master": "1.2.x-dev"
         }
     }
 }

docs/en/index.rst

@@ -11,7 +11,7 @@ If this documentation is not helping to answer questions you have about
 Doctrine Reflection don't panic. You can get help from different sources:
 
 -  The `Doctrine Mailing List <https://groups.google.com/group/doctrine-user>`_
--  Gitter chat room `#doctrine/reflection <https://gitter.im/doctrine/reflection>`_
+-  Slack chat room `#reflection <https://www.doctrine-project.org/slack>`_
 -  Report a bug on `GitHub <https://github.com/doctrine/reflection/issues>`_.
 -  On `StackOverflow <https://stackoverflow.com/questions/tagged/doctrine-reflection>`_
 

lib/Doctrine/Common/Reflection/ReflectionProviderInterface.php

@@ -2,12 +2,16 @@
 
 namespace Doctrine\Common\Reflection;
 
+use ReflectionClass;
+use ReflectionMethod;
+use ReflectionProperty;
+
 interface ReflectionProviderInterface
 {
     /**
      * Gets the ReflectionClass equivalent for this class.
      *
-     * @return \ReflectionClass
+     * @return ReflectionClass
      */
     public function getReflectionClass();
 
@@ -16,7 +20,7 @@ interface ReflectionProviderInterface
      *
      * @param string $name
      *
-     * @return \ReflectionMethod
+     * @return ReflectionMethod
      */
     public function getReflectionMethod($name);
 
@@ -25,7 +29,7 @@ interface ReflectionProviderInterface
      *
      * @param string $name
      *
-     * @return \ReflectionProperty
+     * @return ReflectionProperty
      */
     public function getReflectionProperty($name);
 }

lib/Doctrine/Common/Reflection/RuntimePublicReflectionProperty.php

@@ -7,7 +7,6 @@ use ReflectionProperty;
 
 /**
  * PHP Runtime Reflection Public Property - special overrides for public properties.
- *
  */
 class RuntimePublicReflectionProperty extends ReflectionProperty
 {
@@ -39,6 +38,7 @@ class RuntimePublicReflectionProperty extends ReflectionProperty
      *
      * Avoids triggering lazy loading via `__set` if the provided object
      * is a {@see \Doctrine\Common\Proxy\Proxy}.
+     *
      * @link https://bugs.php.net/bug.php?id=63463
      */
     public function setValue($object, $value = null)

lib/Doctrine/Common/Reflection/StaticReflectionMethod.php

@@ -89,7 +89,7 @@ class StaticReflectionMethod extends ReflectionMethod
     /**
      * {@inheritDoc}
      */
-    public function getClosure($object)
+    public function getClosure($object = null)
     {
         throw new ReflectionException('Method not implemented');
     }

lib/Doctrine/Common/Reflection/StaticReflectionParser.php

@@ -8,6 +8,7 @@ use const T_CLASS;
 use const T_DOC_COMMENT;
 use const T_EXTENDS;
 use const T_FUNCTION;
+use const T_NEW;
 use const T_PAAMAYIM_NEKUDOTAYIM;
 use const T_PRIVATE;
 use const T_PROTECTED;
@@ -161,7 +162,7 @@ class StaticReflectionParser implements ReflectionProviderInterface
                     $docComment = $token[1];
                     break;
                 case T_CLASS:
-                    if ($last_token !== T_PAAMAYIM_NEKUDOTAYIM) {
+                    if ($last_token !== T_PAAMAYIM_NEKUDOTAYIM && $last_token !== T_NEW) {
                         $this->docComment['class'] = $docComment;
                         $docComment                = '';
                     }

lib/Doctrine/Common/Reflection/TypedNoDefaultReflectionProperty.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace Doctrine\Common\Reflection;
+
+use ReflectionProperty;
+
+/**
+ * PHP Typed No Default Reflection Property - special override for typed properties without a default value.
+ */
+class TypedNoDefaultReflectionProperty extends ReflectionProperty
+{
+    /**
+     * {@inheritDoc}
+     *
+     * Checks that a typed property is initialized before accessing its value.
+     * This is neccessary to avoid PHP error "Error: Typed property must not be accessed before initialization".
+     * Should be used only for reflecting typed properties without a default value.
+     */
+    public function getValue($object = null)
+    {
+        return $object !== null && $this->isInitialized($object) ? parent::getValue($object) : null;
+    }
+
+    /**
+     * {@inheritDoc}
+     *
+     * Works around the problem with setting typed no default properties to
+     * NULL which is not supported, instead unset() to uninitialize.
+     *
+     * @link https://github.com/doctrine/orm/issues/7999
+     */
+    public function setValue($object, $value = null)
+    {
+        if ($value === null && $this->hasType() && ! $this->getType()->allowsNull()) {
+            $propertyName = $this->getName();
+
+            $unsetter = function () use ($propertyName) {
+                unset($this->$propertyName);
+            };
+            $unsetter = $unsetter->bindTo($object, $this->getDeclaringClass()->getName());
+            $unsetter();
+
+            return;
+        }
+
+        parent::setValue($object, $value);
+    }
+}

phpstan.neon.dist

@@ -1,6 +1,11 @@
-includes:
-    - vendor/phpstan/phpstan-phpunit/extension.neon
-
 parameters:
+    level: 3
+    paths:
+        - %rootDir%/../../../lib
+        - %rootDir%/../../../tests
+
     ignoreErrors:
         - '#Doctrine\\Common\\Reflection\\StaticReflection[a-zA-Z0-9_]+::__construct\(\) does not call parent constructor from Reflection[a-zA-Z0-9_]+#'
+
+includes:
+    - vendor/phpstan/phpstan-phpunit/extension.neon

phpunit.xml.dist

@@ -8,7 +8,8 @@
 >
     <testsuites>
         <testsuite name="Doctrine Event Manager Test Suite">
-            <directory>./tests/Doctrine/</directory>
+            <directory>./tests/Doctrine/Tests</directory>
+            <directory phpVersion="7.4" phpVersionOperator=">=">./tests/Doctrine/Tests_PHP74</directory>
         </testsuite>
     </testsuites>
 

tests/Doctrine/Tests/Common/Reflection/AnnotationClassWithAnonymousClass.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace Doctrine\Tests\Common\Reflection;
+
+use Doctrine\Common\Annotations\Annotation;
+
+/**
+ * @Annotation(
+ *   key = "value"
+ * )
+ */
+class AnnotationClassWithAnonymousClass
+{
+    public function foo()
+    {
+        $new_class = new class () {
+        };
+    }
+}

tests/Doctrine/Tests/Common/Reflection/AnnotationClassWithScopeResolution.php

@@ -3,6 +3,7 @@
 namespace Doctrine\Tests\Common\Reflection;
 
 use Doctrine\Common\Annotations\Annotation;
+use stdClass;
 
 /**
  * @Annotation(
@@ -11,10 +12,10 @@ use Doctrine\Common\Annotations\Annotation;
  */
 class AnnotationClassWithScopeResolution
 {
-    public const FOO = \stdClass::class;
+    public const FOO = stdClass::class;
 
   /**
    * Example with comment.
    */
-    public const BAR = \stdClass::class;
+    public const BAR = stdClass::class;
 }

tests/Doctrine/Tests/Common/Reflection/Psr0FindFileTest.php

@@ -4,6 +4,7 @@ namespace Doctrine\Tests\Common\Reflection;
 
 use Doctrine\Common\Reflection\Psr0FindFile;
 use PHPUnit\Framework\TestCase;
+use const DIRECTORY_SEPARATOR;
 use function sprintf;
 use function strlen;
 use function substr;
@@ -17,7 +18,7 @@ class Psr0FindFileTest extends TestCase
     {
         $file = $this->psr0FindFile->findFile(NoParent::class);
 
-        self::assertSame(sprintf('%s/NoParent.php', __DIR__), $file);
+        self::assertSame(sprintf('%s%sNoParent.php', __DIR__, DIRECTORY_SEPARATOR), $file);
     }
 
     public function testFindFileNotFound() : void
@@ -29,14 +30,14 @@ class Psr0FindFileTest extends TestCase
     {
         $file = $this->psr0FindFile->findFile('\Doctrine\Tests\Common\Reflection\NoParent');
 
-        self::assertSame(sprintf('%s/NoParent.php', __DIR__), $file);
+        self::assertSame(sprintf('%s%sNoParent.php', __DIR__, DIRECTORY_SEPARATOR), $file);
     }
 
     public function testFindFileFromPearLikeClassName() : void
     {
         $file = $this->psr0FindFile->findFile('Doctrine_Tests_Common_Reflection_NoParent');
 
-        self::assertSame(sprintf('%s/NoParent.php', __DIR__), $file);
+        self::assertSame(sprintf('%s%sNoParent.php', __DIR__, DIRECTORY_SEPARATOR), $file);
     }
 
     protected function setUp() : void

tests/Doctrine/Tests/Common/Reflection/RuntimePublicReflectionPropertyTest.php

@@ -2,6 +2,7 @@
 
 namespace Doctrine\Tests\Common\Reflection;
 
+use Closure;
 use Doctrine\Common\Proxy\Proxy;
 use Doctrine\Common\Reflection\RuntimePublicReflectionProperty;
 use PHPUnit\Framework\TestCase;
@@ -39,7 +40,7 @@ class RuntimePublicReflectionPropertyTest extends TestCase
     {
         $getCheckMock = $this->getMockBuilder('stdClass')->setMethods(['callGet'])->getMock();
         $getCheckMock->expects($this->never())->method('callGet');
-        $initializer = function () use ($getCheckMock) {
+        $initializer = static function () use ($getCheckMock) {
             call_user_func($getCheckMock);
         };
 
@@ -60,7 +61,7 @@ class RuntimePublicReflectionPropertyTest extends TestCase
     {
         $setCheckMock = $this->getMockBuilder('stdClass')->setMethods(['neverCallSet'])->getMock();
         $setCheckMock->expects($this->never())->method('neverCallSet');
-        $initializer = function () use ($setCheckMock) {
+        $initializer = static function () use ($setCheckMock) {
             call_user_func([$setCheckMock, 'neverCallSet']);
         };
 
@@ -81,7 +82,7 @@ class RuntimePublicReflectionPropertyTest extends TestCase
 
         $setCheckMock = $this->getMockBuilder('stdClass')->setMethods(['callSet'])->getMock();
         $setCheckMock->expects($this->once())->method('callSet');
-        $initializer = function () use ($setCheckMock) {
+        $initializer = static function () use ($setCheckMock) {
             call_user_func([$setCheckMock, 'callSet']);
         };
 
@@ -99,7 +100,7 @@ class RuntimePublicReflectionPropertyTest extends TestCase
  */
 class RuntimePublicReflectionPropertyTestProxyMock implements Proxy
 {
-    /** @var \Closure|null */
+    /** @var Closure|null */
     private $initializer = null;
 
     /** @var bool */
@@ -119,7 +120,7 @@ class RuntimePublicReflectionPropertyTestProxyMock implements Proxy
     /**
      * {@inheritDoc}
      */
-    public function __setInitializer(?\Closure $initializer = null)
+    public function __setInitializer(?Closure $initializer = null)
     {
         $this->initializer = $initializer;
     }
@@ -200,7 +201,7 @@ class RuntimePublicReflectionPropertyTestProxyMock implements Proxy
     /**
      * {@inheritDoc}
      */
-    public function __setCloner(?\Closure $cloner = null)
+    public function __setCloner(?Closure $cloner = null)
     {
     }
 

tests/Doctrine/Tests/Common/Reflection/StaticReflectionClassTest.php

@@ -56,7 +56,7 @@ class StaticReflectionClassTest extends TestCase
 
     public function testGetMethod() : void
     {
-        $staticReflectionMethod = $this->createMock(StaticReflectionMethod::class);
+        $staticReflectionMethod = $this->createPartialMock(StaticReflectionMethod::class, []);
 
         $this->staticReflectionParser->expects($this->once())
             ->method('getReflectionMethod')
@@ -79,9 +79,9 @@ class StaticReflectionClassTest extends TestCase
     }
 
     /**
-     * @dataProvider getNotImplementedMethods
-     *
      * @param mixed[] $args
+     *
+     * @dataProvider getNotImplementedMethods
      */
     public function testNotImplemented(string $method, array $args) : void
     {

tests/Doctrine/Tests/Common/Reflection/StaticReflectionMethodTest.php

@@ -27,7 +27,7 @@ class StaticReflectionMethodTest extends TestCase
 
     public function testGetDeclaringClass() : void
     {
-        $staticReflectionClass = $this->createMock(StaticReflectionClass::class);
+        $staticReflectionClass = $this->createPartialMock(StaticReflectionClass::class, []);
 
         $this->staticReflectionParser->expects($this->once())
             ->method('getReflectionClass')
@@ -47,7 +47,7 @@ class StaticReflectionMethodTest extends TestCase
 
     public function testGetDocComment() : void
     {
-        $staticReflectionClass = $this->createMock(StaticReflectionClass::class);
+        $staticReflectionClass = $this->createPartialMock(StaticReflectionClass::class, []);
 
         $this->staticReflectionParser->expects($this->once())
             ->method('getDocComment')
@@ -59,7 +59,7 @@ class StaticReflectionMethodTest extends TestCase
 
     public function testGetUseStatements() : void
     {
-        $staticReflectionClass = $this->createMock(StaticReflectionClass::class);
+        $staticReflectionClass = $this->createPartialMock(StaticReflectionClass::class, []);
 
         $this->staticReflectionParser->expects($this->once())
             ->method('getUseStatements')
@@ -69,9 +69,9 @@ class StaticReflectionMethodTest extends TestCase
     }
 
     /**
-     * @dataProvider getNotImplementedMethods
-     *
      * @param mixed[] $args
+     *
+     * @dataProvider getNotImplementedMethods
      */
     public function testNotImplemented(string $method, array $args) : void
     {

tests/Doctrine/Tests/Common/Reflection/StaticReflectionParserTest.php

@@ -13,13 +13,13 @@ use function substr;
 class StaticReflectionParserTest extends DoctrineTestCase
 {
     /**
-     * @dataProvider parentClassData
-     *
      * @param bool   $classAnnotationOptimize
      * @param string $parsedClassName
      * @param string $expectedClassName
      *
      * @return void
+     *
+     * @dataProvider parentClassData
      */
     public function testParentClass($classAnnotationOptimize, $parsedClassName, $expectedClassName)
     {
@@ -89,7 +89,7 @@ class StaticReflectionParserTest extends DoctrineTestCase
     {
         $testsRoot              = substr(__DIR__, 0, -strlen(__NAMESPACE__) - 1);
         $paths                  = [
-          'Doctrine\\Tests' => [$testsRoot],
+            'Doctrine\\Tests' => [$testsRoot],
         ];
         $staticReflectionParser = new StaticReflectionParser($class, new Psr0FindFile($paths), $classAnnotationOptimize);
         $expectedDocComment     = '/**
@@ -101,7 +101,7 @@ class StaticReflectionParserTest extends DoctrineTestCase
     }
 
     /**
-     * @return string[]|bool[]
+     * @return array<array<bool|string>>
      */
     public function classAnnotationOptimize()
     {
@@ -110,6 +110,8 @@ class StaticReflectionParserTest extends DoctrineTestCase
             [ExampleAnnotationClass::class, true],
             [AnnotationClassWithScopeResolution::class, false],
             [AnnotationClassWithScopeResolution::class, true],
+            [AnnotationClassWithAnonymousClass::class, false],
+            [AnnotationClassWithAnonymousClass::class, true],
         ];
     }
 }

tests/Doctrine/Tests/Common/Reflection/StaticReflectionPropertyTest.php

@@ -27,7 +27,7 @@ class StaticReflectionPropertyTest extends TestCase
 
     public function testGetDeclaringClass() : void
     {
-        $staticReflectionClass = $this->createMock(StaticReflectionClass::class);
+        $staticReflectionClass = $this->createPartialMock(StaticReflectionClass::class, []);
 
         $this->staticReflectionParser->expects($this->once())
             ->method('getReflectionClass')
@@ -38,7 +38,7 @@ class StaticReflectionPropertyTest extends TestCase
 
     public function testGetDocComment() : void
     {
-        $staticReflectionClass = $this->createMock(StaticReflectionClass::class);
+        $staticReflectionClass = $this->createPartialMock(StaticReflectionClass::class, []);
 
         $this->staticReflectionParser->expects($this->once())
             ->method('getDocComment')
@@ -50,7 +50,7 @@ class StaticReflectionPropertyTest extends TestCase
 
     public function testGetUseStatements() : void
     {
-        $staticReflectionClass = $this->createMock(StaticReflectionClass::class);
+        $staticReflectionClass = $this->createPartialMock(StaticReflectionClass::class, []);
 
         $this->staticReflectionParser->expects($this->once())
             ->method('getUseStatements')
@@ -60,9 +60,9 @@ class StaticReflectionPropertyTest extends TestCase
     }
 
     /**
-     * @dataProvider getNotImplementedMethods
-     *
      * @param mixed[] $args
+     *
+     * @dataProvider getNotImplementedMethods
      */
     public function testNotImplemented(string $method, array $args) : void
     {

tests/Doctrine/Tests/TestInit.php

@@ -14,7 +14,7 @@ use function strtr;
 error_reporting(E_ALL | E_STRICT);
 
 // register silently failing autoloader
-spl_autoload_register(function ($class) {
+spl_autoload_register(static function ($class) {
     if (strpos($class, 'Doctrine\Tests\\') !== 0) {
         return;
     }

tests/Doctrine/Tests_PHP74/Common/Reflection/TypedNoDefaultReflectionPropertyTest.php

@@ -0,0 +1,86 @@
+<?php
+
+namespace Doctrine\Tests_PHP74\Common\Reflection;
+
+use Doctrine\Common\Reflection\TypedNoDefaultReflectionProperty;
+use PHPUnit\Framework\TestCase;
+
+class TypedNoDefaultReflectionPropertyTest extends TestCase
+{
+    public function testGetValue() : void
+    {
+        $object = new TypedNoDefaultReflectionPropertyTestClass();
+
+        $reflProperty = new TypedNoDefaultReflectionProperty(TypedNoDefaultReflectionPropertyTestClass::class, 'test');
+
+        self::assertNull($reflProperty->getValue($object));
+
+        $object->test = 'testValue';
+
+        self::assertSame('testValue', $reflProperty->getValue($object));
+
+        unset($object->test);
+
+        self::assertNull($reflProperty->getValue($object));
+    }
+
+    public function testSetValueNull() : void
+    {
+        $reflection = new TypedNoDefaultReflectionProperty(TypedFoo::class, 'id');
+        $reflection->setAccessible(true);
+
+        $object = new TypedFoo();
+        $object->setId(1);
+
+        self::assertTrue($reflection->isInitialized($object));
+
+        $reflection->setValue($object, null);
+
+        self::assertNull($reflection->getValue($object));
+        self::assertFalse($reflection->isInitialized($object));
+    }
+
+    public function testSetValueNullOnNullableProperty() : void
+    {
+        $reflection = new TypedNoDefaultReflectionProperty(TypedNullableFoo::class, 'value');
+        $reflection->setAccessible(true);
+
+        $object = new TypedNullableFoo();
+
+        $reflection->setValue($object, null);
+
+        self::assertNull($reflection->getValue($object));
+        self::assertTrue($reflection->isInitialized($object));
+        self::assertNull($object->getValue());
+    }
+}
+
+class TypedNoDefaultReflectionPropertyTestClass
+{
+    public string $test;
+}
+
+class TypedFoo
+{
+    private int $id;
+
+    public function setId($id)
+    {
+        $this->id = $id;
+    }
+}
+
+class TypedNullableFoo
+{
+    private ?string $value;
+
+    public function setValue($value)
+    {
+        $this->value = $value;
+    }
+
+    public function getValue()
+    {
+        return $this->value;
+    }
+}