Skip to content

Fix "ob_get_*() === false will always evaluate to false" false positive#6097

Merged
staabm merged 4 commits into
phpstan:2.2.xfrom
staabm:benevo
Jul 24, 2026
Merged

Fix "ob_get_*() === false will always evaluate to false" false positive#6097
staabm merged 4 commits into
phpstan:2.2.xfrom
staabm:benevo

Conversation

@staabm

@staabm staabm commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

@phpstan-bot

Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

@staabm
staabm requested a review from VincentLanglet July 24, 2026 06:18
@VincentLanglet

Copy link
Copy Markdown
Contributor

I don't like this fix cause this just seems to be a hack in order to satisfy one issue but some others could be open like the fact that ob_get_level doesn't return the right value if a ob_start fail.

Honestly I wonder if we should just revert #5964
Looking at the original issue phpstan/phpstan#10348 it was already wrong since the author didn't check the return value from ob_start. We shouldn't make the asumption it worked if the author don't check it.

The right fix is technically #6087 but maybe we could simplify it.

@staabm

staabm commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Looking at the original issue phpstan/phpstan#10348 it was already wrong since the author didn't check the return value from ob_start. We shouldn't make the asumption it worked if the author don't check it.

IMO the whole point of this issue was/is, that checking error state return values of ob_* functions is essentially useless as this usually means that your system is out of memory and your process is going to die anyway.

thats why error states are not checked.
and in phpstan/phpstan#14985 error states, which are "not that useful" are checked, which leads to these errors.

so thats why I think benevolent is the right tool for the job, as it helps us to treat not important error states in a less aggressive way

@VincentLanglet

Copy link
Copy Markdown
Contributor

IMO the whole point of this issue was/is, that checking error state return values of ob_* functions is essentially useless as this usually means that your system is out of memory and your process is going to die anyway.

I kinda agree with your fix. I'm just annoyed by the assertiong

assertType('int<1, max>', ob_get_level());

It's like ob_start could technically fail for ob_clean but not for ob_get_level...

Anyway that might be good enough.
Maybe might be worth to add a comment above the benevolentUnion to explain the reason ?

@staabm
staabm merged commit 6e85ec9 into phpstan:2.2.x Jul 24, 2026
737 of 739 checks passed
@staabm
staabm deleted the benevo branch July 24, 2026 08:20
@staabm

staabm commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

thank you @VincentLanglet

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.

ob_get_clean() can return false

3 participants