From 56bab8e4b10b1502559111ec66e8ef76dc410b50 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Dec 2016 19:42:57 +0000 Subject: [PATCH 01/11] #751 nullable non-optional parameters (pre-7.1 style) seem to break the proxy generation - test asset to demonstrate that --- .../Common/Proxy/NullableNonOptionalHintClass.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php diff --git a/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php b/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php new file mode 100644 index 000000000..46f42db43 --- /dev/null +++ b/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php @@ -0,0 +1,15 @@ + Date: Fri, 2 Dec 2016 19:48:17 +0000 Subject: [PATCH 02/11] #751 test case: nullable non-optional parameters (pre-7.1 style) generate 7.0 incompatible code --- .../Tests/Common/Proxy/ProxyGeneratorTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php index 79949481d..984b9121b 100644 --- a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php +++ b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php @@ -277,6 +277,23 @@ public function testClassWithNullableReturnTypesOnProxiedMethods() $this->assertEquals(1, substr_count($classCode, 'function returnsNullableSelf(): ?\\' . $className)); } + public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods() + { + $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') + ); + } + public function testClassWithVoidReturnType() { if (PHP_VERSION_ID < 70100) { From ffafeb1fe80bfa151964f8701c3a5e73ee8a37d3 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Dec 2016 19:49:02 +0000 Subject: [PATCH 03/11] #751 fix - using `isDefaultValueAvailable` instead of `isOptional` correctly reports the type of the parameter --- lib/Doctrine/Common/Proxy/ProxyGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/Common/Proxy/ProxyGenerator.php b/lib/Doctrine/Common/Proxy/ProxyGenerator.php index 2c23bc870..70871d330 100644 --- 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; } From 6e25a848167b5b06b59172724ac256405134c56d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Dec 2016 19:49:50 +0000 Subject: [PATCH 04/11] #751 annotating new test case with `@group` --- tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php index 984b9121b..232c19480 100644 --- a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php +++ b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php @@ -277,6 +277,9 @@ public function testClassWithNullableReturnTypesOnProxiedMethods() $this->assertEquals(1, substr_count($classCode, 'function returnsNullableSelf(): ?\\' . $className)); } + /** + * @group #751 + */ public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods() { $className = NullableNonOptionalHintClass::class; From 4c6eb7a336235358c555e2d515717763ae6a0ef6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Dec 2016 19:53:48 +0000 Subject: [PATCH 05/11] #751 test-asset method for non-optional parameters with a default value --- .../Tests/Common/Proxy/NullableNonOptionalHintClass.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php b/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php index 46f42db43..312086a68 100644 --- a/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php +++ b/tests/Doctrine/Tests/Common/Proxy/NullableNonOptionalHintClass.php @@ -12,4 +12,8 @@ class NullableNonOptionalHintClass public function midSignatureNullableParameter(\stdClass $param = null, $secondParam) { } + + public function midSignatureNotNullableHintedParameter(string $param = 'foo', $secondParam) + { + } } From bb40f84e84710d19d484720aa3ae2041c7f81f6b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Dec 2016 19:54:19 +0000 Subject: [PATCH 06/11] #751 mid-signature parameters with a default parameters should not be considered nullable by default --- tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php index 232c19480..1eb66fffc 100644 --- a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php +++ b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php @@ -279,6 +279,8 @@ public function testClassWithNullableReturnTypesOnProxiedMethods() /** * @group #751 + * + * @requires PHP 7.0 */ public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods() { @@ -295,6 +297,11 @@ public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods() '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') + ); } public function testClassWithVoidReturnType() From 5c7970315cd96a5a8fe4b93c6e25f499aff2122c Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Dec 2016 19:57:43 +0000 Subject: [PATCH 07/11] #751 using `@requires` instead of a `PHP_VERSION_ID` comparison in tests that need skipping --- .../Tests/Common/Proxy/ProxyGeneratorTest.php | 53 ++++++++----------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php index 1eb66fffc..57da600f4 100644 --- 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']); @@ -304,12 +296,11 @@ public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods() ); } + /** + * @requires PHP 7.0 + */ public function testClassWithVoidReturnType() { - if (PHP_VERSION_ID < 70100) { - $this->markTestSkipped('Void method return types are only supported in PHP >= 7.1.0.'); - } - $className = VoidReturnTypeClass::class; if (!class_exists('Doctrine\Tests\Common\ProxyProxy\__CG__\VoidReturnTypeClass', false)) { $metadata = $this->createClassMetadata($className, ['id']); @@ -323,12 +314,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); } @@ -361,12 +351,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); From cb24146ce963dba17645517b8944d665f1883be2 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Dec 2016 20:03:25 +0000 Subject: [PATCH 08/11] #751 corrected skipped test requirements (`void` requires PHP 7.1) --- tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php index 57da600f4..94ed52cb9 100644 --- a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php +++ b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php @@ -297,7 +297,7 @@ public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods() } /** - * @requires PHP 7.0 + * @requires PHP 7.1 */ public function testClassWithVoidReturnType() { From 8c4cb0b53515057c83f42a9e0ec282005617c2ae Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 3 Dec 2016 05:06:25 +0000 Subject: [PATCH 09/11] #751 test asset - PHP 7.1-style nullable parameter with default value, yet not optional (not in last position in the signature) --- .../Tests/Common/Proxy/ProxyGeneratorTest.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php index 94ed52cb9..68d30f9ee 100644 --- a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php +++ b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php @@ -296,6 +296,33 @@ public function testClassWithNullableOptionalNonLastParameterOnProxiedMethods() ); } + /** + * @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') + ); + + $this->assertContains( + 'public function midSignatureNotNullableHintedParameter(?string $param = \'foo\', $secondParam)', + file_get_contents(__DIR__ . '/generated/__CG__DoctrineTestsCommonProxyPhp71NullableDefaultedNonOptionalHintClass.php') + ); + } + /** * @requires PHP 7.1 */ From 84c02e4dd1769c1eee4b273ffdd34e0a39cd2e26 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 3 Dec 2016 05:06:49 +0000 Subject: [PATCH 10/11] #751 checking signature of proxies of PHP 7.1-style nullable parameters with default value, yet not optional (not in last position in the signature) --- ...1NullableDefaultedNonOptionalHintClass.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 tests/Doctrine/Tests/Common/Proxy/Php71NullableDefaultedNonOptionalHintClass.php diff --git a/tests/Doctrine/Tests/Common/Proxy/Php71NullableDefaultedNonOptionalHintClass.php b/tests/Doctrine/Tests/Common/Proxy/Php71NullableDefaultedNonOptionalHintClass.php new file mode 100644 index 000000000..d66eff963 --- /dev/null +++ b/tests/Doctrine/Tests/Common/Proxy/Php71NullableDefaultedNonOptionalHintClass.php @@ -0,0 +1,20 @@ + Date: Sat, 3 Dec 2016 05:16:00 +0000 Subject: [PATCH 11/11] #751 adjusting test case: `foo(?string $bar = null, $baz)` is equivalent to `foo(string $bar = null, $baz)`, so no need to be too strict --- tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php index 68d30f9ee..16be54a50 100644 --- a/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php +++ b/tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php @@ -313,8 +313,9 @@ public function testClassWithPhp71NullableOptionalNonLastParameterOnProxiedMetho } $this->assertContains( - 'public function midSignatureNullableParameter(?string $param = NULL, $secondParam)', - file_get_contents(__DIR__ . '/generated/__CG__DoctrineTestsCommonProxyPhp71NullableDefaultedNonOptionalHintClass.php') + '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(