Skip to content

Use getType() instead of getNativeType() for loop iteration detection when treatPhpDocTypesAsCertain is false#5524

Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-jt1o394
Open

Use getType() instead of getNativeType() for loop iteration detection when treatPhpDocTypesAsCertain is false#5524
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-jt1o394

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When treatPhpDocTypesAsCertain is set to false, for and while loops used getNativeType() to evaluate whether the loop condition is always true (determining $isIterableAtLeastOnce). Since built-in functions like max() return mixed as their native type, a condition like 0 <= max(0, $n) evaluated to bool instead of true, making PHPStan think the loop might not execute. This caused the pre-loop scope (with e.g. $total = 0) to be merged into the post-loop scope, widening types incorrectly.

Changes

  • src/Analyser/NodeScopeResolver.php: Changed $isIterableAtLeastOnce determination in for loops (line ~1665) to always use getType() instead of switching based on treatPhpDocTypesAsCertain
  • src/Analyser/NodeScopeResolver.php: Changed $beforeCondBooleanType in while loops (line ~1445) to always use getType() — this variable is used for both the "never enters" check and $isIterableAtLeastOnce
  • src/Analyser/NodeScopeResolver.php: Changed $condBooleanType in while loops (line ~1496) to always use getType() — used for $alwaysIterates and $neverIterates
  • src/Analyser/NodeScopeResolver.php: Changed $condBooleanType in do-while loops (line ~1594) to always use getType() — used for $alwaysIterates

Analogous cases probed

  • foreach loop: Already uses $scope->getType() for isIterableAtLeastOnce() — no fix needed
  • for loop $alwaysIterates: Already uses $bodyScope->getType() unconditionally — no fix needed
  • if/elseif statements: Also switch based on treatPhpDocTypesAsCertain, but these control branch reachability (different concern) — left unchanged
  • do-while loop: Fixed for consistency, as $alwaysIterates was using the same conditional pattern

Root cause

The treatPhpDocTypesAsCertain flag was designed to control error reporting — whether errors like "comparison is always true" should be reported when the type comes from PHPDoc. However, it was also being used to control flow analysis (scope merging after loops), where using native types is too imprecise because built-in functions like max() have native return type mixed even though PHPStan's type extensions know the precise return type.

The inconsistency was visible within the same code: the for loop's $alwaysIterates already used getType() unconditionally, while $isIterableAtLeastOnce switched based on the flag. And foreach always used getType() for its equivalent check.

Test

  • tests/PHPStan/Analyser/Bug14522Test.php + tests/PHPStan/Analyser/data/bug-14522.php + tests/PHPStan/Analyser/bug-14522.neon: Regression test with treatPhpDocTypesAsCertain: false that verifies correct type inference for $total after for and while loops that are guaranteed to execute at least once
  • tests/PHPStan/Analyser/nsrt/bug-14522.php: NSRT test with default settings covering for loop always-enters, never-enters, maybe-enters, and while loop always-enters and maybe-enters

Fixes phpstan/phpstan#14522

…tion when `treatPhpDocTypesAsCertain` is false

- Change `for` loop `$isIterableAtLeastOnce` to always use `getType()` for
  condition evaluation, matching `foreach` behavior which already uses
  `getType()` unconditionally
- Apply the same fix to `while` loop `$beforeCondBooleanType` and
  `$condBooleanType` (used for `$isIterableAtLeastOnce` and
  `$alwaysIterates`)
- Apply the same fix to `do-while` loop `$condBooleanType` (used for
  `$alwaysIterates`)
- The `for` loop's own `$alwaysIterates` already used `getType()`
  unconditionally, making the `$isIterableAtLeastOnce` switch inconsistent
- `foreach` was already correct — it uses `$scope->getType()` for
  `isIterableAtLeastOnce()` detection
- `if/elseif` left unchanged — those control branch reachability rather
  than scope merging
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.

1 participant