Invalidate maybe-impure function return values after impure method/static calls#5667
Open
phpstan-bot wants to merge 1 commit into
Open
Invalidate maybe-impure function return values after impure method/static calls#5667phpstan-bot wants to merge 1 commit into
phpstan-bot wants to merge 1 commit into
Conversation
…atic calls When a maybe-impure function call's return value was narrowed via assert() (e.g. assert(count(MyRecord::find()) === 1)), the narrowed type persisted even after an impure method call like $msg2->insert(). This happened because invalidateExpression() only invalidated expressions containing the specific callee variable, not unrelated maybe-impure expressions whose results could have been affected by the side effects. Add invalidateAllMaybeImpureFunctionReturnValues() to MutatingScope which walks stored expression types and removes any that contain maybe-impure function/method/static calls. Call it from MethodCallHandler and StaticCallHandler when processing calls with definite side effects (hasSideEffects()->yes()), skipping $this-> calls which already invalidate via invalidateExpression(). Closes phpstan/phpstan#13416
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.
Summary
assert(count(MyRecord::find()) === 1), calling an impure method like$msg2->insert()now correctly invalidates the narrowed count, socount(MyRecord::find())returnsint<0, max>instead of staying narrowed to1MutatingScope::invalidateAllMaybeImpureFunctionReturnValues()which removes stored expression types that contain maybe-impure function/method/static callsMethodCallHandlerandStaticCallHandlerfor calls withhasSideEffects()->yes(), skipping$this->calls to avoid over-invalidationDetails
The root cause was that
specifyTypesForCountFuncCall()narrows the argument expression (e.g.,MyRecord::find()->array{0: MyRecord}) and stores it in scope. Sincefind()hashasSideEffects()->maybe()andrememberPossiblyImpureFunctionValuesis enabled, the narrowing persists. When$msg2->insert()runs,invalidateExpression()only invalidates expressions containing$msg2, soMyRecord::find()was never cleared.The fix adds a new method that walks all stored expression types with
NodeFinder, checking each for function calls, method calls, or static calls that are not proven pure (hasSideEffects()->no()). Any expression containing such a call is removed from the scope.Test plan
tests/PHPStan/Analyser/nsrt/bug-13416.phpwith test cases covering:strlen()of impure call invalidated by method callCloses phpstan/phpstan#13416