Repository: PHP-DI/PHP-DI (php)

Issue:
#[Inject] on readonly constructor-promoted properties
Injection currently only works when the Inject attribute is used as a method attribute on the constructor. 

When the Inject attribute is used on the readonly constructor-promoted property, PHP-DI attempts to set the value through ```\DI\Definition\Resolver\ObjectCreator::setPrivatePropertyValue()``` but fails because ```setValue()``` can't be called on a readonly property.

Pull request:
Fix injection of readonly promoted properties
When `#[Inject]` annotations are used on promoted properties, PHP-DI quietly injects the value twice: once as a constructor argument, and once as an injected property. This is because PHP [replicates the attribute on the constructor argument to the property](https://www.php.net/manual/en/language.oop5.decon.php#language.oop5.decon.constructor.promotion).

Generally, the second unneeded injection goes unnoticed, but when `readonly` properties are used, the property injection fails:

    Error: Cannot modify readonly property DI\Test\IntegrationTest\Attributes\PromotedReadonlyProperty::$promotedProperty

This PR excludes promoted properties from injection. Promoted properties should never be injected because they will already have been assigned a value in the constructor.

This fixes #853.

Patch:
diff --git a/src/Definition/Source/AttributeBasedAutowiring.php b/src/Definition/Source/AttributeBasedAutowiring.php
index 5b8580c5..a3010280 100644
--- a/src/Definition/Source/AttributeBasedAutowiring.php
+++ b/src/Definition/Source/AttributeBasedAutowiring.php
@@ -77,9 +77,6 @@ class AttributeBasedAutowiring implements DefinitionSource, Autowiring
     private function readProperties(ReflectionClass $class, ObjectDefinition $definition) : void
     {
         foreach ($class->getProperties() as $property) {
-            if ($property->isStatic()) {
-                continue;
-            }
             $this->readProperty($property, $definition);
         }
 
@@ -87,9 +84,6 @@ class AttributeBasedAutowiring implements DefinitionSource, Autowiring
         /** @noinspection PhpAssignmentInConditionInspection */
         while ($class = $class->getParentClass()) {
             foreach ($class->getProperties(ReflectionProperty::IS_PRIVATE) as $property) {
-                if ($property->isStatic()) {
-                    continue;
-                }
                 $this->readProperty($property, $definition, $class->getName());
             }
         }
@@ -100,6 +94,10 @@ class AttributeBasedAutowiring implements DefinitionSource, Autowiring
      */
     private function readProperty(ReflectionProperty $property, ObjectDefinition $definition, string $classname = null) : void
     {
+        if ($property->isStatic() || $property->isPromoted()) {
+            return;
+        }
+
         // Look for #[Inject] attribute
         try {
             $attribute = $property->getAttributes(Inject::class)[0] ?? null;


Test patch:
diff --git a/tests/IntegrationTest/Attributes/AttributesTest.php b/tests/IntegrationTest/Attributes/AttributesTest.php
index 450d35c3..33597dd0 100644
--- a/tests/IntegrationTest/Attributes/AttributesTest.php
+++ b/tests/IntegrationTest/Attributes/AttributesTest.php
@@ -105,4 +105,29 @@ class AttributesTest extends BaseContainerTest
         $builder->useAttributes(true);
         $builder->build()->get(NamedInjection::class);
     }
+
+    /**
+     * @test
+     * @dataProvider provideContainer
+     */
+    public function inject_promoted_property(ContainerBuilder $builder)
+    {
+        $builder->useAttributes(true);
+        $object = $builder->build()->get(PromotedProperty::class);
+        $this->assertInstanceOf(A::class, $object->promotedProperty);
+    }
+
+    /**
+     * @test
+     * @dataProvider provideContainer
+     */
+    public function inject_promoted_readonly_property(ContainerBuilder $builder)
+    {
+        if (PHP_VERSION_ID < 80100) {
+            $this->markTestSkipped("PHP 8.1 required for readonly properties");
+        }
+        $builder->useAttributes(true);
+        $object = $builder->build()->get(PromotedReadonlyProperty::class);
+        $this->assertInstanceOf(A::class, $object->promotedProperty);
+    }
 }
diff --git a/tests/IntegrationTest/Attributes/PromotedProperty.php b/tests/IntegrationTest/Attributes/PromotedProperty.php
new file mode 100644
index 00000000..116ff7f3
--- /dev/null
+++ b/tests/IntegrationTest/Attributes/PromotedProperty.php
@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DI\Test\IntegrationTest\Attributes;
+
+use DI\Attribute\Inject;
+
+class PromotedProperty
+{
+    public function __construct(#[Inject(A::class)] public $promotedProperty)
+    {
+    }
+}
diff --git a/tests/IntegrationTest/Attributes/PromotedReadonlyProperty.php b/tests/IntegrationTest/Attributes/PromotedReadonlyProperty.php
new file mode 100644
index 00000000..774d6685
--- /dev/null
+++ b/tests/IntegrationTest/Attributes/PromotedReadonlyProperty.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DI\Test\IntegrationTest\Attributes;
+
+use DI\Attribute\Inject;
+
+class PromotedReadonlyProperty
+{
+    public function __construct(
+        #[Inject(A::class)] public readonly object $promotedProperty
+    ) {
+    }
+}
diff --git a/tests/UnitTest/Definition/Source/AttributeBasedAutowiringTest.php b/tests/UnitTest/Definition/Source/AttributeBasedAutowiringTest.php
index 0204e2ec..1d837e32 100644
--- a/tests/UnitTest/Definition/Source/AttributeBasedAutowiringTest.php
+++ b/tests/UnitTest/Definition/Source/AttributeBasedAutowiringTest.php
@@ -20,6 +20,7 @@ use DI\Test\UnitTest\Definition\Source\Fixtures\AnnotationFixtureScalarTypedProp
 use DI\Test\UnitTest\Definition\Source\Fixtures\AnnotationFixtureTypedProperties;
 use DI\Test\UnitTest\Definition\Source\Fixtures\AnnotationInjectableFixture;
 use DI\Test\UnitTest\Definition\Source\Fixtures\AttributeFixture;
+use DI\Test\UnitTest\Definition\Source\Fixtures\AttributeFixturePromotedProperty;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -242,6 +243,19 @@ class AttributeBasedAutowiringTest extends TestCase
         $this->assertHasPropertyInjection($definition, 'propertyParentPrivate');
     }
 
+    public function testPromotedProperties(): void
+    {
+        $definition = (new AttributeBasedAutowiring)->autowire(AttributeFixturePromotedProperty::class);
+        $this->assertNotHasPropertyInjection($definition, 'promotedProperty');
+
+        $constructorInjection = $definition->getConstructorInjection();
+        $this->assertInstanceOf(MethodInjection::class, $constructorInjection);
+
+        $parameters = $constructorInjection->getParameters();
+        $this->assertCount(1, $parameters);
+        $this->assertEquals(new Reference('foo'), $parameters[0]);
+    }
+
     private function getMethodInjection(ObjectDefinition $definition, $name) : ?MethodInjection
     {
         $methodInjections = $definition->getMethodInjections();
@@ -279,7 +293,7 @@ class AttributeBasedAutowiringTest extends TestCase
         $propertyInjections = $definition->getPropertyInjections();
         foreach ($propertyInjections as $propertyInjection) {
             if ($propertyInjection->getPropertyName() === $propertyName) {
-                $this->fail('No property injection found for ' . $propertyName);
+                $this->fail('Unexpected property injection found for ' . $propertyName);
             }
         }
     }
diff --git a/tests/UnitTest/Definition/Source/Fixtures/AttributeFixturePromotedProperty.php b/tests/UnitTest/Definition/Source/Fixtures/AttributeFixturePromotedProperty.php
new file mode 100644
index 00000000..9a1a14eb
--- /dev/null
+++ b/tests/UnitTest/Definition/Source/Fixtures/AttributeFixturePromotedProperty.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DI\Test\UnitTest\Definition\Source\Fixtures;
+
+use DI\Attribute\Inject;
+
+class AttributeFixturePromotedProperty
+{
+    public function __construct(#[Inject("foo")] public $promotedProperty) {
+    }
+}