Merge pull request #754 from doctrine/fix/#751-non-optional-type-hint…

…ed-parameters-pre-7.1-support-2.7 Fix #751 - backport #752 - proxy generation broken on non-optional parameters with default value
doctrine · Dec 3, 2016 · 5954c29 · 5954c29
2 parents 223a292 + 6fe29f8
commit 5954c29
Show file tree

Hide file tree

Showing 4 changed files with 115 additions and 32 deletions.
diff --git a/lib/Doctrine/Common/Proxy/ProxyGenerator.php b/lib/Doctrine/Common/Proxy/ProxyGenerator.php
@@ -1083,7 +1083,7 @@ private function formatType(
         }
 
         if ($type->allowsNull()
-            && (null === $parameter || ! $parameter->isOptional() || null !== $parameter->getDefaultValue())
+            && (null === $parameter || ! $parameter->isDefaultValueAvailable() || null !== $parameter->getDefaultValue())
         ) {
             $name = '?' . $name;
         }

diff --git a/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php b/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace Doctrine\Tests\Common\Proxy;
+
+/**
+ * Test PHP 7.0 compatibility of nullable type hints generation on a non-optional hinted parameter that is nullable
+ *
+ * @see https://github.com/doctrine/common/issues/751
+ */
+class NullableNonOptionalHintClass
+{
+    public function midSignatureNullableParameter(\stdClass $param = null, $secondParam)
+    {
+    }
+
+    public function midSignatureNotNullableHintedParameter(string $param = 'foo', $secondParam)
+    {
+    }
+}
diff --git a/tests/Doctrine/Tests/Common/Proxy/Php71NullableDefaultedNonOptionalHintClass.php b/tests/Doctrine/Tests/Common/Proxy/Php71NullableDefaultedNonOptionalHintClass.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace Doctrine\Tests\Common\Proxy;
+
+/**
+ * Test PHP 7.1 compatibility of nullable type hints generation on a non-optional hinted parameter that is nullable,
+ * yet has a default parameter
+ *
+ * @see https://github.com/doctrine/common/issues/751
+ */
+class Php71NullableDefaultedNonOptionalHintClass
+{
+    public function midSignatureNullableParameter(?string $param = null, $secondParam)
+    {
+    }
+
+    public function midSignatureNotNullableHintedParameter(?string $param = 'foo', $secondParam)
+    {
+    }
+}
diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php
@@ -164,10 +164,6 @@ public function testClassWithCallableTypeHintOnProxiedMethod()
 
     public function testClassWithVariadicArgumentOnProxiedMethod()
     {
-        if (PHP_VERSION_ID < 50600) {
-            $this->markTestSkipped('`...` is only supported in PHP >=5.6.0');
-        }
-
         if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\VariadicTypeHintClass', false)) {
             $className = VariadicTypeHintClass::class;
             $metadata = $this->createClassMetadata($className, ['id']);
@@ -183,12 +179,11 @@ public function testClassWithVariadicArgumentOnProxiedMethod()
         $this->assertEquals(1, substr_count($classCode, 'parent::addType(...$types)'));
     }
 
+    /**
+     * @requires PHP 7.0
+     */
     public function testClassWithScalarTypeHintsOnProxiedMethods()
     {
-        if (PHP_VERSION_ID < 70000) {
-            $this->markTestSkipped('Scalar type hints are only supported in PHP >= 7.0.0.');
-        }
-
         if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\ScalarTypeHintsClass', false)) {
             $className = ScalarTypeHintsClass::class;
             $metadata = $this->createClassMetadata($className, ['id']);
@@ -207,12 +202,11 @@ public function testClassWithScalarTypeHintsOnProxiedMethods()
         $this->assertEquals(1, substr_count($classCode, 'function withDefaultValueNull(int $foo = NULL)'));
     }
 
+    /**
+     * @requires PHP 7.0
+     */
     public function testClassWithReturnTypesOnProxiedMethods()
     {
-        if (PHP_VERSION_ID < 70000) {
-            $this->markTestSkipped('Method return types are only supported in PHP >= 7.0.0.');
-        }
-
         $className = ReturnTypesClass::class;
         if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\ReturnTypesClass', false)) {
             $metadata = $this->createClassMetadata($className, ['id']);
@@ -232,12 +226,11 @@ public function testClassWithReturnTypesOnProxiedMethods()
         $this->assertEquals(1, substr_count($classCode, 'function returnsInterface(): \Countable'));
     }
 
+    /**
+     * @requires PHP 7.1
+     */
     public function testClassWithNullableTypeHintsOnProxiedMethods()
     {
-        if (PHP_VERSION_ID < 70100) {
-            $this->markTestSkipped('Nullable type hints are only supported in PHP >= 7.1.0.');
-        }
-
         if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\NullableTypeHintsClass', false)) {
             $className = NullableTypeHintsClass::class;
             $metadata = $this->createClassMetadata($className, ['id']);
@@ -256,12 +249,11 @@ public function testClassWithNullableTypeHintsOnProxiedMethods()
         $this->assertEquals(1, substr_count($classCode, 'function notNullableTypeHintWithDefaultNull(int $param = NULL)'));
     }
 
