Merge branch '3.4' into 4.4

* 3.4: Fix more quotes in exception messages [3.4] Minor fixes [PropertyAccess] Improved errors when reading uninitialized properties
symfony · Mar 16, 2020 · cbe50a7 · cbe50a7
2 parents 4a7fcda + 2baa812
commit cbe50a7
Show file tree

Hide file tree

Showing 17 changed files with 117 additions and 34 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -1196,7 +1196,7 @@ private function registerTranslatorConfiguration(array $config, ContainerBuilder
             if ($container->fileExists($dir)) {
                 $dirs[] = $transPaths[] = $dir;
             } else {
-                throw new \UnexpectedValueException(sprintf('%s defined in translator.paths does not exist or is not a directory.', $dir));
+                throw new \UnexpectedValueException(sprintf('"%s" defined in translator.paths does not exist or is not a directory.', $dir));
             }
         }
 

diff --git a/src/Symfony/Component/Cache/Traits/RedisTrait.php b/src/Symfony/Component/Cache/Traits/RedisTrait.php
@@ -56,7 +56,7 @@ private function init($redisClient, string $namespace, int $defaultLifetime, ?Ma
         }
 
         if (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \RedisCluster && !$redisClient instanceof \Predis\ClientInterface && !$redisClient instanceof RedisProxy && !$redisClient instanceof RedisClusterProxy) {
-            throw new InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\ClientInterface, "%s" given.', __METHOD__, \is_object($redisClient) ? \get_class($redisClient) : \gettype($redisClient)));
+            throw new InvalidArgumentException(sprintf('"%s()" expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\ClientInterface, "%s" given.', __METHOD__, \is_object($redisClient) ? \get_class($redisClient) : \gettype($redisClient)));
         }
 
         if ($redisClient instanceof \Predis\ClientInterface && $redisClient->getOptions()->exceptions) {

diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php
@@ -62,7 +62,7 @@ public function getArguments(Request $request, $controller): array
                 }
 
                 if (!$atLeastOne) {
-                    throw new \InvalidArgumentException(sprintf('%s::resolve() must yield at least one value.', \get_class($resolver)));
+                    throw new \InvalidArgumentException(sprintf('"%s::resolve()" must yield at least one value.', \get_class($resolver)));
                 }
 
                 // continue to the next controller argument

diff --git a/src/Symfony/Component/Intl/NumberFormatter/NumberFormatter.php b/src/Symfony/Component/Intl/NumberFormatter/NumberFormatter.php
@@ -363,7 +363,7 @@ public function format($value, $type = self::TYPE_DEFAULT)
         }
 
         if (self::CURRENCY === $this->style) {
-            throw new NotImplementedException(sprintf('%s() method does not support the formatting of currencies (instance with CURRENCY style). "%s".', __METHOD__, NotImplementedException::INTL_INSTALL_MESSAGE));
+            throw new NotImplementedException(sprintf('"%s()" method does not support the formatting of currencies (instance with CURRENCY style). "%s".', __METHOD__, NotImplementedException::INTL_INSTALL_MESSAGE));
         }
 
         // Only the default type is supported.

diff --git a/src/Symfony/Component/Lock/Store/MemcachedStore.php b/src/Symfony/Component/Lock/Store/MemcachedStore.php
@@ -47,7 +47,7 @@ public function __construct(\Memcached $memcached, int $initialTtl = 300)
         }
 
         if ($initialTtl < 1) {
-            throw new InvalidArgumentException(sprintf('%s() expects a strictly positive TTL. Got %d.', __METHOD__, $initialTtl));
+            throw new InvalidArgumentException(sprintf('"%s()" expects a strictly positive TTL. Got %d.', __METHOD__, $initialTtl));
         }
 
         $this->memcached = $memcached;
@@ -86,7 +86,7 @@ public function waitAndSave(Key $key)
     public function putOffExpiration(Key $key, $ttl)
     {
         if ($ttl < 1) {
-            throw new InvalidTtlException(sprintf('%s() expects a TTL greater or equals to 1 second. Got %s.', __METHOD__, $ttl));
+            throw new InvalidTtlException(sprintf('"%s()" expects a TTL greater or equals to 1 second. Got %s.', __METHOD__, $ttl));
         }
 
         // Interface defines a float value but Store required an integer.

diff --git a/src/Symfony/Component/Lock/Store/RedisStore.php b/src/Symfony/Component/Lock/Store/RedisStore.php
@@ -153,7 +153,7 @@ private function evaluate(string $script, string $resource, array $args)
             return $this->redis->eval(...array_merge([$script, 1, $resource], $args));
         }
 
-        throw new InvalidArgumentException(sprintf('%s() expects being initialized with a Redis, RedisArray, RedisCluster or Predis\ClientInterface, "%s" given.', __METHOD__, \is_object($this->redis) ? \get_class($this->redis) : \gettype($this->redis)));
+        throw new InvalidArgumentException(sprintf('"%s()" expects being initialized with a Redis, RedisArray, RedisCluster or Predis\ClientInterface, "%s" given.', __METHOD__, \is_object($this->redis) ? \get_class($this->redis) : \gettype($this->redis)));
     }
 
     private function getUniqueToken(Key $key): string

diff --git a/src/Symfony/Component/Process/InputStream.php b/src/Symfony/Component/Process/InputStream.php
@@ -45,7 +45,7 @@ public function write($input)
             return;
         }
         if ($this->isClosed()) {
-            throw new RuntimeException(sprintf('%s is closed.', static::class));
+            throw new RuntimeException(sprintf('"%s" is closed.', static::class));
         }
         $this->input[] = ProcessUtils::validateInput(__METHOD__, $input);
     }

diff --git a/src/Symfony/Component/Process/Pipes/AbstractPipes.php b/src/Symfony/Component/Process/Pipes/AbstractPipes.php
@@ -103,7 +103,7 @@ protected function write(): ?array
             } elseif (!isset($this->inputBuffer[0])) {
                 if (!\is_string($input)) {
                     if (!is_scalar($input)) {
-                        throw new InvalidArgumentException(sprintf('%s yielded a value of type "%s", but only scalars and stream resources are supported.', \get_class($this->input), \gettype($input)));
+                        throw new InvalidArgumentException(sprintf('"%s" yielded a value of type "%s", but only scalars and stream resources are supported.', \get_class($this->input), \gettype($input)));
                     }
                     $input = (string) $input;
                 }

diff --git a/src/Symfony/Component/Process/ProcessUtils.php b/src/Symfony/Component/Process/ProcessUtils.php
@@ -61,7 +61,7 @@ public static function validateInput($caller, $input)
                 return new \IteratorIterator($input);
             }
 
-            throw new InvalidArgumentException(sprintf('%s only accepts strings, Traversable objects or stream resources.', $caller));
+            throw new InvalidArgumentException(sprintf('"%s" only accepts strings, Traversable objects or stream resources.', $caller));
         }
 
         return $input;

diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php
@@ -293,7 +293,7 @@ public function testSetInputWhileRunningThrowsAnException()
     public function testInvalidInput($value)
     {
         $this->expectException('Symfony\Component\Process\Exception\InvalidArgumentException');
-        $this->expectExceptionMessage('Symfony\Component\Process\Process::setInput only accepts strings, Traversable objects or stream resources.');
+        $this->expectExceptionMessage('"Symfony\Component\Process\Process::setInput" only accepts strings, Traversable objects or stream resources.');
         $process = $this->getProcess('foo');
         $process->setInput($value);
     }

diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php
@@ -378,30 +378,50 @@ private function readProperty(array $zval, string $property, bool $ignoreInvalid
         $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]}();
+            } elseif (!$ignoreInvalidProperty) {
+                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]}();
-        } elseif (!$ignoreInvalidProperty) {
-            throw new NoSuchPropertyException($access[self::ACCESS_NAME]);
+
+            throw $e;
         }
 
         // Objects are always passed around by reference

diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/UninitializedPrivateProperty.php b/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;
+    }
+}
diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/UninitializedProperty.php b/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;
+}
diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php
@@ -28,6 +28,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
 {
@@ -131,6 +133,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');

diff --git a/src/Symfony/Component/Security/Core/User/UserInterface.php b/src/Symfony/Component/Security/Core/User/UserInterface.php
@@ -23,7 +23,7 @@
  *
  * Regardless of how your users are loaded or where they come from (a database,
  * configuration, web service, etc.), you will have a class that implements
- * this interface. Objects that implement this interface are created and 
+ * this interface. Objects that implement this interface are created and
  * loaded by different objects that implement UserProviderInterface.
  *
  * @see UserProviderInterface

diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php
@@ -222,7 +222,7 @@ private function triggerRememberMe(AuthenticatorInterface $guardAuthenticator, R
         }
 
         if (!$response instanceof Response) {
-            throw new \LogicException(sprintf('%s::onAuthenticationSuccess *must* return a Response if you want to use the remember me functionality. Return a Response, or set remember_me to false under the guard configuration.', \get_class($guardAuthenticator)));
+            throw new \LogicException(sprintf('"%s::onAuthenticationSuccess()" *must* return a Response if you want to use the remember me functionality. Return a Response, or set remember_me to false under the guard configuration.', \get_class($guardAuthenticator)));
         }
 
         $this->rememberMeServices->loginSuccess($request, $response, $token);

diff --git a/src/Symfony/Component/VarDumper/Cloner/Data.php b/src/Symfony/Component/VarDumper/Cloner/Data.php
@@ -122,7 +122,7 @@ public function count()
     public function getIterator()
     {
         if (!\is_array($value = $this->getValue())) {
-            throw new \LogicException(sprintf('%s object holds non-iterable type "%s".', self::class, \gettype($value)));
+            throw new \LogicException(sprintf('"%s" object holds non-iterable type "%s".', self::class, \gettype($value)));
         }
 
         yield from $value;