Skip to content

Keep the analysed PHP version as the floor of a PHP_VERSION_ID < X guard - #6746

Open
HawkForge wants to merge 2 commits into
phpstan:2.3.xfrom
HawkForge:fix/php-version-id-narrowing-below-configured
Open

HawkForge wants to merge 2 commits into
phpstan:2.3.xfrom
HawkForge:fix/php-version-id-narrowing-below-configured

Conversation

@HawkForge

Copy link
Copy Markdown
Contributor

Fixes phpstan/phpstan#15425. Thanks @lstrojny for the report and the reproducer.

When a single PHP version is analysed (a fixed phpVersion, or no version range from the config or composer.json), PHP_VERSION_ID is int<50207, max>. Scope::getPhpVersion() treats that as "no information" and answers with the analysed version. A guard like if (PHP_VERSION_ID < 80400) narrows the constant to int<50207, 80399>, and getPhpVersion() returned that range as is, so inside the branch the version dropped to PHP 5.2.7. Since 2.3 the "supported only on PHP X" rules read the version from the scope, so with phpVersion: 80500 a class with promoted properties in that branch got the non-ignorable property.promotedNotSupported error. A guard the analysed version passes, like PHP_VERSION_ID < 80600, hit the same error.

Only the upper bound of such a range comes from the guard. getPhpVersion() now raises the lower bound to the analysed version, or to the guard's upper bound when the guard excludes the analysed version. With phpVersion: 80500, PHP_VERSION_ID < 80600 gives int<80500, 80599> and PHP_VERSION_ID < 80400 gives 80399. Guards that set a lower bound (PHP_VERSION_ID >= 80300, the bug-13133 case) and configured version ranges are unchanged. A PHP_VERSION_ID < 80000 branch still reports promoted properties, as testConditionallyDeclaredClass expects. The narrowed type of the PHP_VERSION_ID constant itself does not change.

MutatingScope is shadowed by turbo-ext, so MutatingScope.cpp gets the same change and the generated header is regenerated. turbo-ext/tests/scope-family.php gets two guarded scopes pinned to phpVersion: 80500 (one where the guard excludes the analysed version, one where it doesn't). The smoke test fails on them if the native side is wrong. A separate commit bumps the expected turbo version.

InvalidPromotedPropertiesRuleTest::testBug15425 analyses the issue's snippet and expects no errors. Without the fix it fails with Promoted properties are supported only on PHP 8.0 and later. The full test suite passes with and without the turbo extension loaded.

Closes phpstan/phpstan#15425

When a single PHP version is analysed (a fixed `phpVersion`, or no version
range from the config or composer.json), PHP_VERSION_ID is int<50207, max>
and Scope::getPhpVersion() treats that as no information and answers with
the analysed version. A guard like `if (PHP_VERSION_ID < 80400)` narrows
the constant to int<50207, 80399>, and getPhpVersion() returned that range
as is, so the version dropped to PHP 5.2.7 inside the branch. Since the
"supported only on PHP X" rules read the version from the scope, a class
with promoted properties in such a branch got a non-ignorable
property.promotedNotSupported error with `phpVersion: 80500`.

Only the upper bound of that range comes from the guard. Raise the lower
bound to the analysed version, or to the guard's upper bound when the guard
excludes the analysed version. Guards with a lower bound
(`PHP_VERSION_ID >= 80300`) and configured version ranges are unchanged.
The turbo-ext mirror gets the same change.

Closes phpstan/phpstan#15425

@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.

this PR looks like changing way more conditions than it adds tests. could you double check all changes are covered by tests (or alternativly remove unrelated changes)?

please rebase

namespace Bug15425;

if (PHP_VERSION_ID < 80400) {
\PHPStan\dumpType(PHP_VERSION_ID);

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.

please drop the debug statement

@staabm

staabm commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

//cc @SanderMuller


namespace Bug15425;

if (PHP_VERSION_ID < 80400) {

@staabm staabm Oct 17, 2026 •

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.

please add another variant of this condition in the test in which the previously reported false-positive-error would be valid

@SanderMuller SanderMuller 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.

I checked dea7568 against its base 55df31e, with the reporter's config (phpVersion: 80500, level 0):

  • The base reports property.promotedNotSupported and this PR does not, for four cases:
    • the issue's snippet,
    • a < 80600 guard,
    • the polyfill guard from the issue (< 80400 && !class_exists(...)),
    • the else branch of >= 80400.
  • < 80000 and >= 70100 && < 80400 still report it on both.
  • With phpVersion: {min: 80100, max: 80500} the output is the same on both.
  • testBug15425 fails without the change in MutatingScope.

On the coverage question, I ran the full suite once per mutation of the new branch:

  • Branch switched off: only testBug15425 fails.
  • Floor always the analysed version: 25 failures in 21 test methods.
  • No getMin() === PHP_MIN_ANALYZABLE_VERSION_ID check: 3 tests fail.
  • No getConfiguredPhpVersionRange() === null check: nothing fails.
  • Floor always the guard's maximum, so no min(): nothing fails.

So two of the conditions have no test, and both change results:

  • The configured-range check matters for a Composer constraint without a lower bound. With "php": "<8.4" and a < 80300 guard, this PR reports the promoted-property error, which is correct. Without the check, the error goes away.
  • The min() matters when the analysed version is below the guard. Take phpVersion: 80100 and a readonly class in a < 80400 branch. This PR reports "Readonly classes are supported only on PHP 8.2 and later.", which is correct. Without the min(), the error goes away.

A test for each would cover the rest of the change. ScopePhpVersionTest, or rule tests with their own config, could hold them.

For the variant where the error is valid: testConditionallyDeclaredClass already has the < 80000 case, and it goes through the new branch. It is one of the tests that fail when the floor is always the analysed version. A variant in bug-15425.php would still put it next to the fix. I agree the dumpType() call can go, because the rule test does not report it.

The extraction of getConfiguredPhpVersionRange() keeps the old behaviour: the inverted condition is the same test, and the configured-range runs above show no change. The C++ mirror follows the PHP branch for branch. I did not build the extension locally, but the Turbo differential and make tests jobs pass on this head.

Cold self-analysis, 3 interleaved rounds without Turbo: the median is 16.55 s on the base and 17.04 s with this PR. The runs took 16.50 to 17.05 s and 16.55 to 17.19 s, so there is no measurable difference. The new branch does not run there, because phpstan-src has a Composer PHP range.

make tests, make phpstan and phpcs pass on this head. The 4 red jobs also fail on #6754 and #6756. For the rebase, only the version in TurboExtensionEnabler.php conflicts.

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.

2.3.0: property.promotedNotSupported in a PHP_VERSION_ID-guarded branch when phpVersion is fixed

3 participants