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

Support intersection types (PHP 8.1+ / ported from v2 to v3) #209

Merged
merged 2 commits into from
Feb 5, 2022

Conversation

bzikarsky
Copy link
Contributor

See #195 for details.

Unrelated: I fixed an E_DEPRECATED on 8.1 in RejectedPromiseTest (Deprecated: Exception::__construct(): Passing null to parameter #1 ($message) of type string is deprecated in /code/tests/Internal/RejectedPromiseTest.php on line 40) - Are you fine with adding this in an additional commit to the PR or do you want to have a separate PR (and more rebase action)?

@clue clue changed the title Port union-type support in _checkTypehint from v2 to v3 Support intersection types (PHP 8.1+ / ported from v2 to v3) Feb 5, 2022
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.

@bzikarsky Thank you for taking over and porting this also to Promise v3! 👍

The changes LGTM, but I've noticed some potential to simplify this given we can finally(!) rely on more recent PHP features (#149). Can you look into this?

src/functions.php Outdated Show resolved Hide resolved
tests/FunctionCheckTypehintTest.php Outdated Show resolved Hide resolved
@clue
Copy link
Member

clue commented Feb 5, 2022

Unrelated: I fixed an E_DEPRECATED on 8.1 in RejectedPromiseTest (Deprecated: Exception::__construct(): Passing null to parameter #1 ($message) of type string is deprecated in /code/tests/Internal/RejectedPromiseTest.php on line 40) - Are you fine with adding this in an additional commit to the PR or do you want to have a separate PR (and more rebase action)?

@bzikarsky I can indeed see the same error on PHP 8.1 (#199) and you're right that this seems to be unrelated to this PR, so perhaps better file this as a separate PR? :shipit:

@bzikarsky
Copy link
Contributor Author

Thanks @clue for going over it.

Those 7.1 related simplifications make sense and I can also supply that extra fix in a separate PR. It may have to wait until Monday though - depending on what my family is up to over the weekend. 🙂

@bzikarsky
Copy link
Contributor Author

Improvements are in. 🙂

Unrelated: I fixed an E_DEPRECATED on 8.1 in RejectedPromiseTest (Deprecated: Exception::__construct(): Passing null to parameter #1 ($message) of type string is deprecated in /code/tests/Internal/RejectedPromiseTest.php on line 40) - Are you fine with adding this in an additional commit to the PR or do you want to have a separate PR (and more rebase action)?

@bzikarsky I can indeed see the same error on PHP 8.1 (#199) and you're right that this seems to be unrelated to this PR, so perhaps better file this as a separate PR? :shipit:

PR is at #210.

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.

@bzikarsky Thanks for the update, changes LGTM, keep it up! :shipit:

@clue clue added this to the v3.0.0 milestone Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants