Skip to content

Commit 40ed629

Browse files
koriymclaude
andauthored
Support constructor method-level qualifiers in BcParameterQualifier (#308)
This change enhances backward compatibility by allowing method-level Qualifier attributes on constructors, addressing the issue where packages like bear/streamer use Qualifier-only attributes (without InjectInterface) on single-parameter constructors. Key changes: - Remove TARGET_PARAMETER requirement: Method-level qualifiers no longer need to support TARGET_PARAMETER, as they are meant to be used at the method level - Add constructor-specific logic: For constructors, Qualifier-only attributes are sufficient since InjectInterface is implicit - For setters: Still require InjectInterface + Qualifier combination All changes are confined to src-deprecated/ directory, keeping the core codebase clean. This embodies Ray.Di's BC (backward compatibility) philosophy. Test updates: - Add constructor test case with Qualifier-only attribute - Update existing tests to reflect new Qualifier inference behavior - Fix binding declarations in test modules 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2e5aceb commit 40ed629

File tree

9 files changed

+82
-57
lines changed

9 files changed

+82
-57
lines changed

src-deprecated/di/BcParameterQualifier.php

Lines changed: 23 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
1313
use function count;
1414

1515
/**
16-
* Backward compatible parameter qualifier for single-parameter setters
16+
* Backward compatible parameter qualifier for single-parameter methods
1717
*
18-
* Automatically applies method-level InjectInterface+Qualifier attributes to parameters
19-
* when the method has only one parameter and no parameter-level qualifier is specified.
18+
* Automatically applies method-level Qualifier attributes to parameters when:
19+
* - Method has exactly one parameter
20+
* - Parameter has no explicit qualifier
21+
* - For constructors: Method has a Qualifier attribute (InjectInterface is implicit)
22+
* - For setters: Method has an attribute implementing both InjectInterface and Qualifier
2023
*
2124
* This behavior is deprecated for the following reasons:
2225
* - Violates Single Responsibility Principle (one attribute serving dual purposes)
@@ -44,10 +47,10 @@ final class BcParameterQualifier
4447
* Returns parameter name mapping if:
4548
* 1. Method has exactly one parameter
4649
* 2. Parameter has no qualifier attribute
47-
* 3. Method has an attribute implementing both InjectInterface and Qualifier
48-
* 4. The attribute supports TARGET_PARAMETER (not just TARGET_METHOD)
50+
* 3. For setters: Method has an attribute implementing both InjectInterface and Qualifier
51+
* 4. For constructors: Method has a Qualifier attribute (InjectInterface is implicit)
4952
*
50-
* @param ReflectionMethod $method The setter method to analyze
53+
* @param ReflectionMethod $method The method to analyze
5154
*
5255
* @return array<string, string> Parameter name to qualifier mapping (empty if not applicable)
5356
*/
@@ -74,10 +77,10 @@ public static function getNames(ReflectionMethod $method): array
7477
* Returns the qualifier name if:
7578
* 1. Method has exactly one parameter
7679
* 2. Parameter has no qualifier attribute
77-
* 3. Method has an attribute implementing both InjectInterface and Qualifier
78-
* 4. The attribute supports TARGET_PARAMETER (not just TARGET_METHOD)
80+
* 3. For setters: Method has an attribute implementing both InjectInterface and Qualifier
81+
* 4. For constructors: Method has a Qualifier attribute (InjectInterface is implicit)
7982
*
80-
* @param ReflectionMethod $method The setter method to analyze
83+
* @param ReflectionMethod $method The method to analyze
8184
*
8285
* @return string The qualifier class name, or empty string if not applicable
8386
*/
@@ -97,33 +100,29 @@ private static function getQualifier(ReflectionMethod $method): string
97100
return '';
98101
}
99102

100-
// Check method-level attributes for InjectInterface+Qualifier combination
103+
$isConstructor = $method->name === '__construct';
104+
105+
// Check method-level attributes for Qualifier
101106
$methodAttributes = $method->getAttributes();
102107
foreach ($methodAttributes as $attr) {
103108
$instance = $attr->newInstance();
104-
105-
// Must implement InjectInterface
106-
if (! $instance instanceof InjectInterface) {
107-
continue;
108-
}
109-
110-
// Must also be marked with Qualifier
111109
$attrClass = new ReflectionClass($attr->getName());
112110
$qualifierAttr = $attrClass->getAttributes(Qualifier::class);
113111

112+
// Skip if not a Qualifier
114113
if ($qualifierAttr === []) {
115114
continue;
116115
}
117116

118-
// IMPORTANT: Only infer if attribute supports TARGET_PARAMETER
119-
// If it's TARGET_METHOD only, it's meant for Provider/InjectionPoint pattern
120-
if (! self::supportsParameterTarget($attrClass)) {
121-
continue;
117+
// For constructors: Qualifier alone is sufficient (InjectInterface is implicit)
118+
if ($isConstructor) {
119+
return $attr->getName();
122120
}
123121

124-
// Found a method-level attribute that is both Inject and Qualifier
125-
// AND supports being used at parameter level
126-
return $attr->getName();
122+
// For setters: Must also implement InjectInterface
123+
if ($instance instanceof InjectInterface) {
124+
return $attr->getName();
125+
}
127126
}
128127

129128
return '';
@@ -147,25 +146,4 @@ private static function hasParameterQualifier(array $attributes): bool
147146

148147
return false;
149148
}
150-
151-
/**
152-
* Check if attribute supports TARGET_PARAMETER
153-
*
154-
* @param ReflectionClass<object> $attrClass
155-
*/
156-
private static function supportsParameterTarget(ReflectionClass $attrClass): bool
157-
{
158-
$attributeAttrs = $attrClass->getAttributes(\Attribute::class);
159-
if ($attributeAttrs === []) {
160-
return false;
161-
}
162-
163-
$attributeInstance = $attributeAttrs[0]->newInstance();
164-
if (! $attributeInstance instanceof \Attribute) {
165-
return false;
166-
}
167-
168-
// Check if Attribute::TARGET_PARAMETER is included in the flags
169-
return ($attributeInstance->flags & \Attribute::TARGET_PARAMETER) !== 0;
170-
}
171149
}

tests/di/AnnotatedClassTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function testInvoke(): void
2929
(new Bind($container, FakeHandleInterface::class))->toProvider(FakeHandleProvider::class);
3030
(new Bind($container, FakeMirrorInterface::class))->annotatedWith('right')->to(FakeMirrorRight::class);
3131
(new Bind($container, FakeMirrorInterface::class))->annotatedWith('left')->to(FakeMirrorRight::class);
32-
(new Bind($container, FakeGearStickInterface::class))->toProvider(FakeGearStickProvider::class);
32+
(new Bind($container, FakeGearStickInterface::class))->annotatedWith(FakeGearStickInject::class)->toProvider(FakeGearStickProvider::class);
3333
$car = $newInstance($container);
3434
if (! $car instanceof FakeCar) {
3535
throw new LogicException();

tests/di/BcParameterQualifierTest.php

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use PHPUnit\Framework\TestCase;
88
use Ray\Di\Annotation\FakeInjectOne;
9+
use Ray\Di\Annotation\FakeQualifierOnly;
910
use ReflectionMethod;
1011

1112
class BcParameterQualifierTest extends TestCase
@@ -55,14 +56,25 @@ public function testNoNamesForMethodWithoutInjectInterface(): void
5556
$this->assertSame([], $names);
5657
}
5758

