From 9e2fe6d7d7f58885168575f3248f1405075d35bb Mon Sep 17 00:00:00 2001 From: Vincent Date: Tue, 19 Jan 2021 10:10:57 +0100 Subject: [PATCH 1/4] Default prop with @NamedArgumentConstructor Use the first constructor argument as default property when @NamedArgumentConstructor is used --- docs/en/custom.rst | 49 +++++++++----- lib/Doctrine/Common/Annotations/DocParser.php | 32 ++++++--- .../NamedArgumentConstructorAnnotation.php | 3 + .../Common/Annotations/DocParserTest.php | 67 +++++++++++++++++++ 4 files changed, 124 insertions(+), 27 deletions(-) diff --git a/docs/en/custom.rst b/docs/en/custom.rst index ef9a34a13..11fbe1a31 100644 --- a/docs/en/custom.rst +++ b/docs/en/custom.rst @@ -58,20 +58,27 @@ Optional: Constructors with Named Parameters Starting with Annotations v1.11 a new annotation instantiation strategy is available that aims at compatibility of Annotation classes with the PHP 8 -attribute feature. +attribute feature. You need to declare a constructor with regular parameter +names that match the named arguments in the annotation syntax. -You can implement the +To enable this feature, you can tag your annotation class with +``@NamedArgumentConstructor`` (available from v1.12) or implement the ``Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation`` interface -and then declare a constructor with regular parameter names that are matched -from the named arguments in the annotation syntax. +(available from v1.11 and deprecated as of v1.12). +When using the ``@NamedArgumentConstructor`` tag, the first argument of the +constructor is considered as the default one. + + +Usage with the ``@NamedArgumentContrustor`` tag .. code-block:: php namespace MyCompany\Annotations; - use Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation; - - /** @Annotation */ + /** + * @Annotation + * @NamedArgumentConstructor + */ class Bar implements NamedArgumentConstructorAnnotation { private $foo; @@ -82,7 +89,8 @@ from the named arguments in the annotation syntax. } } - /** Useable with @Bar(foo="baz") */ + /** Usable with @Bar(foo="baz") */ + /** Usable with @Bar("baz") */ In combination with PHP 8's constructor property promotion feature you can simplify this to: @@ -91,30 +99,35 @@ you can simplify this to: namespace MyCompany\Annotations; - use Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation; - - /** @Annotation */ + /** + * @Annotation + * @NamedArgumentConstructor + */ class Bar implements NamedArgumentConstructorAnnotation { public function __construct(private string $foo) {} } -Alternatively, you can annotate your annotation class with -``@NamedArgumentConstructor`` in case you cannot use the marker interface. +Usage with the +``Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation`` +interface (v1.11, deprecated as of v1.12): .. code-block:: php namespace MyCompany\Annotations; - /** - * @Annotation - * @NamedArgumentConstructor - */ - class Bar + use Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation; + + /** @Annotation */ + class Bar implements NamedArgumentConstructorAnnotation { + private $foo; + public function __construct(private string $foo) {} } + /** Usable with @Bar(foo="baz") */ + Annotation Target ----------------- diff --git a/lib/Doctrine/Common/Annotations/DocParser.php b/lib/Doctrine/Common/Annotations/DocParser.php index 4d7ecbbe2..21c5bf345 100644 --- a/lib/Doctrine/Common/Annotations/DocParser.php +++ b/lib/Doctrine/Common/Annotations/DocParser.php @@ -547,6 +547,10 @@ class_exists(NamedArgumentConstructor::class); if ($annotation instanceof NamedArgumentConstructor) { $metadata['has_named_argument_constructor'] = $metadata['has_constructor']; + if ($metadata['has_named_argument_constructor']) { + // choose the first argument as the default property + $metadata['default_property'] = $constructor->getParameters()[0]->getName(); + } } if (! ($annotation instanceof Attributes)) { @@ -859,7 +863,17 @@ private function Annotation() ); } - $values = $this->MethodCall(); + $defaultProperty = 'value'; + // Change the default property only if the @NamedArgumentConstructor + // tag and the default_property are set + if ( + self::$annotationMetadata[$name]['has_named_argument_constructor'] + && self::$annotationMetadata[$name]['default_property'] !== null + ) { + $defaultProperty = self::$annotationMetadata[$name]['default_property']; + } + + $values = $this->MethodCall($defaultProperty); if (isset(self::$annotationMetadata[$name]['enum'])) { // checks all declared attributes @@ -1013,7 +1027,7 @@ private function Annotation() * @throws AnnotationException * @throws ReflectionException */ - private function MethodCall(): array + private function MethodCall(string $defaultProperty): array { $values = []; @@ -1024,7 +1038,7 @@ private function MethodCall(): array $this->match(DocLexer::T_OPEN_PARENTHESIS); if (! $this->lexer->isNextToken(DocLexer::T_CLOSE_PARENTHESIS)) { - $values = $this->Values(); + $values = $this->Values($defaultProperty); } $this->match(DocLexer::T_CLOSE_PARENTHESIS); @@ -1040,7 +1054,7 @@ private function MethodCall(): array * @throws AnnotationException * @throws ReflectionException */ - private function Values(): array + private function Values(string $defaultProperty): array { $values = [$this->Value()]; @@ -1064,14 +1078,14 @@ private function Values(): array foreach ($values as $k => $value) { if (is_object($value) && $value instanceof stdClass) { $values[$value->name] = $value->value; - } elseif (! isset($values['value'])) { - $values['value'] = $value; + } elseif (! isset($values[$defaultProperty])) { + $values[$defaultProperty] = $value; } else { - if (! is_array($values['value'])) { - $values['value'] = [$values['value']]; + if (! is_array($values[$defaultProperty])) { + $values[$defaultProperty] = [$values[$defaultProperty]]; } - $values['value'][] = $value; + $values[$defaultProperty][] = $value; } unset($values[$k]); diff --git a/lib/Doctrine/Common/Annotations/NamedArgumentConstructorAnnotation.php b/lib/Doctrine/Common/Annotations/NamedArgumentConstructorAnnotation.php index f87a2c5d7..8af224c0b 100644 --- a/lib/Doctrine/Common/Annotations/NamedArgumentConstructorAnnotation.php +++ b/lib/Doctrine/Common/Annotations/NamedArgumentConstructorAnnotation.php @@ -5,6 +5,9 @@ /** * Marker interface for PHP7/PHP8 compatible support * for named arguments (and constructor property promotion). + * + * @deprecated Implementing this interface is deprecated + * Use the Annotation @NamedArgumentConstructor instead */ interface NamedArgumentConstructorAnnotation { diff --git a/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php b/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php index c16ad90c9..c55da9884 100644 --- a/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php +++ b/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php @@ -1611,6 +1611,41 @@ public function testNamedArgumentsConstructorAnnotationWithDefaultValue(): void self::assertSame('baz', $result[0]->getFoo()); self::assertSame(1234, $result[0]->getBar()); } + + public function testNamedArgumentsConstructorAnnotationWithDefaultProperty(): void + { + $result = $this + ->createTestParser() + ->parse('/** @AnotherNamedAnnotation("baz") */'); + + self::assertCount(1, $result); + self::assertInstanceOf(AnotherNamedAnnotation::class, $result[0]); + self::assertSame('baz', $result[0]->getFoo()); + self::assertSame(1234, $result[0]->getBar()); + } + + public function testNamedArgumentsConstructorAnnotationWithDefaultPropertyAsArray(): void + { + $result = $this + ->createTestParser() + ->parse('/** @NamedAnnotationWithArray({"foo","bar","baz"},bar=567) */'); + + self::assertCount(1, $result); + self::assertInstanceOf(NamedAnnotationWithArray::class, $result[0]); + self::assertSame(['foo', 'bar', 'baz'], $result[0]->getFoo()); + self::assertSame(567, $result[0]->getBar()); + } + + public function testNamedArgumentsConstructorAnnotationWithDefaultPropertySet(): void + { + $result = $this + ->createTestParser() + ->parse('/** @AnotherNamedAnnotation("baz", foo="bar") */'); + + self::assertCount(1, $result); + self::assertInstanceOf(AnotherNamedAnnotation::class, $result[0]); + self::assertSame('bar', $result[0]->getFoo()); + } } /** @Annotation */ @@ -1666,6 +1701,38 @@ public function getBar(): int } } +/** + * @Annotation + * @NamedArgumentConstructor + */ +class NamedAnnotationWithArray +{ + /** @var mixed[] */ + private $foo; + /** @var int */ + private $bar; + + /** + * @param mixed[] $foo + */ + public function __construct(array $foo, int $bar = 1234) + { + $this->foo = $foo; + $this->bar = $bar; + } + + /** @return mixed[] */ + public function getFoo(): array + { + return $this->foo; + } + + public function getBar(): int + { + return $this->bar; + } +} + /** @Annotation */ class SettingsAnnotation { From f1f293ade13b63c6820d2d37bcdb84bbd2e00ce9 Mon Sep 17 00:00:00 2001 From: Vincent Date: Sat, 6 Feb 2021 14:36:30 +0100 Subject: [PATCH 2/4] Values() will return an array of position --- lib/Doctrine/Common/Annotations/DocParser.php | 57 ++++++++++++------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/lib/Doctrine/Common/Annotations/DocParser.php b/lib/Doctrine/Common/Annotations/DocParser.php index 21c5bf345..a97913e17 100644 --- a/lib/Doctrine/Common/Annotations/DocParser.php +++ b/lib/Doctrine/Common/Annotations/DocParser.php @@ -15,8 +15,10 @@ use function array_keys; use function array_map; +use function array_values; use function class_exists; use function constant; +use function count; use function defined; use function explode; use function gettype; @@ -607,7 +609,7 @@ class_exists(NamedArgumentConstructor::class); // choose the first property as default property $metadata['default_property'] = reset($metadata['properties']); - } elseif (PHP_VERSION_ID < 80000 && $metadata['has_named_argument_constructor']) { + } elseif ($metadata['has_named_argument_constructor']) { foreach ($constructor->getParameters() as $parameter) { $metadata['constructor_args'][$parameter->getName()] = [ 'position' => $parameter->getPosition(), @@ -863,17 +865,36 @@ private function Annotation() ); } - $defaultProperty = 'value'; - // Change the default property only if the @NamedArgumentConstructor - // tag and the default_property are set + $arguments = $this->MethodCall(); + + $positionalArguments = $arguments['positional_arguments'] ?? []; + $namedArguments = $arguments['named_arguments'] ?? []; + if ( self::$annotationMetadata[$name]['has_named_argument_constructor'] && self::$annotationMetadata[$name]['default_property'] !== null ) { - $defaultProperty = self::$annotationMetadata[$name]['default_property']; - } + $values = $namedArguments; + foreach (self::$annotationMetadata[$name]['constructor_args'] as $property => $parameter) { + $position = $parameter['position']; + if (isset($values[$property]) || ! isset($positionalArguments[$position])) { + continue; + } + + $values[$property] = $positionalArguments[$position]; + } + } else { + $values = $namedArguments; + if (! empty($positionalArguments) && ! isset($values['value'])) { + if (count($positionalArguments) === 1) { + $value = $positionalArguments[0]; + } else { + $value = array_values($positionalArguments); + } - $values = $this->MethodCall($defaultProperty); + $values['value'] = $value; + } + } if (isset(self::$annotationMetadata[$name]['enum'])) { // checks all declared attributes @@ -1027,7 +1048,7 @@ private function Annotation() * @throws AnnotationException * @throws ReflectionException */ - private function MethodCall(string $defaultProperty): array + private function MethodCall(): array { $values = []; @@ -1038,7 +1059,7 @@ private function MethodCall(string $defaultProperty): array $this->match(DocLexer::T_OPEN_PARENTHESIS); if (! $this->lexer->isNextToken(DocLexer::T_CLOSE_PARENTHESIS)) { - $values = $this->Values($defaultProperty); + $values = $this->Values(); } $this->match(DocLexer::T_CLOSE_PARENTHESIS); @@ -1054,7 +1075,7 @@ private function MethodCall(string $defaultProperty): array * @throws AnnotationException * @throws ReflectionException */ - private function Values(string $defaultProperty): array + private function Values(): array { $values = [$this->Value()]; @@ -1075,23 +1096,17 @@ private function Values(string $defaultProperty): array $values[] = $value; } + $namedArguments = []; + $positionalArguments = []; foreach ($values as $k => $value) { if (is_object($value) && $value instanceof stdClass) { - $values[$value->name] = $value->value; - } elseif (! isset($values[$defaultProperty])) { - $values[$defaultProperty] = $value; + $namedArguments[$value->name] = $value->value; } else { - if (! is_array($values[$defaultProperty])) { - $values[$defaultProperty] = [$values[$defaultProperty]]; - } - - $values[$defaultProperty][] = $value; + $positionalArguments[$k] = $value; } - - unset($values[$k]); } - return $values; + return ['named_arguments' => $namedArguments, 'positional_arguments' => $positionalArguments]; } /** From 5c2388979e7de72fcfdb13be1464941ef335869b Mon Sep 17 00:00:00 2001 From: Vincent Date: Sun, 7 Feb 2021 15:52:02 +0100 Subject: [PATCH 3/4] Move values resolution in dedicated helper --- lib/Doctrine/Common/Annotations/DocParser.php | 79 +++++++++++-------- 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/lib/Doctrine/Common/Annotations/DocParser.php b/lib/Doctrine/Common/Annotations/DocParser.php index a97913e17..32f591a42 100644 --- a/lib/Doctrine/Common/Annotations/DocParser.php +++ b/lib/Doctrine/Common/Annotations/DocParser.php @@ -601,7 +601,7 @@ class_exists(NamedArgumentConstructor::class); } $metadata['enum'][$property->name]['value'] = $annotation->value; - $metadata['enum'][$property->name]['literal'] = ( ! empty($annotation->literal)) + $metadata['enum'][$property->name]['literal'] = (! empty($annotation->literal)) ? $annotation->literal : $annotation->value; } @@ -866,35 +866,7 @@ private function Annotation() } $arguments = $this->MethodCall(); - - $positionalArguments = $arguments['positional_arguments'] ?? []; - $namedArguments = $arguments['named_arguments'] ?? []; - - if ( - self::$annotationMetadata[$name]['has_named_argument_constructor'] - && self::$annotationMetadata[$name]['default_property'] !== null - ) { - $values = $namedArguments; - foreach (self::$annotationMetadata[$name]['constructor_args'] as $property => $parameter) { - $position = $parameter['position']; - if (isset($values[$property]) || ! isset($positionalArguments[$position])) { - continue; - } - - $values[$property] = $positionalArguments[$position]; - } - } else { - $values = $namedArguments; - if (! empty($positionalArguments) && ! isset($values['value'])) { - if (count($positionalArguments) === 1) { - $value = $positionalArguments[0]; - } else { - $value = array_values($positionalArguments); - } - - $values['value'] = $value; - } - } + $values = $this->resolvePositionalValues($arguments, $name); if (isset(self::$annotationMetadata[$name]['enum'])) { // checks all declared attributes @@ -1132,9 +1104,9 @@ private function Constant() case ! empty($this->namespaces): foreach ($this->namespaces as $ns) { if (class_exists($ns . '\\' . $className) || interface_exists($ns . '\\' . $className)) { - $className = $ns . '\\' . $className; - $found = true; - break; + $className = $ns . '\\' . $className; + $found = true; + break; } } @@ -1161,7 +1133,7 @@ private function Constant() } if ($found) { - $identifier = $className . '::' . $const; + $identifier = $className . '::' . $const; } } @@ -1434,4 +1406,43 @@ private function isIgnoredAnnotation(string $name): bool return false; } + + /** + * Resolve positional arguments (without name) to named ones + * + * @param array $arguments + * + * @return array + */ + private function resolvePositionalValues(array $arguments, string $name): array + { + $positionalArguments = $arguments['positional_arguments'] ?? []; + $values = $arguments['named_arguments'] ?? []; + + if ( + self::$annotationMetadata[$name]['has_named_argument_constructor'] + && self::$annotationMetadata[$name]['default_property'] !== null + ) { + foreach (self::$annotationMetadata[$name]['constructor_args'] as $property => $parameter) { + $position = $parameter['position']; + if (isset($values[$property]) || ! isset($positionalArguments[$position])) { + continue; + } + + $values[$property] = $positionalArguments[$position]; + } + } else { + if (count($positionalArguments) > 0 && ! isset($values['value'])) { + if (count($positionalArguments) === 1) { + $value = $positionalArguments[0]; + } else { + $value = array_values($positionalArguments); + } + + $values['value'] = $value; + } + } + + return $values; + } } From e917927be01c01fa1fa9197d0266fe0ca1076289 Mon Sep 17 00:00:00 2001 From: Vincent Date: Mon, 8 Feb 2021 10:53:47 +0100 Subject: [PATCH 4/4] Allow multi positional values --- lib/Doctrine/Common/Annotations/DocParser.php | 18 +++++++++--- .../Common/Annotations/DocParserTest.php | 29 ++++++++++++++++++- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/Common/Annotations/DocParser.php b/lib/Doctrine/Common/Annotations/DocParser.php index 32f591a42..0ac2a7004 100644 --- a/lib/Doctrine/Common/Annotations/DocParser.php +++ b/lib/Doctrine/Common/Annotations/DocParser.php @@ -1061,10 +1061,6 @@ private function Values(): array $token = $this->lexer->lookahead; $value = $this->Value(); - if (! is_object($value) && ! is_array($value)) { - throw $this->syntaxError('Value', $token); - } - $values[] = $value; } @@ -1423,6 +1419,20 @@ private function resolvePositionalValues(array $arguments, string $name): array self::$annotationMetadata[$name]['has_named_argument_constructor'] && self::$annotationMetadata[$name]['default_property'] !== null ) { + // We must ensure that we don't have positional arguments after named ones + $positions = array_keys($positionalArguments); + $lastPosition = null; + foreach ($positions as $position) { + if ( + ($lastPosition === null && $position !== 0 ) || + ($lastPosition !== null && $position !== $lastPosition + 1) + ) { + throw $this->syntaxError('Positional arguments after named arguments is not allowed'); + } + + $lastPosition = $position; + } + foreach (self::$annotationMetadata[$name]['constructor_args'] as $property => $parameter) { $position = $parameter['position']; if (isset($values[$property]) || ! isset($positionalArguments[$position])) { diff --git a/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php b/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php index c55da9884..e18656fc4 100644 --- a/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php +++ b/tests/Doctrine/Tests/Common/Annotations/DocParserTest.php @@ -87,6 +87,15 @@ public function testBasicAnnotations(): void self::assertInstanceOf(Name::class, $annot->value[0]); self::assertInstanceOf(Name::class, $annot->value[1]); + // Multiple scalar values + $result = $parser->parse('@Name("foo", "bar")'); + $annot = $result[0]; + + self::assertInstanceOf(Name::class, $annot); + self::assertIsArray($annot->value); + self::assertEquals('foo', $annot->value[0]); + self::assertEquals('bar', $annot->value[1]); + // Multiple types as values $result = $parser->parse('@Name(foo="Bar", @Name, {"key1"="value1", "key2"="value2"})'); $annot = $result[0]; @@ -1646,6 +1655,16 @@ public function testNamedArgumentsConstructorAnnotationWithDefaultPropertySet(): self::assertInstanceOf(AnotherNamedAnnotation::class, $result[0]); self::assertSame('bar', $result[0]->getFoo()); } + + public function testNamedArgumentsConstructorAnnotationWithInvalidArguments(): void + { + $parser = $this->createTestParser(); + $this->expectException(AnnotationException::class); + $this->expectExceptionMessage( + '[Syntax Error] Expected Positional arguments after named arguments is not allowed' + ); + $parser->parse('/** @AnotherNamedAnnotation("foo", bar=666, "hey") */'); + } } /** @Annotation */ @@ -1683,11 +1702,14 @@ class AnotherNamedAnnotation private $foo; /** @var int */ private $bar; + /** @var string */ + private $baz; - public function __construct(string $foo, int $bar = 1234) + public function __construct(string $foo, int $bar = 1234, string $baz = 'baz') { $this->foo = $foo; $this->bar = $bar; + $this->baz = $baz; } public function getFoo(): string @@ -1699,6 +1721,11 @@ public function getBar(): int { return $this->bar; } + + public function getBaz(): string + { + return $this->baz; + } } /**