Skip to content
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

[4.x] Improve type safety for test environment #86

Merged
merged 1 commit into from
May 22, 2024

Conversation

SimonFrings
Copy link
Member

This pull request adds some missing Promise v3 template types that have been reported by PHPStan. The same changes should also be applied to the 3.x branch, so once this is merged I will create a follow-up PR to do so.

Builds on top of reactphp/promise#247, clue/framework-x/pull/235, #76 and others

@SimonFrings SimonFrings added the new feature New feature or request label May 2, 2024
@SimonFrings SimonFrings added this to the v4.3.0 milestone May 2, 2024
@WyriHaximus
Copy link
Member

@SimonFrings PHPStan 1.11 came out a couple of days ago, did you run this again it? I know updating versions will always be a thing. But 1.11 introduces error identifiers which would make the ignore lines a bit cleaner and more specific what is being ignored: https://phpstan.org/blog/phpstan-1-11-errors-identifiers-phpstan-pro-reboot

Copy link
Member Author

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

I updated the PR a bit, happy about your input on this 👍

PHPStan 1.11 came out a couple of days ago, did you run this again it? know updating versions will always be a thing. But 1.11 introduces error identifiers which would make the ignore lines a bit cleaner and more specific what is being ignored: […]

Tried this but didn't help with the problem I was running into (see below). We could go on PHPStan 11 but I think this is better suite for a follow-up PR.

src/functions.php Outdated Show resolved Hide resolved
src/functions.php Outdated Show resolved Hide resolved
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

@SimonFrings Thanks for the update, changes LGTM! :shipit:

The work-around looks good enough to me, but glad you've reported this upstream (phpstan/phpstan#11032) so we can revisit this in the future 👍

@WyriHaximus WyriHaximus merged commit 16d653f into reactphp:4.x May 22, 2024
6 checks passed
@SimonFrings SimonFrings changed the title [4.x] Use Promise v3 template types [4.x] Improve type safety for test environment Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants