Skip to content

[PropertyInfo] Add PropertyAttributesExtractor #57601

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: 7.4
Choose a base branch
from
Open

[PropertyInfo] Add PropertyAttributesExtractor #57601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5e667cf
[PropertyInfo] Add attributes extractor
alyamovsky Jun 24, 2024
aaa35ac
[PropertyInfo] Add interface implementation
alyamovsky Jun 24, 2024
8c72472
[PropertyInfo] Fix DI
alyamovsky Jun 26, 2024
3928042
[PropertyInfo] Add tests
alyamovsky Jun 26, 2024
5001a53
[PropertyInfo] Update changelog
alyamovsky Jun 26, 2024
80e402b
[PropertyInfo] Update changelog
alyamovsky Jun 26, 2024
450cabb
[PropertyInfo] Update phpdoc
alyamovsky Jun 26, 2024
ceafe5d
[PropertyInfo] Fix code review
alyamovsky Jul 1, 2024
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
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class UnusedTagsPass implements CompilerPassInterface
'property_info.initializable_extractor',
'property_info.list_extractor',
'property_info.type_extractor',
'property_info.attributes_extractor',
'proxy',
'remote_event.consumer',
'routing.condition_service',
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@
use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
use Symfony\Component\PropertyInfo\Extractor\PhpStanExtractor;
use Symfony\Component\PropertyInfo\PropertyAccessExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyAttributesExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyDescriptionExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyInitializableExtractorInterface;
Expand Down Expand Up @@ -642,6 +643,8 @@ public function load(array $configs, ContainerBuilder $container): void
->addTag('property_info.access_extractor');
$container->registerForAutoconfiguration(PropertyInitializableExtractorInterface::class)
->addTag('property_info.initializable_extractor');
$container->registerForAutoconfiguration(PropertyAttributesExtractorInterface::class)
->addTag('property_info.attributes_extractor');
$container->registerForAutoconfiguration(EncoderInterface::class)
->addTag('serializer.encoder');
$container->registerForAutoconfiguration(DecoderInterface::class)
Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/Resources/config/property_info.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
use Symfony\Component\PropertyInfo\PropertyAccessExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyAttributesExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyDescriptionExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyInfoCacheExtractor;
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
Expand All @@ -26,14 +27,15 @@
return static function (ContainerConfigurator $container) {
$container->services()
->set('property_info', PropertyInfoExtractor::class)
->args([[], [], [], [], []])
->args([[], [], [], [], [], []])

->alias(PropertyAccessExtractorInterface::class, 'property_info')
->alias(PropertyDescriptionExtractorInterface::class, 'property_info')
->alias(PropertyInfoExtractorInterface::class, 'property_info')
->alias(PropertyTypeExtractorInterface::class, 'property_info')
->alias(PropertyListExtractorInterface::class, 'property_info')
->alias(PropertyInitializableExtractorInterface::class, 'property_info')
->alias(PropertyAttributesExtractorInterface::class, 'property_info')

->set('property_info.cache', PropertyInfoCacheExtractor::class)
->decorate('property_info')
Expand All @@ -45,6 +47,7 @@
->tag('property_info.type_extractor', ['priority' => -1002])
->tag('property_info.access_extractor', ['priority' => -1000])
->tag('property_info.initializable_extractor', ['priority' => -1000])
->tag('property_info.attributes_extractor', ['priority' => -1000])
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
->tag('property_info.attributes_extractor', ['priority' => -1000])
->tag('property_info.attribute_extractor', ['priority' => -1000])

To be consistent? (same in FrameworkExtension and PropertyInfoPass)


->alias(PropertyReadInfoExtractorInterface::class, 'property_info.reflection_extractor')
->alias(PropertyWriteInfoExtractorInterface::class, 'property_info.reflection_extractor')
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Component/PropertyInfo/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

7.2
---

* Add `PropertyAttributesExtractorInterface` to extract property attributes (implemented by `ReflectionExtractor`)

7.1
---

Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/PropertyInfo/DependencyInjection/PropertyInfoPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,8 @@ public function process(ContainerBuilder $container): void

$initializableExtractors = $this->findAndSortTaggedServices('property_info.initializable_extractor', $container);
$definition->setArgument(4, new IteratorArgument($initializableExtractors));

$attributesExtractor = $this->findAndSortTaggedServices('property_info.attributes_extractor', $container);
$definition->setArgument(5, new IteratorArgument($attributesExtractor));
}
}
27 changes: 26 additions & 1 deletion src/Symfony/Component/PropertyInfo/Extractor/ReflectionExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\PropertyInfo\Extractor;

use Symfony\Component\PropertyInfo\PropertyAccessExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyAttributesExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyInitializableExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyListExtractorInterface;
use Symfony\Component\PropertyInfo\PropertyReadInfo;
Expand All @@ -36,7 +37,7 @@
*
* @final
*/
class ReflectionExtractor implements PropertyListExtractorInterface, PropertyTypeExtractorInterface, PropertyAccessExtractorInterface, PropertyInitializableExtractorInterface, PropertyReadInfoExtractorInterface, PropertyWriteInfoExtractorInterface, ConstructorArgumentTypeExtractorInterface
class ReflectionExtractor implements PropertyListExtractorInterface, PropertyTypeExtractorInterface, PropertyAccessExtractorInterface, PropertyInitializableExtractorInterface, PropertyReadInfoExtractorInterface, PropertyWriteInfoExtractorInterface, ConstructorArgumentTypeExtractorInterface, PropertyAttributesExtractorInterface
{
/**
* @internal
Expand Down Expand Up @@ -507,6 +508,30 @@ public function getWriteInfo(string $class, string $property, array $context = [
return $noneProperty;
}

public function getAttributes(string $class, string $property, array $context = []): ?array
{
try {
$reflClass = new \ReflectionClass($class);
} catch (\ReflectionException) {
return null;
}

if (!$reflClass->hasProperty($property)) {
return null;
}

$attributes = [];
$reflProperty = $reflClass->getProperty($property);
foreach ($reflProperty->getAttributes() as $attribute) {
$attributes[] = [
'name' => $attribute->getName(),
'arguments' => $attribute->getArguments(),
];
}

return $attributes;
Comment on lines +525 to +532
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
foreach ($reflProperty->getAttributes() as $attribute) {
$attributes[] = [
'name' => $attribute->getName(),
'arguments' => $attribute->getArguments(),
];
}
return $attributes;
return $reflProperty->getAttributes();

Why not returning attributes themselves? So that we can have all the attribute data (repeated, etc) and use the newInstance method.

If it was because you want to decouple getAttributes from \ReflectionXXX, I think that we shouldn't try to decouple as it's part of the PHP language itself.

}

/**
* @return LegacyType[]|null
*/
Expand Down
29 changes: 29 additions & 0 deletions src/Symfony/Component/PropertyInfo/PropertyAttributesExtractorInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\PropertyInfo;

/**
* @author Andrew Alyamovsky <andrew.alyamovsky@gmail.com>
*/
interface PropertyAttributesExtractorInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
interface PropertyAttributesExtractorInterface
interface PropertyAttributeExtractorInterface

To be consistent with other extractors. For example, even if getTypes is plural, PropertyTypeExtractorInterface is singular.

{
/**
* Gets the attributes of the property.
*
* Returns an array of attributes, each attribute is an associative array with the following keys:
* - name: The fully-qualified class name of the attribute
* - arguments: An associative array of attribute arguments if present
Comment on lines +23 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use @param annotation to describe these parameters

Copy link
Author

Choose a reason for hiding this comment

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

Since the @param annotation is used for describing method's arguments, are you suggesting using it for the string $class, string $property, array $context = [] arguments?

*
* @return array<int, array{name: string, arguments: array<array-key, mixed>}>|null
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @return array<int, array{name: string, arguments: array<array-key, mixed>}>|null
* @return list<array{name: string, arguments: array<array-key, mixed>}>|null

*/
public function getAttributes(string $class, string $property, array $context = []): ?array;
}
7 changes: 6 additions & 1 deletion src/Symfony/Component/PropertyInfo/PropertyInfoCacheExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*
* @final
*/
class PropertyInfoCacheExtractor implements PropertyInfoExtractorInterface, PropertyInitializableExtractorInterface
class PropertyInfoCacheExtractor implements PropertyInfoExtractorInterface, PropertyInitializableExtractorInterface, PropertyAttributesExtractorInterface
{
private array $arrayCache = [];

Expand Down Expand Up @@ -69,6 +69,11 @@ public function getTypes(string $class, string $property, array $context = []):
return $this->extract('getTypes', [$class, $property, $context]);
}

public function getAttributes(string $class, string $property, array $context = []): ?array
{
return $this->extract('getAttributes', [$class, $property, $context]);
}

public function isInitializable(string $class, string $property, array $context = []): ?bool
{
return $this->extract('isInitializable', [$class, $property, $context]);
Expand Down
9 changes: 8 additions & 1 deletion src/Symfony/Component/PropertyInfo/PropertyInfoExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,23 @@
*
* @final
*/
class PropertyInfoExtractor implements PropertyInfoExtractorInterface, PropertyInitializableExtractorInterface
class PropertyInfoExtractor implements PropertyInfoExtractorInterface, PropertyInitializableExtractorInterface, PropertyAttributesExtractorInterface
{
/**
* @param iterable<mixed, PropertyListExtractorInterface> $listExtractors
* @param iterable<mixed, PropertyTypeExtractorInterface> $typeExtractors
* @param iterable<mixed, PropertyDescriptionExtractorInterface> $descriptionExtractors
* @param iterable<mixed, PropertyAccessExtractorInterface> $accessExtractors
* @param iterable<mixed, PropertyInitializableExtractorInterface> $initializableExtractors
* @param iterable<mixed, PropertyAttributesExtractorInterface> $attributesExtractors
*/
public function __construct(
private readonly iterable $listExtractors = [],
private readonly iterable $typeExtractors = [],
private readonly iterable $descriptionExtractors = [],
private readonly iterable $accessExtractors = [],
private readonly iterable $initializableExtractors = [],
private readonly iterable $attributesExtractors = [],
) {
}

Expand Down Expand Up @@ -66,6 +68,11 @@ public function getTypes(string $class, string $property, array $context = []):
return $this->extract($this->typeExtractors, 'getTypes', [$class, $property, $context]);
}

public function getAttributes(string $class, string $property, array $context = []): ?array
{
return $this->extract($this->attributesExtractors, 'getAttributes', [$class, $property, $context]);
}

public function isReadable(string $class, string $property, array $context = []): ?bool
{
return $this->extract($this->accessExtractors, 'isReadable', [$class, $property, $context]);
Expand Down
6 changes: 4 additions & 2 deletions src/Symfony/Component/PropertyInfo/Tests/DependencyInjection/PropertyInfoPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function testServicesAreOrderedAccordingToPriority($index, $tag)
{
$container = new ContainerBuilder();

$definition = $container->register('property_info')->setArguments([null, null, null, null, null]);
$definition = $container->register('property_info')->setArguments([null, null, null, null, null, null]);
$container->register('n2')->addTag($tag, ['priority' => 100]);
$container->register('n1')->addTag($tag, ['priority' => 200]);
$container->register('n3')->addTag($tag);
Expand All @@ -50,14 +50,15 @@ public static function provideTags()
[2, 'property_info.description_extractor'],
[3, 'property_info.access_extractor'],
[4, 'property_info.initializable_extractor'],
[5, 'property_info.attributes_extractor'],
];
}

public function testReturningEmptyArrayWhenNoService()
{
$container = new ContainerBuilder();
$propertyInfoExtractorDefinition = $container->register('property_info')
->setArguments([[], [], [], [], []]);
->setArguments([[], [], [], [], [], []]);

$propertyInfoPass = new PropertyInfoPass();
$propertyInfoPass->process($container);
Expand All @@ -67,5 +68,6 @@ public function testReturningEmptyArrayWhenNoService()
$this->assertEquals(new IteratorArgument([]), $propertyInfoExtractorDefinition->getArgument(2));
$this->assertEquals(new IteratorArgument([]), $propertyInfoExtractorDefinition->getArgument(3));
$this->assertEquals(new IteratorArgument([]), $propertyInfoExtractorDefinition->getArgument(4));
$this->assertEquals(new IteratorArgument([]), $propertyInfoExtractorDefinition->getArgument(5));
}
}
104 changes: 104 additions & 0 deletions src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
use Symfony\Component\PropertyInfo\Tests\Fixtures\ConstructorDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\DefaultValue;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Dummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\DummyAttribute;
use Symfony\Component\PropertyInfo\Tests\Fixtures\DummyWithAttributes;
use Symfony\Component\PropertyInfo\Tests\Fixtures\NotInstantiable;
use Symfony\Component\PropertyInfo\Tests\Fixtures\ParentDummy;
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php71Dummy;
Expand Down Expand Up @@ -498,6 +500,108 @@ public static function getInitializableProperties(): array
];
}

/**
* @dataProvider attributesProvider
*/
public function testGetAttributes(string $class, string $property, ?array $expected)
{
$this->assertSame($expected, $this->extractor->getAttributes($class, $property));
}

public static function attributesProvider(): array
{
return [
[
DummyWithAttributes::class,
'a',
[
[
'name' => DummyAttribute::class,
'arguments' => [
'type' => 'foo',
'name' => 'nameA',
'version' => 1,
],
],
],
],
[
DummyWithAttributes::class,
'b',
[
[
'name' => DummyAttribute::class,
'arguments' => [
'type' => 'bar',
'name' => 'nameB',
'version' => 2,
],
],
],
],
[
DummyWithAttributes::class,
'c',
[
[
'name' => DummyAttribute::class,
'arguments' => [
'type' => 'baz',
'name' => 'nameC',
'version' => 3,
],
],
],
],
[
DummyWithAttributes::class,
'd',
[
[
'name' => DummyAttribute::class,
'arguments' => [
0 => 'foo',
1 => 'nameD',
2 => 4,
],
],
],
],
[
DummyWithAttributes::class,
'e',
[
[
'name' => DummyAttribute::class,
'arguments' => [
'type' => 'foo',
'name' => 'nameE1',
'version' => 5,
],
],
[
'name' => DummyAttribute::class,
'arguments' => [
'type' => 'foo',
'name' => 'nameE2',
'version' => 10,
],
],
],
],
[
DummyWithAttributes::class,
'f',
[],
],
[
DummyWithAttributes::class,
'nonExistentProperty',
null,
],
];
}

/**
* @group legacy
*
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Component/PropertyInfo/Tests/Fixtures/DummyAttribute.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace Symfony\Component\PropertyInfo\Tests\Fixtures;

#[\Attribute(\Attribute::IS_REPEATABLE | \Attribute::TARGET_PROPERTY)]
class DummyAttribute
{
public function __construct(
public string $type,
public string $name,
public int $version,
) {
}
}
Loading
Loading