Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 31 additions & 18 deletions docs/en/custom.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:
Expand All @@ -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
-----------------

Expand Down
94 changes: 72 additions & 22 deletions lib/Doctrine/Common/Annotations/DocParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -547,6 +549,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)) {
Expand Down Expand Up @@ -595,15 +601,15 @@ 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;
}
}

// 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(),
Expand Down Expand Up @@ -859,7 +865,8 @@ private function Annotation()
);
}

$values = $this->MethodCall();
$arguments = $this->MethodCall();
$values = $this->resolvePositionalValues($arguments, $name);

if (isset(self::$annotationMetadata[$name]['enum'])) {
// checks all declared attributes
Expand Down Expand Up @@ -1054,30 +1061,20 @@ 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;
}

$namedArguments = [];
$positionalArguments = [];
foreach ($values as $k => $value) {
if (is_object($value) && $value instanceof stdClass) {
$values[$value->name] = $value->value;
} elseif (! isset($values['value'])) {
$values['value'] = $value;
$namedArguments[$value->name] = $value->value;
} else {
if (! is_array($values['value'])) {
$values['value'] = [$values['value']];
}

$values['value'][] = $value;
$positionalArguments[$k] = $value;
}

unset($values[$k]);
}

return $values;
return ['named_arguments' => $namedArguments, 'positional_arguments' => $positionalArguments];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In resolvePositionalValues, you validate $positionalArguments. Can't we do this here already? I don't like that we pass around potentially invalid metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Values() method is part of the parsing process. If we move the validation inside, it will have to know what kind of annotation we are talking about. The validation is different is you use @NamedArgumentConstructor or not.
This is valid without @Name("1", foo="bar", "2") but should not be valid with @NamedArgumentConstructor as it's supposed to mimic a regular constructor call with named arguments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, thank you for clarifying. 👍🏻

}

/**
Expand All @@ -1103,9 +1100,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;
}
}

Expand All @@ -1132,7 +1129,7 @@ private function Constant()
}

if ($found) {
$identifier = $className . '::' . $const;
$identifier = $className . '::' . $const;
}
}

Expand Down Expand Up @@ -1405,4 +1402,57 @@ private function isIgnoredAnnotation(string $name): bool

return false;
}

/**
* Resolve positional arguments (without name) to named ones
*
* @param array<string,mixed> $arguments
*
* @return array<string,mixed>
*/
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
) {
// 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');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw $this->syntaxError('Positional arguments after named arguments is not allowed');
throw $this->syntaxError('Positional arguments are not allowed after named arguments');

}

$lastPosition = $position;
}

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];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #405

} else {
$value = array_values($positionalArguments);
}

$values['value'] = $value;
}
}

return $values;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
96 changes: 95 additions & 1 deletion tests/Doctrine/Tests/Common/Annotations/DocParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -1611,6 +1620,51 @@ 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());
}

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 */
Expand Down Expand Up @@ -1648,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
Expand All @@ -1664,6 +1721,43 @@ public function getBar(): int
{
return $this->bar;
}

public function getBaz(): string
{
return $this->baz;
}
}

/**
* @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 */
Expand Down