58-
public function testNoNamesForTargetMethodOnly(): void
59+
public function testNamesForTargetMethodOnly(): void
5960
{
60-
// FakeGearStickInject has TARGET_METHOD only, not TARGET_PARAMETER
61-
// It's meant for Provider/InjectionPoint pattern, not parameter binding
61+
// FakeGearStickInject has TARGET_METHOD only
62+
// BC parameter qualifier now supports TARGET_METHOD-only attributes for backward compatibility
6263
$method = new ReflectionMethod(FakeBcParameterQualifierClass::class, 'setSingleParamMethodOnly');
6364
/** @psalm-suppress DeprecatedClass */
6465
$names = BcParameterQualifier::getNames($method);
6566

66-
$this->assertSame([], $names);
67+
$this->assertSame(['param' => FakeGearStickInject::class], $names);
68+
}
69+
70+
public function testConstructorWithQualifierOnly(): void
71+
{
72+
// Constructor with Qualifier-only attribute (no InjectInterface)
73+
// BC parameter qualifier should apply for constructors (InjectInterface is implicit)
74+
$method = new ReflectionMethod(FakeBcConstructorQualifierClass::class, '__construct');
75+
/** @psalm-suppress DeprecatedClass */
76+
$names = BcParameterQualifier::getNames($method);
77+
78+
$this->assertSame(['param' => FakeQualifierOnly::class], $names);
6779
}
6880
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ray\Di\Annotation;
6+
7+
use Attribute;
8+
use Ray\Di\Di\Qualifier;
9+
10+
/**
11+
* Qualifier-only attribute for testing (no InjectInterface)
12+
*/
13+
#[Attribute(Attribute::TARGET_METHOD), Qualifier]
14+
final class FakeQualifierOnly
15+
{
16+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ray\Di;
6+
7+
use Ray\Di\Annotation\FakeQualifierOnly;
8+
9+
class FakeBcConstructorQualifierClass
10+
{
11+
/**
12+
* Constructor with method-level Qualifier-only attribute (no InjectInterface)
13+
* This should trigger BC parameter qualifier for constructors
14+
*/
15+
#[FakeQualifierOnly]
16+
public function __construct(public $param)
17+
{
18+
}
19+
}

tests/di/Fake/FakeBcParameterQualifierClass.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class FakeBcParameterQualifierClass
1818

1919
/**
2020
* Single parameter with method-level InjectInterface+Qualifier
21-
* This should trigger BC parameter qualifier (FakeInjectOne supports TARGET_PARAMETER)
21+
* This should trigger BC parameter qualifier
2222
*/
2323
#[FakeInjectOne]
2424
public function setSingleParam(FakeGearStickInterface $param): void
@@ -63,12 +63,12 @@ public function setSingleParamNoInject(FakeGearStickInterface $param): void
6363
}
6464

6565
/**
66-
* Method-level attribute with TARGET_METHOD only (Provider pattern)
67-
* Should NOT infer because it doesn't support TARGET_PARAMETER
66+
* Method-level attribute with TARGET_METHOD only
67+
* BC parameter qualifier now supports this for backward compatibility
6868
*/
6969
#[FakeGearStickInject('test')]
7070
public function setSingleParamMethodOnly(FakeGearStickInterface $param): void
7171
{
72-
// This is for Provider/InjectionPoint pattern, not parameter binding
72+
$this->singleParam = $param;
7373
}
7474
}

tests/di/Fake/FakeCarModule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ protected function configure()
1818
$this->bind(FakeMirrorInterface::class)->annotatedWith('left')->to(FakeMirrorLeft::class)->in(Scope::SINGLETON); // named binding
1919
$this->bind('')->annotatedWith('logo')->toInstance('momo');
2020
$this->bind(FakeHandleInterface::class)->toProvider(FakeHandleProvider::class);
21-
$this->bind(FakeGearStickInterface::class)->to(FakeLeatherGearStick::class);
21+
$this->bind(FakeGearStickInterface::class)->annotatedWith(FakeGearStickInject::class)->to(FakeLeatherGearStick::class);
2222
$this->bindInterceptor(
2323
$this->matcher->any(),
2424
$this->matcher->any('start'),

tests/di/Fake/FakePhp8CarModule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ protected function configure()
2121
$this->bind(FakeMirrorInterface::class)->annotatedWith(FakeRight::class)->to(FakeMirrorRight::class); // named binding
2222
$this->bind('')->annotatedWith('logo')->toInstance('momo');
2323
$this->bind(FakeHandleInterface::class)->toProvider(FakeHandleProvider::class);
24-
$this->bind(FakeGearStickInterface::class)->toProvider(FakeGearStickProvider::class);
24+
$this->bind(FakeGearStickInterface::class)->annotatedWith(FakeGearStickInject::class)->toProvider(FakeGearStickProvider::class);
2525
$this->bind()->annotatedWith(FakeInjectOne::class)->toInstance(1);
2626
}
2727
}

tests/di/VisitorTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public function visitArgument(string $index, bool $isDefaultAvailable, $defaultV
173173

174174
$this->dependency->accept($visitor);
175175
$this->assertStringContainsString('Ray\Di\FakeCar', $collector->newInstance);
176-
$this->assertStringContainsString('(prototype.Ray\Di\FakeGearStickInterface-)', $collector->newInstance);
176+
$this->assertStringContainsString('(prototype.Ray\Di\FakeGearStickInterface-Ray\Di\FakeGearStickInject)', $collector->newInstance);
177177
$this->assertSame('setTires(prototype.Ray\Di\FakeEngineInterface-)', $collector->methods[0]);
178178
$this->assertSame('setHardtop(prototype.Ray\Di\FakeTyreInterface-,prototype.Ray\Di\FakeTyreInterface-)', $collector->methods[1]);
179179
}

0 commit comments

Comments
 (0)