+    /**
+     * @requires PHP 7.1
+     */
     public function testClassWithNullableReturnTypesOnProxiedMethods()
     {
-        if (PHP_VERSION_ID < 70100) {
-            $this->markTestSkipped('Nullable method return types are only supported in PHP >= 7.1.0.');
-        }
-
         $className = NullableTypeHintsClass::class;
         if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\NullableTypeHintsClass', false)) {
             $metadata = $this->createClassMetadata($className, ['id']);
@@ -277,12 +269,66 @@ public function testClassWithNullableReturnTypesOnProxiedMethods()
         $this->assertEquals(1, substr_count($classCode, 'function returnsNullableSelf(): ?\\' . $className));
     }
 
-    public function testClassWithVoidReturnType()
+    /**
+     * @group #751
+     *
+     * @requires PHP 7.0
+     */
+    public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods()
     {
-        if (PHP_VERSION_ID < 70100) {
-            $this->markTestSkipped('Void method return types are only supported in PHP >= 7.1.0.');
+        $className = NullableNonOptionalHintClass::class;
+
+        if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\NullableNonOptionalHintClass', false)) {
+            $metadata = $this->createClassMetadata($className, ['id']);
+
+            $proxyGenerator = new ProxyGenerator(__DIR__ . '/generated', __NAMESPACE__ . 'Proxy', true);
+            $this->generateAndRequire($proxyGenerator, $metadata);
         }
 
+        $this->assertContains(
+            'public function midSignatureNullableParameter(\stdClass $param = NULL, $secondParam)',
+            file_get_contents(__DIR__ . '/generated/__CG__DoctrineTestsCommonProxyNullableNonOptionalHintClass.php')
+        );
+
+        $this->assertContains(
+            'public function midSignatureNotNullableHintedParameter(string $param = \'foo\', $secondParam)',
+            file_get_contents(__DIR__ . '/generated/__CG__DoctrineTestsCommonProxyNullableNonOptionalHintClass.php')
+        );
+    }
+
+    /**
+     * @group #751
+     *
+     * @requires PHP 7.1
+     */
+    public function testClassWithPhp71NullableOptionalNonLastParameterOnProxiedMethods()
+    {
+        $className = Php71NullableDefaultedNonOptionalHintClass::class;
+
+        if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\Php71NullableDefaultedNonOptionalHintClass', false)) {
+            $metadata = $this->createClassMetadata($className, ['id']);
+
+            $proxyGenerator = new ProxyGenerator(__DIR__ . '/generated', __NAMESPACE__ . 'Proxy', true);
+            $this->generateAndRequire($proxyGenerator, $metadata);
+        }
+
+        $this->assertContains(
+            'public function midSignatureNullableParameter(string $param = NULL, $secondParam)',
+            file_get_contents(__DIR__ . '/generated/__CG__DoctrineTestsCommonProxyPhp71NullableDefaultedNonOptionalHintClass.php'),
+            'Signature allows nullable type, although explicit "?" marker isn\'t used in the proxy'
+        );
+
+        $this->assertContains(
+            'public function midSignatureNotNullableHintedParameter(?string $param = \'foo\', $secondParam)',
+            file_get_contents(__DIR__ . '/generated/__CG__DoctrineTestsCommonProxyPhp71NullableDefaultedNonOptionalHintClass.php')
+        );
+    }
+
+    /**
+     * @requires PHP 7.1
+     */
+    public function testClassWithVoidReturnType()
+    {
         $className = VoidReturnTypeClass::class;
         if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\VoidReturnTypeClass', false)) {
             $metadata = $this->createClassMetadata($className, ['id']);
@@ -296,12 +342,11 @@ public function testClassWithVoidReturnType()
         $this->assertEquals(1, substr_count($classCode, 'function returnsVoid(): void'));
     }
 
+    /**
+     * @requires PHP 7.0
+     */
     public function testClassWithIterableTypeHint()
     {
-        if (PHP_VERSION_ID < 70000) {
-            $this->markTestSkipped('Method return types are only supported in PHP >= 7.0.0.');
-        }
-
         if (PHP_VERSION_ID < 70100) {
             $this->expectException(UnexpectedValueException::class);
         }
@@ -334,12 +379,11 @@ public function testClassWithInvalidTypeHintOnProxiedMethod()
         $proxyGenerator->generateProxyClass($metadata);
     }
 
+    /**
+     * @requires PHP 7.0
+     */
     public function testClassWithInvalidReturnTypeOnProxiedMethod()
     {
-        if (PHP_VERSION_ID < 70000) {
-            $this->markTestSkipped('Method return types are only supported in PHP >= 7.0.0.');
-        }
-
         $className = InvalidReturnTypeClass::class;
         $metadata = $this->createClassMetadata($className, ['id']);
         $proxyGenerator = new ProxyGenerator(__DIR__ . '/generated', __NAMESPACE__ . 'Proxy', true);