Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compatibility with named parameters and constructor property promotion #369

Merged
merged 5 commits into from
Oct 17, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
45 changes: 45 additions & 0 deletions lib/Doctrine/Common/Annotations/DocParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use function interface_exists;
use function is_array;
use function is_object;
use function is_subclass_of;
use function json_encode;
use function ltrim;
use function preg_match;
Expand All @@ -38,6 +39,8 @@
use function substr;
use function trim;

use const PHP_VERSION_ID;

/**
* A parser for docblock annotations.
*
Expand Down Expand Up @@ -502,6 +505,7 @@ class_exists(Attributes::class);
$metadata = [
'default_property' => null,
'has_constructor' => $constructor !== null && $constructor->getNumberOfParameters() > 0,
'constructor_args' => [],
'properties' => [],
'property_types' => [],
'attribute_types' => [],
Expand All @@ -510,6 +514,15 @@ class_exists(Attributes::class);
'is_annotation' => strpos($docComment, '@Annotation') !== false,
];

if (PHP_VERSION_ID < 80000 && $class->implementsInterface(NamedArgumentConstructorAnnotation::class)) {
foreach ($constructor->getParameters() as $parameter) {
$metadata['constructor_args'][$parameter->getName()] = [
'position' => $parameter->getPosition(),
'default' => $parameter->isOptional() ? $parameter->getDefaultValue() : null,
];
}
}

// verify that the class is really meant to be an annotation
if ($metadata['is_annotation']) {
self::$metadataParser->setTarget(Target::TARGET_CLASS);
Expand Down Expand Up @@ -897,6 +910,38 @@ private function Annotation()
}
}

if (is_subclass_of($name, NamedArgumentConstructorAnnotation::class)) {
if (PHP_VERSION_ID >= 80000) {
return new $name(...$values);
}

$positionalValues = [];
foreach (self::$annotationMetadata[$name]['constructor_args'] as $property => $parameter) {
$positionalValues[$parameter['position']] = $parameter['default'];
}

foreach ($values as $property => $value) {
if (! isset(self::$annotationMetadata[$name]['constructor_args'][$property])) {
throw AnnotationException::creationError(sprintf(
<<<'EXCEPTION'
The annotation @%s declared on %s does not have a property named "%s"
that can be set through its named arguments constructor.
Available named arguments: %s
EXCEPTION
,
$originalName,
$this->context,
$property,
implode(', ', array_keys(self::$annotationMetadata[$name]['constructor_args']))
));
}

$positionalValues[self::$annotationMetadata[$name]['constructor_args'][$property]['position']] = $value;
}

return new $name(...$positionalValues);
}

// check if the annotation expects values via the constructor,
// or directly injected into public properties
if (self::$annotationMetadata[$name]['has_constructor'] === true) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Doctrine\Common\Annotations;

/**
* Marker interface for PHP7/PHP8 compatible support
* for named arguments (and constructor property promotion).
Copy link
Member

Choose a reason for hiding this comment

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

Should constructor property promotion really be mentioned here? You did not write a test for it, so I believe this means it does not really come into play here: it will work the same regardless of whether you use that feature or not.

*/
interface NamedArgumentConstructorAnnotation
{
}
50 changes: 50 additions & 0 deletions tests/Doctrine/Tests/Common/Annotations/DocParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Doctrine\Common\Annotations\AnnotationException;
use Doctrine\Common\Annotations\AnnotationRegistry;
use Doctrine\Common\Annotations\DocParser;
use Doctrine\Common\Annotations\NamedArgumentConstructorAnnotation;
use Doctrine\Tests\Common\Annotations\Fixtures\AnnotationTargetAll;
use Doctrine\Tests\Common\Annotations\Fixtures\AnnotationWithConstants;
use Doctrine\Tests\Common\Annotations\Fixtures\ClassWithConstants;
Expand Down Expand Up @@ -1537,6 +1538,55 @@ public function testWillParseAnnotationSucceededByANonImmediateDash(): void
self::assertCount(1, $result);
self::assertInstanceOf(SomeAnnotationClassNameWithoutConstructorAndProperties::class, $result[0]);
}

public function testNamedArgumentsConstructorAnnotation(): void
{
$result = $this
->createTestParser()
->parse('/** @NamedAnnotation(bar=2222, foo="baz") */');
SenseException marked this conversation as resolved.
Show resolved Hide resolved

self::assertCount(1, $result);
self::assertInstanceOf(NamedAnnotation::class, $result[0]);
self::assertSame('baz', $result[0]->getFoo());
self::assertSame(2222, $result[0]->getBar());
}

public function testNamedArgumentsConstructorAnnotationWithDefaultValue(): void
{
$result = $this
->createTestParser()
->parse('/** @NamedAnnotation(foo="baz") */');

self::assertCount(1, $result);
self::assertInstanceOf(NamedAnnotation::class, $result[0]);
self::assertSame('baz', $result[0]->getFoo());
self::assertSame(1234, $result[0]->getBar());
}
}

/** @Annotation */
class NamedAnnotation implements NamedArgumentConstructorAnnotation
{
/** @var string */
private $foo;
/** @var int */
private $bar;

public function __construct(string $foo, int $bar = 1234)
{
$this->foo = $foo;
$this->bar = $bar;
}

public function getFoo(): string
{
return $this->foo;
}

public function getBar(): int
{
return $this->bar;
}
}

/** @Annotation */
Expand Down