Skip to content

Widen inferred template type arguments that only refine one of the bound's alternatives - #6146

Open
phpstan-bot wants to merge 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-xy3n62b
Open

Widen inferred template type arguments that only refine one of the bound's alternatives#6146
phpstan-bot wants to merge 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-xy3n62b

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

With @template Value of string|list<string>, new LanguageProperty(['abc']) was inferred as LanguageProperty<array{string}>. Because the template type is invariant, assigning it to a LanguageProperty<list<string>> property was reported as an error, even though array{string} says nothing more than list<string> does. The same happened for the case reported in the issue comments: new LanguageProperty('abc' . doFoo()) was inferred as LanguageProperty<non-falsy-string> and rejected by LanguageProperty<string>.

The fix widens an inferred template type argument to the bound alternative it merely refines, so the inferred generic type is usable in the invariant positions the class author declared.

Changes

  • src/Type/Generic/TemplateTypeHelper.php:
    • generalizeInferredTemplateType() was restructured (the early return for scalar bounds keeps the previous behaviour of preserving literals for @template T of string) and now finishes by calling the new widenToBoundType().
    • New widenToBoundType() picks the single alternative of the bound that the inferred type only refines and returns it. It bails out when several alternatives match, and when the inferred type is already at least as general as the alternative (so, for example, an implicit mixed is not replaced by the bound's explicit mixed).
    • New onlyRefinesType() decides whether the inferred type says the same thing as a bound alternative with more precision. Arrays are compared key type to key type and value type to value type (recursively, so nested shapes work); everything else is compared against its GeneralizePrecision::lessSpecific() generalization. An array that is definitely empty refines any array/iterable bound.
  • tests/PHPStan/Analyser/nsrt/bug-15027.php — new type inference test.
  • tests/PHPStan/Rules/Properties/data/bug-15027.php + testBug15027() in tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php — new rule test for the reported false positive on instance and static properties.

Root cause

generalizeInferredTemplateType() only generalized constant values ('abc'string, array{'abc'}array{string} with the shape kept). Anything else - array shapes, accessory string types, integer ranges, non-empty arrays - was passed through unchanged. For an invariant template type, any such refinement makes the resulting generic type incompatible with the type the class author declared in the bound, which is what users write in their @var/@param/@return PHPDocs. The pattern therefore showed up in every rule that compares generic types:

  • assign.propertyType (the reported case) and the static-property variant
  • argument.type when passing the object to a function/method
  • return.type when returning it

All of them share the single inference site, so widening at inference fixes the whole family. Both callers of generalizeInferredTemplateType() benefit: PHPStan\Analyser\ExprHandler\NewHandler (new C(...), new static(...)) and PHPStan\Reflection\ResolvedFunctionVariantWithOriginal (static factory methods returning self<T>, generic functions returning C<T>).

Deliberately not widened, because the bound says less than the argument does and the precision is the point (all covered by assertions in the new test):

  • @template T (mixed bound) — new Unbounded(['abc']) stays Unbounded<array{string}>
  • @template T of arraynew BoundToArray(['abc']) stays BoundToArray<array{string}>
  • @template T of array<string, mixed> — array shapes are preserved (this is what nsrt/bug-7788.php relies on for T[K] offset access)
  • object types — @template T of \DateTimeInterface|string keeps DateTimeImmutable, and a \Traversable bound keeps ArrayIterator
  • @template-covariant templates are untouched, as before

Test

tests/PHPStan/Analyser/nsrt/bug-15027.php reproduces the playground sample from the issue and asserts the inferred generic types. Beyond the reported case it covers the analogous constructs probed along the "invariant template argument inferred too narrowly" axis:

  • the issue comment's case: non-falsy-string and numeric-string arguments for a string|list<string> bound → LanguageProperty<string>
  • static factory method LanguageProperty::create() returning self<T> and a generic function returning LanguageProperty<T>
  • @template T of list<string> (non-union bound), @template T of string|list<list<string>> (nested lists), @template T of iterable<int, string>, @template T of int|list<int> (integer range refinement), @template T of array<string, string>
  • non-empty-list<string> and empty array arguments
  • the negative cases listed above (mixed, array, array<string, mixed>, object bounds)

tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest::testBug15027() analyses the reported snippet and expects no errors for the instance property (array{string} case) and the static property (non-falsy-string case).

Both tests were verified to fail before the fix (with exactly the messages from the issue) and to pass after it. The full test suite, the exec integration group and PHPStan's self-analysis are green, and self-analysis run time is unchanged.

Fixes phpstan/phpstan#15027

@staabm staabm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we get a failing test in https://github.com/phpstan/phpstan-src/actions/runs/30531943199/job/90837179166?pr=6146

I think when parameters are optional and not passed in, we should take their default value into account

@phpstan-bot

Copy link
Copy Markdown
Collaborator Author

Pushed as 9ae47d11c.

What the failing test was

The linked job was integration-tests / Integration - shopware/shopware, one error:

src/Core/System/SalesChannel/NoContentResponse.php:16
Parameter #1 $object of method StoreApiResponse<ArrayStruct<array>>::__construct()
expects ArrayStruct<array{}>, ArrayStruct<array> given.   [argument.type]

ArrayStruct is @template TData of array = array<array-key, mixed> with __construct(protected array $data = [], ...), and NoContentResponse declares @extends StoreApiResponse<ArrayStruct<array{}>> and calls parent::__construct(new ArrayStruct()).

Diagnosis

Your hunch was right about where it goes wrong, and PHPStan does already infer from the default value — new ArrayStruct() yields array{} from $data = [] (verified by checking out the pre-PR TemplateTypeHelper and dumping the type). What broke it was that my new widening then threw that array{} away.

The culprit was a shortcut in onlyRefinesType(): "an array that is definitely empty refines any array/iterable bound". That made the empty case inconsistent with every other array argument — for @template TData of array, new ArrayStruct(['a' => 1]) correctly kept array{a: int} (the bound says less than the argument), but new ArrayStruct([]) was widened to array.

Fix

Dropped the shortcut (src/Type/Generic/TemplateTypeHelper.php:208), letting the regular key-to-key/value-to-value comparison handle empty arrays. array{} has never key and value types, which are more precise than any bound's, so it is never widened now — which is the right answer, since array{} says something no array bound does and the class author may well have written Foo<array{}> deliberately, as shopware did.

Tests updated in tests/PHPStan/Analyser/nsrt/bug-15027.php: added a DefaultedArgument class mirroring the shopware signature (optional param + template default), and assertions pinning that new DefaultedArgument(), new DefaultedArgument([]), new LanguageProperty([]) and new BoundToList([]) all keep array{}, while new DefaultedArgument(['a' => 1]) keeps array{a: int}. The previous LanguageProperty([])list<string> assertion is replaced accordingly.

Verified the new assertions fail without the change (Expected: LanguageProperty<array{}> / Actual: LanguageProperty<list<string>>, DefaultedArgument<array{}> / DefaultedArgument<array>) and pass with it, and that the exact shopware snippet no longer reports argument.type. make tests (17751 tests, 97 skipped, no failures) and make phpstan are green.

One note on the run you linked

The same run also failed Integration - shipmonk/dead-code-detector with 18: Unused Default\MyPropertiesExtensionProvider::EXTENSION_TAG. That one is not from this PR — it reproduces identically on the unrelated resolve-type-rewrite-2 branch (run 30533452175), and ReadWritePropertiesExtensionProvider doesn't exist in phpstan-src at all. I left it alone.

phpstan-bot and others added 2 commits July 30, 2026 15:14
…und's alternatives

- `TemplateTypeHelper::generalizeInferredTemplateType()` now widens a non-covariant
  template type argument to the bound alternative it merely refines, so
  `new LanguageProperty(['abc'])` with `@template Value of string|list<string>`
  is inferred as `LanguageProperty<list<string>>` instead of `LanguageProperty<array{string}>`.
- Widening only happens when the inferred type says nothing more than the bound
  alternative: array shapes are compared key type to key type and value type to value
  type, everything else is compared against its `lessSpecific` generalization. Bounds
  that are more general than the argument (`@template T`, `@template T of array`,
  `@template T of array<string, mixed>`) keep the precise type, and object types keep
  their class.
- The same widening also covers accessory-refined scalars reported in the issue
  comments: `non-falsy-string` inferred for a `string|list<string>` bound now becomes
  `string`.
- Analogous cases fixed by the same change (each covered by a test): static factory
  methods returning `self<T>`, generic functions returning `C<T>`, nested list bounds,
  `iterable<int, string>` bounds, `non-empty-list` and integer-range refinements, empty
  arrays, and array-map bounds. The property/static-property/argument/return rule errors
  all disappear because they share the inference root cause.
array{} carries information no array bound does - it says the array is
empty - so widening it to a bound alternative throws that away. It also
made the empty case inconsistent with every other array argument: for
@template TData of array, new ArrayStruct(['a' => 1]) kept
array{a: int} while new ArrayStruct([]) became array.

This showed up in shopware, where an optional array $data = [] parameter
left ArrayStruct<array> where the class author had declared
@extends StoreApiResponse<ArrayStruct<array{}>>.

Dropping the shortcut lets the regular key-to-key/value-to-value
comparison handle empty arrays: their never key and value types are more
precise than the bound's, so array{} is preserved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@staabm
staabm force-pushed the create-pull-request/patch-xy3n62b branch from 9ae47d1 to d17ad29 Compare July 30, 2026 13:14
@staabm

staabm commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

@jiripudil could you double check this makes sense to you from a generics point of view?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

generic property false positive: LanguageProperty<list<string>>|null) does not accept LanguageProperty<array{string}>

2 participants