Skip to content

Declare Memcached methods as impure#5536

Merged
VincentLanglet merged 7 commits intophpstan:2.1.xfrom
VincentLanglet:bug13444
Apr 27, 2026
Merged

Declare Memcached methods as impure#5536
VincentLanglet merged 7 commits intophpstan:2.1.xfrom
VincentLanglet:bug13444

Conversation

@VincentLanglet
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet commented Apr 26, 2026

@phpstan-bot
Copy link
Copy Markdown
Collaborator

You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x.

@VincentLanglet VincentLanglet changed the base branch from 2.2.x to 2.1.x April 26, 2026 08:00
@VincentLanglet VincentLanglet marked this pull request as ready for review April 26, 2026 12:15
@VincentLanglet VincentLanglet requested a review from staabm April 26, 2026 12:15
@phpstan-bot
Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

Comment thread stubs/Memcached.stub
Copy link
Copy Markdown
Contributor

@staabm staabm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood #4344 (comment) correctly, it seems some Memcached methods should still be pure

@VincentLanglet
Copy link
Copy Markdown
Contributor Author

If I understood #4344 (comment) correctly, it seems some Memcached methods should still be pure

I follow the same strategy than used for Redis in #4422.

By default, all the method are marked as impure since it's 99% of them and then if an issue is opened about a specific method we'll update the stubs ; but so far I won't recommend adding complexity by searching for possibly pure method while we're not fully sure about it.

@VincentLanglet VincentLanglet requested a review from staabm April 27, 2026 08:05
/**
* @phpstan-all-methods-impure
*/
class SplFileObject {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should/can SplFileObject::* be removed now from resources/functionMetadata.php?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a test file, it's just to prove that pure annotation is correctly handled for native classes as #5539 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand yet, why we will not ship @phpstan-all-methods-impure for SplFileObject, but do/can for Memcache.

other that that this LGTM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know SplFileObject was impure, I never use this class.
I just focus on the open issue.

Feel free to refactor SplFileObject to use @phpstan-all-methods-impure if you want to

There is maybe more class which can be refactored the same way

Comment thread tests/PHPStan/Rules/Comparison/data/bug-14534.php
@VincentLanglet VincentLanglet merged commit fe750c5 into phpstan:2.1.x Apr 27, 2026
659 of 661 checks passed
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.

Recognize the \Memcached::cas() can change the \Memcached::getResultCode()

3 participants