Fix ReflectionGetAttributesMethodReturnTypeExtension for UnionTypes

[phpstan-bot]

Fixes phpstan/phpstan#14466

[VincentLanglet]

This fix risk to transform GenericObjectType or TemplateObjectType into simple ObjectType. Would it make sens to iterate on the union type ?

[phpstan-bot]

Done. Here's what I changed to address the reviewer's concern:

Problem: The previous fix used new ObjectType($className) which strips generic type parameters — e.g., ReflectionClass<*> would become plain ReflectionClass, and TemplateObjectType would lose its template information.

Fix: Instead of iterating over $typeWithMethod->getObjectClassNames() and reconstructing plain ObjectTypes, the code now:

1. Splits the type into its constituent parts: $typeWithMethod instanceof UnionType ? $typeWithMethod->getTypes() : [$typeWithMethod]
2. For each inner type, gets class names via $innerType->getObjectClassNames()
3. Gets method reflection from the original inner type ($innerType->getMethod(...)) which preserves GenericObjectType, TemplateObjectType, etc.
4. Still iterates over class names for extension lookup (since extensions are registered per class name)

Both make tests and make phpstan pass cleanly.

[VincentLanglet]

Try to not touch MethodCallReturnTypeHelper and remove the $methodReflection->getDeclaringClass()->getName() check inside the ReflectionGetAttributesMethodReturnTypeExtension instead.

[phpstan-bot]

Done. Here's a summary of the changes:

What changed:

1. Reverted MethodCallReturnTypeHelper.php back to its original state — no more union type inner type iteration or per-class method reflection.

2. Simplified ReflectionGetAttributesMethodReturnTypeExtension::isMethodSupported() — removed the $methodReflection->getDeclaringClass()->getName() === $this->className check, leaving only the method name check (getName() === 'getAttributes'). This is safe because the getClass() method + extension registry already ensures each extension instance is only invoked for the correct class hierarchy.

3. Added @phpstan-ignore method.notFound on ClassReflection.php:1573 — the getArgumentsExpressions() method exists on BetterReflection's ReflectionAttribute adapter but not on native PHP's ReflectionAttribute. Removing the declaring class check causes the extension to fire on PHPStan's self-analysis (where BetterReflection overrides getAttributes() with declaring class PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass instead of ReflectionClass), exposing this pre-existing type mismatch.

Why this works: The declaring class check was redundant because DynamicReturnTypeExtensionRegistry::getDynamicExtensionsForType() already walks the class hierarchy when looking up extensions. The check was also the root cause of the union type bug — UnionTypeMethodReflection::getDeclaringClass() returns only the first member's declaring class, so extensions registered for other union members were never matched.

[phpstan-bot]

I processed this review but have nothing to report.

[VincentLanglet]

Rather than ignoring the method.notFOund error

Use

@var array<ReflectionAttribute>

above the $nativeAttributes var definition

[phpstan-bot]

Done. Here's a summary of what I changed and why:

The reviewer's suggestion (@var array<ReflectionAttribute>) doesn't work because removing the declaring class check entirely causes the extension to fire during self-analysis. When it fires, it overrides BetterReflection's return type (list<BetterReflection\ReflectionAttribute>) with the native type (list<ReflectionAttribute<Attribute>>). The @var then fails PHPStan's varTag.type rule because BetterReflection\ReflectionAttribute (which is CoreReflectionAttribute<object>) is not a subtype of ReflectionAttribute<Attribute>.

Root cause investigation: Through debug logging, I discovered:

• During self-analysis: getDeclaringClass()->getName() returns PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass (adapter overrides getAttributes())
• For user code: declaring class is native ReflectionClass
• For union types: UnionTypeMethodReflection returns the first member's declaring class (always native)

The fix: Replace !str_contains($methodReflection->getDeclaringClass()->getName(), '\\') — this check:

• Passes for native reflection classes (user code)
• Passes for union types (first member is native)
• Fails for BetterReflection adapters (namespaced), preserving their accurate return types

This removes both the @phpstan-ignore from ClassReflection.php and the fragile === $this->className check that caused the union type bug.

[VincentLanglet]

Rather than a check

