Skip to content

Generalize a template argument resolution when writes widen the object beyond its initial type - #6814

Open
phpstan-bot wants to merge 1 commit into
phpstan:2.3.xfrom
phpstan-bot:create-pull-request/patch-19lp9o0
Open

phpstan-bot wants to merge 1 commit into
phpstan:2.3.xfrom
phpstan-bot:create-pull-request/patch-19lp9o0

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

With unresolvedTemplateArguments (bleeding edge), an object created with a known template argument and then given values derived from reading it back kept a too-specific type:

$count = new Box(0);
repeat(5, function () use ($count): void {
	$count->set($count->get() + 1);
});
// Box<0|1>, and "Parameter #1 $value of method Box<int>::set() expects 0|1, 1|2 given."

Now the resolution is generalized when the body widens the argument beyond its initial type, so the result is Box<int> and set() accepts any int.

Changes

  • src/Analyser/Generics/TemplateArgumentSolver.php: in resolveObservation(), when the union of the initial type and the lower bounds is not covered by the initial type, generalize it with GeneralizePrecision::lessSpecific().
  • Updated two expectations that pinned the literal union for an object created with a known argument and then given an unrelated value: template-argument-multiple-sites.php (Coll<1|2|3> → Coll<int>) and TemplateArgumentResolverTest::testLowerBoundsUnionWithTheInitialUnlessASendWins (1|2|'a'|null → int|string|null). The solver cannot tell whether a written value was derived from a read, so any write that widens a known initial type is generalized. This trades literal precision in those cases for soundness.
  • TemplateArgumentSolver has no native mirror (the native TemplateArgumentResolver instantiates the PHP solver), so no turbo-ext change is needed. The new tests pass both with and without the turbo extension loaded.

Root cause

The observation pass reads a not-yet-resolved template argument out of the object (Box<T>::get(): T) as its initial type (0). A value derived from that read and written back (0 + 1) becomes a lower bound, and the argument resolves to 0|1. In the second pass the read returns the resolution 0|1, so the write is 1|2, which Box<0|1> rejects. The resolution has to be a fixpoint of read → derive → write, which is the same situation as a loop's variables. Once writes widen the argument past its initial type, the reads the observation saw were stale, so the resolution is generalized the way loop variables are. Writes that stay within the initial type ($box->set($box->get())) keep the precise type.

The same pattern affected every way of creating the object (new, a static generic factory Box::of(0), a generic function wrap(0)). It showed up in straight-line code, for loops, closures, arrow functions, and closures invoked several times, with int arithmetic, string concatenation, and array appends/shapes. All of these are fixed by the single change in the solver.

Probed and left unchanged: assigning to a template-typed public property ($box->value = 1) is not observed as a lower bound at all (the property write goes through collectSend()). That is a separate gap in AssignHandler, not a too-specific resolution.

Test

  • tests/PHPStan/Analyser/nsrt/bug-15430.php: the reported snippet plus straight-line, for-loop, arrow-function, static-factory, generic-function, string-concat, list-append and array-shape variants, and a write-back-unchanged case that stays precise (Box<0>).
  • tests/PHPStan/Rules/Methods/CallMethodsRuleTest::testBug15430() with the reported snippet: no argument.type error is reported anymore.

Fixes phpstan/phpstan#15430

🤖 Generated with Claude Code

…t beyond its initial type

- TemplateArgumentSolver::resolveObservation(): when the lower bounds put
  into an object (`$box->set(...)`) are not covered by the type it was
  created with, the union of the initial type and the lower bounds is
  generalized (GeneralizePrecision::lessSpecific()). The observation pass
  reads the object as its initial type, so a value derived from a read and
  written back (`$box->set($box->get() + 1)`) is one step of a loop: the
  second pass read `0|1` and wrote `1|2`, which `Box<0|1>` rejected.
- Covers `new`, static generic factories and generic functions creating the
  object, in straight-line code, loops, closures and arrow functions, and
  int/string/array values.
- Writes that stay within the initial type keep the precise resolution.
- Updated two expectations that pinned the literal union after an
  unrelated write widened a known initial type (now generalized).
- Probed: writes to template-typed properties are not observed as lower bounds
  at all (a separate gap in AssignHandler, unchanged here).
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.

unresolvedTemplateArguments keeps the more specific type when it shouldn't

1 participant