phpstan-src icon indicating copy to clipboard operation
phpstan-src copied to clipboard

Fix incorrect type inference for @phpstan-assert-if-true on $this with union types

Open yt-catpaw opened this issue 4 months ago • 7 comments

closes https://github.com/phpstan/phpstan/issues/13358 closes https://github.com/phpstan/phpstan/issues/11441

Summary

@phpstan-assert-if-true on $this with union types produced incorrect type inference.

Repro

See added test: tests/PHPStan/Rules/Methods/AssertIfTrueOnThisTest.php.

Cause

Union handling for $this assertions wasn’t propagated into TypeSpecifier (assert-based refinement path).

Fix

Normalize/refine $this union members before producing SpecifiedTypes from assert info.

Tests

  • Added/updated tests in AssertIfTrueOnThisTest.php.
  • Full test suite passes locally.

Also fixes #11441

  • Added regression test in tests/PHPStan/Analyser/nsrt/bug-11441.php.
  • This ensures that @phpstan-assert !null $this->getParam() works correctly when called on union types (Foo|Bar), where each class defines its own method.
  • Previously, the null type was not removed in such cases. After the fix, int|string is correctly inferred.

yt-catpaw avatar Aug 17 '25 07:08 yt-catpaw

please have a look at the issue-bot github action job summary. it seems this PR also affects other issues.

please add regression tests for those which are getting fixed

staabm avatar Aug 18 '25 07:08 staabm

Thanks, @staabm. I’ll review the issue-bot GitHub Action job summary and add regression tests for the additionally affected issues. This is my first time contributing to open source (and my first PR to PHPStan), so I’ll update this PR in small batches as I add tests. If you prefer a specific location or pattern for these tests, please let me know.

yt-catpaw avatar Aug 18 '25 14:08 yt-catpaw

Welcome 🤗

Type related tests go usually into the https://github.com/phpstan/phpstan-src/tree/2.1.x/tests/PHPStan/Analyser/nsrt/ directory.

Files located in this directory will automatically checked against using assertType().

staabm avatar Aug 18 '25 14:08 staabm

Regression tests are typically added to existing test classes for rules that reported the (disappeared) errors.

Alternatively covering some bugs with simple type inference tests (in tests/PHPStan/Analyser/nsrt) is good enough.

ondrejmirtes avatar Aug 18 '25 14:08 ondrejmirtes

@staabm Hi, I’ve moved the test case for bug-13358 into tests/PHPStan/Analyser/nsrt/bug-13358.php, but now I’m not sure how to run just this specific test file.

The reason I want to run it individually is because I’d like to debug a specific function and check only the information related to this test case, without running everything else.

Could you tell me how to do that?

Thanks!

yt-catpaw avatar Aug 19 '25 15:08 yt-catpaw

This pull request has been marked as ready for review.

phpstan-bot avatar Aug 21 '25 15:08 phpstan-bot

Added regression test for #11441 (bug-11441.php) and updated the description accordingly. Let me know if anything else is needed!

yt-catpaw avatar Aug 21 '25 16:08 yt-catpaw