Preserve conditional expression holders across more scope merges - #6385
Open
ondrejmirtes wants to merge 3 commits into
Open
Preserve conditional expression holders across more scope merges#6385ondrejmirtes wants to merge 3 commits into
ondrejmirtes wants to merge 3 commits into
Conversation
… in the other branch An if-merge intersects conditional expression holders by exact key, so a holder recorded only in one branch is dropped even when the other branch's flat state for the holder's target already satisfies the consequent - a subtype with at-least-as-strong certainty. In that case the consequent holds on both paths whenever the guard matches, so firing the holder later is sound and the holder can survive the merge alongside the vacuous-guard rescue. Closes phpstan/phpstan#9628 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA
… across merges Loop convergence merges the body-end scope with the loop-entry scope, and both sides often carry a Yes-certainty holder for the same target under the same guard - just with different consequent types (the pre-loop assignment's type on one side, the body reassignment's type on the other). The exact-key intersection dropped both, losing the guard-implies-defined fact and producing "might not be defined" next to "if condition is always true". Whichever side a merged path came from, the guard matching later implies one of the recorded consequents, so a holder with the union of the consequent types under their shared certainty holds on every merged path. Closes phpstan/phpstan#6830 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA
Collaborator
|
You've opened the pull request against the latest branch 2.3.x. PHPStan 2.3 is not going to be released for months. If your code is relevant on 2.2.x and you want it to be released sooner, please rebase your pull request and change its target to 2.2.x. |
An ErrorType consequent (e.g. an access to an undefined property) is a subtype of everything, so it looked "already satisfied" and its holder was preserved across the merge. Applying it tracked the expression and hid the underlying error - a false negative on undefined-property access. Skip holders whose consequent or other-branch state is an ErrorType, and use early exit in the preservation loop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Keeps conditional-expression holders alive across two more merge situations:
Regression tests added; the full
NodeScopeResolverTeststays green and a local issue-bot run confirms the two fixes with no regressions.Closes phpstan/phpstan#9628
Closes phpstan/phpstan#6830
🤖 Generated with Claude Code