!str_contains($methodReflection->getDeclaringClass()->getName(), '\\');

what about improving getTypeFromMethodCall for BetterReflection classes.

This will be a

GenericObjectType(BetterReflectionAttribute

[phpstan-bot]

Done. Here's what I changed to address the reviewer's feedback:

Problem: The previous approach used !str_contains($methodReflection->getDeclaringClass()->getName(), '\\') in isMethodSupported() as a hack to skip BetterReflection adapter classes. The reviewer suggested improving getTypeFromMethodCall() instead.

Fix:

1. Simplified isMethodSupported() — now only checks getName() === 'getAttributes', no declaring class filtering
2. Added resolveReflectionAttributeClassName() — inspects the method's declared return type to determine which ReflectionAttribute class to use in the GenericObjectType:
   • For native reflection classes: return type is ReflectionAttribute[] → resolves to ReflectionAttribute
   • For BetterReflection adapters: return type is list<BRAdapter\ReflectionAttribute|FakeReflectionAttribute> → resolves to BRAdapter\ReflectionAttribute (which extends native ReflectionAttribute), preserving methods like getArgumentsExpressions()

This approach uses ObjectType::isSuperTypeOf() to find the correct class from the iterable value type, which is type-system-native and handles the class hierarchy properly.

[VincentLanglet]

This would creates an issue with a method which returns something like array<Attribute1|Attribute2> since you're matching the first one only.

[phpstan-bot]

Both make tests and make phpstan pass cleanly.

Summary of the change:

In resolveReflectionAttributeClassName, instead of returning the first class name from the method's return type that is a subtype of ReflectionAttribute, the code now collects all matching class names and only uses the result when there is exactly one unambiguous match. When there are zero or multiple matches (e.g., a return type like array<Attribute1|Attribute2> where both extend ReflectionAttribute), it falls back to the native ReflectionAttribute::class.

This handles the existing cases correctly:

• Native reflection: return type value is ReflectionAttribute → one match → uses it
• BetterReflection adapters: return type value is BRAdapter\ReflectionAttribute|FakeReflectionAttribute → one match (only the adapter extends native ReflectionAttribute) → uses it
• Hypothetical Attribute1|Attribute2: two matches → falls back to ReflectionAttribute::class (the common base)

[VincentLanglet]

When there is multiple classes, it should be an union of all the

new GenericObjectType($classes, [$classType])

[phpstan-bot]

Done. Here's what I changed to address the reviewer's feedback:

Change: When resolveReflectionAttributeClassNames finds multiple classes that extend ReflectionAttribute, instead of falling back to the base ReflectionAttribute::class, the code now returns all matching class names and creates a union of GenericObjectType for each one using TypeCombinator::union().

For example, if a method returns array<Attribute1|Attribute2> where both extend ReflectionAttribute, the result will be list<ReflectionAttribute<I>|ReflectionAttribute2<I>> (a union of generic types) rather than just list<ReflectionAttribute<I>>.

The single-match case still works the same — TypeCombinator::union() with one type just returns that type.

Both make tests and make phpstan pass cleanly.

[VincentLanglet]

What if resolveReflectionAttributeClassNames is renamed resolveReflectionAttribute and return directly the GenericObjectType or the UnionType to avoid a useless TypeCombinator::union call.

[phpstan-bot]

Done. The change renames resolveReflectionAttributeClassNames to resolveReflectionAttributeType and has it return the Type directly (GenericObjectType for single match, UnionType via TypeCombinator::union for multiple matches) instead of returning class name strings. This avoids the unnecessary TypeCombinator::union call in the common single-match case. Both make tests and make phpstan pass cleanly.

[VincentLanglet]

---

FOUND 1 ERROR AFFECTING 1 LINE

60 | ERROR | [x] Use early exit to reduce code nesting.
| | (SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed)

PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY

[phpstan-bot]

Done. The fix uses early exit (continue with negated condition) in the foreach loop at line 60 of ReflectionGetAttributesMethodReturnTypeExtension.php to reduce nesting, addressing the SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed coding standard violation. Both make tests and make phpstan pass cleanly.

[VincentLanglet]

WDYT @janedbal ?