Skip to content

Fix check for impure property hooks - #6246

Open
AJenbo wants to merge 2 commits into
phpstan:2.2.xfrom
AJenbo:pure-hooks
Open

Fix check for impure property hooks#6246
AJenbo wants to merge 2 commits into
phpstan:2.2.xfrom
AJenbo:pure-hooks

Conversation

@AJenbo

@AJenbo AJenbo commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

The closure gathering impure points from a property hook body wrote to $hookImpurePoints (captured by reference, never initialized), while the code afterward merged in $methodImpurePoints. Property-hook impurity data was silently discarded. Fixed by renaming consistently to $hookImpurePoints.

This issue is probably latents since no build in rule consumes PropertyHookReturnStatementsNode::getImpurePoints() yet.

@ondrejmirtes

Copy link
Copy Markdown
Member

This tells me there's probably something more lacking in this area.

  1. @phpstan-pure/@phpstan-impure above property hooks being checked with a rule (missing PureMethodRule equivalent).
  2. If we do $foo->bar today it probably doesn't generate any impure points. Which is fine because otherwise it'd be very annoying (so this default should be different to how methods behave where the call is possibly-impure by default). But if we're certain that $foo->bar is hooked and we can read hasSideEffects()->yes() about the concrete property hook it SHOULD generate an impure point.

@AJenbo

AJenbo commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Indeed they where always set to be impure and there was no checks either way when accessed inside pure functions.

I implemented the checks for this, with the opposite assumption as methods so that the maybe case doesn't end up being annoying for users.

Should I also implement the check from PHPantom that originally found this issue (in another PR)?

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 participants