bug #36073 [PropertyAccess][DX] Improved errors when reading uninitia…

…lized properties (HeahDude)

This PR was merged into the 3.4 branch.

Discussion
----------

[PropertyAccess][DX] Improved errors when reading uninitialized properties

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | kinda
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36051
| License       | MIT
| Doc PR        | ~

An attempt to fix #36051 by providing better error messages when trying to read uninitialized properties either via calling a return-type-hinted method from PHP 7.0 or by accessing public-typed properties from PHP 7.4.

It would be nice to have a proper exception class in master.

Commits
-------

a71023b [PropertyAccess] Improved errors when reading uninitialized properties

src/Symfony/Component/PropertyAccess/PropertyAccessor.php

@@ -465,30 +465,50 @@ private function readProperty($zval, $property)
         $object = $zval[self::VALUE];
         $access = $this->getReadAccessInfo(\get_class($object), $property);
 
-        if (self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE]) {
-            $result[self::VALUE] = $object->{$access[self::ACCESS_NAME]}();
-        } elseif (self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]) {
-            $result[self::VALUE] = $object->{$access[self::ACCESS_NAME]};
+        try {
+            if (self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE]) {
+                try {
+                    $result[self::VALUE] = $object->{$access[self::ACCESS_NAME]}();
+                } catch (\TypeError $e) {
+                    // handle uninitialized properties in PHP >= 7
+                    if (preg_match((sprintf('/^Return value of %s::%s\(\) must be of the type (\w+), null returned$/', preg_quote(\get_class($object)), $access[self::ACCESS_NAME])), $e->getMessage(), $matches)) {
+                        throw new AccessException(sprintf('The method "%s::%s()" returned "null", but expected type "%3$s". Have you forgotten to initialize a property or to make the return type nullable using "?%3$s" instead?', \get_class($object), $access[self::ACCESS_NAME], $matches[1]), 0, $e);
+                    }
+
+                    throw $e;
+                }
+            } elseif (self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]) {
+                $result[self::VALUE] = $object->{$access[self::ACCESS_NAME]};
 
-            if ($access[self::ACCESS_REF] && isset($zval[self::REF])) {
-                $result[self::REF] = &$object->{$access[self::ACCESS_NAME]};
+                if ($access[self::ACCESS_REF] && isset($zval[self::REF])) {
+                    $result[self::REF] = &$object->{$access[self::ACCESS_NAME]};
+                }
+            } elseif (!$access[self::ACCESS_HAS_PROPERTY] && property_exists($object, $property)) {
+                // Needed to support \stdClass instances. We need to explicitly
+                // exclude $access[self::ACCESS_HAS_PROPERTY], otherwise if
+                // a *protected* property was found on the class, property_exists()
+                // returns true, consequently the following line will result in a
+                // fatal error.
+
+                $result[self::VALUE] = $object->$property;
+                if (isset($zval[self::REF])) {
+                    $result[self::REF] = &$object->$property;
+                }
+            } elseif (self::ACCESS_TYPE_MAGIC === $access[self::ACCESS_TYPE]) {
+                // we call the getter and hope the __call do the job
+                $result[self::VALUE] = $object->{$access[self::ACCESS_NAME]}();
+            } else {
+                throw new NoSuchPropertyException($access[self::ACCESS_NAME]);
             }
-        } elseif (!$access[self::ACCESS_HAS_PROPERTY] && property_exists($object, $property)) {
-            // Needed to support \stdClass instances. We need to explicitly
-            // exclude $access[self::ACCESS_HAS_PROPERTY], otherwise if
-            // a *protected* property was found on the class, property_exists()
-            // returns true, consequently the following line will result in a
-            // fatal error.
+        } catch (\Error $e) {
+            // handle uninitialized properties in PHP >= 7.4
+            if (\PHP_VERSION_ID >= 70400 && preg_match('/^Typed property ([\w\\\]+)::\$(\w+) must not be accessed before initialization$/', $e->getMessage(), $matches)) {
+                $r = new \ReflectionProperty($matches[1], $matches[2]);
 
-            $result[self::VALUE] = $object->$property;
-            if (isset($zval[self::REF])) {
-                $result[self::REF] = &$object->$property;
+                throw new AccessException(sprintf('The property "%s::$%s" is not readable because it is typed "%3$s". You should either initialize it or make it nullable using "?%3$s" instead.', $r->getDeclaringClass()->getName(), $r->getName(), $r->getType()->getName()), 0, $e);
             }
-        } elseif (self::ACCESS_TYPE_MAGIC === $access[self::ACCESS_TYPE]) {
-            // we call the getter and hope the __call do the job
-            $result[self::VALUE] = $object->{$access[self::ACCESS_NAME]}();
-        } else {
-            throw new NoSuchPropertyException($access[self::ACCESS_NAME]);
+
+            throw $e;
         }
 
         // Objects are always passed around by reference

src/Symfony/Component/PropertyAccess/Tests/Fixtures/UninitializedPrivateProperty.php

@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\PropertyAccess\Tests\Fixtures;
+
+class UninitializedPrivateProperty
+{
+    private $uninitialized;
+
+    public function getUninitialized(): array
+    {
+        return $this->uninitialized;
+    }
+}

src/Symfony/Component/PropertyAccess/Tests/Fixtures/UninitializedProperty.php

@@ -0,0 +1,17 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\PropertyAccess\Tests\Fixtures;
+
+class UninitializedProperty
+{
+    public string $uninitialized;
+}

src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php

@@ -25,6 +25,8 @@
 use Symfony\Component\PropertyAccess\Tests\Fixtures\TestSingularAndPluralProps;
 use Symfony\Component\PropertyAccess\Tests\Fixtures\Ticket5775Object;
 use Symfony\Component\PropertyAccess\Tests\Fixtures\TypeHinted;
+use Symfony\Component\PropertyAccess\Tests\Fixtures\UninitializedPrivateProperty;
+use Symfony\Component\PropertyAccess\Tests\Fixtures\UninitializedProperty;
 
 class PropertyAccessorTest extends TestCase
 {
@@ -118,6 +120,28 @@ public function testGetValueThrowsExceptionIfIndexNotFoundAndIndexExceptionsEnab
         $this->propertyAccessor->getValue($objectOrArray, $path);
     }
 
+    /**
+     * @requires PHP 7.4
+     */
+    public function testGetValueThrowsExceptionIfUninitializedProperty()
+    {
+        $this->expectException('Symfony\Component\PropertyAccess\Exception\AccessException');
+        $this->expectExceptionMessage('The property "Symfony\Component\PropertyAccess\Tests\Fixtures\UninitializedProperty::$uninitialized" is not readable because it is typed "string". You should either initialize it or make it nullable using "?string" instead.');
+
+        $this->propertyAccessor->getValue(new UninitializedProperty(), 'uninitialized');
+    }
+
+    /**
+     * @requires PHP 7
+     */
+    public function testGetValueThrowsExceptionIfUninitializedPropertyWithGetter()
+    {
+        $this->expectException('Symfony\Component\PropertyAccess\Exception\AccessException');
+        $this->expectExceptionMessage('The method "Symfony\Component\PropertyAccess\Tests\Fixtures\UninitializedPrivateProperty::getUninitialized()" returned "null", but expected type "array". Have you forgotten to initialize a property or to make the return type nullable using "?array" instead?');
+
+        $this->propertyAccessor->getValue(new UninitializedPrivateProperty(), 'uninitialized');
+    }
+
     public function testGetValueThrowsExceptionIfNotArrayAccess()
     {
         $this->expectException('Symfony\Component\PropertyAccess\Exception\NoSuchIndexException');