Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Treat Redis::connect as non-deterministic #4318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ondrejmirtes merged 1 commit into phpstan:2.1.x from vrana:redis
Sep 12, 2025
Merged

Conversation

Copy link
Contributor

@vrana vrana commented Sep 10, 2025
edited
Loading

Copy link
Member

  1. I'd say connect method has to change instead, that will take care of invalidating remembered isConnected when called.
  2. You edited the correct file but you also need to run bin/ script to regenerate the metadats file that doesn't have "original" in its name.

If you're interested, you can check out this part of my talk about pure/impure and so on here: https://youtu.be/AFjr3RlDOZQ?si=dp_XJWfenGJBRQhp (from about 28:50 timestamp)

Copy link
Contributor Author

vrana commented Sep 11, 2025
edited
Loading

Makes sense. Please suggest how can I test this.

Copy link
Contributor

staabm commented Sep 11, 2025
edited
Loading

you can add a new testXX method into tests/PHPStan/Rules/TooWideTypehints/TooWidePropertyTypeRuleTest.php
and put your code from https://phpstan.org/r/1695567f-4869-428f-8cb6-fdd8cb147be1 into the tests data/ directory.

then assert no errors

similar to

public function testBug13384bOff(): void
{
$this->analyse([__DIR__ . '/data/bug-13384b.php'], []);
}

@vrana vrana force-pushed the redis branch 2 times, most recently from c7c8c89 to 21f4d32 Compare September 11, 2025 10:49
Copy link
Contributor Author

vrana commented Sep 11, 2025

I've added the test but I couldn't reproduce the original behavior before this change.

Copy link
Contributor

staabm commented Sep 11, 2025
edited
Loading

Needs $this->reportTooWideBool = true; in the rule test class

Copy link
Contributor Author

vrana commented Sep 11, 2025

Thanks, hallelujah!

@vrana vrana changed the title (削除) Treat Redis::isConnected as non-deterministic (削除ここまで) (追記) Treat Redis::connect as non-deterministic (追記ここまで) Sep 11, 2025
Copy link
Contributor

staabm commented Sep 11, 2025

don't get confused by the CI results.. we currently investigate spurious CI errors

@ondrejmirtes ondrejmirtes merged commit 1de1dad into phpstan:2.1.x Sep 12, 2025
448 of 457 checks passed
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /