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

PHP 8: method_exists() fixes and composer.json version constraint update #207

Merged
merged 2 commits into from
Jul 5, 2020

Conversation

Ayesh
Copy link
Contributor

@Ayesh Ayesh commented Jun 27, 2020

Ref: sebastianbergmann/phpunit#4325

PHP 8 builds are currently failing because method_exists() calls throw exceptions if the argument is not string|object. This PR fixes these calls by making sure the arguments are string|object before calling method_exists. This is because PHP 8 throws \TypeError and \ValueError exceptions from internal functions.

In preparation for PHP 8, this PR also updates composer.json PHP version constraint to allow PHP 8.

All tests now pass on PHP 8 dev builds.

Ayesh added 2 commits June 28, 2020 00:54
…ring|object

PHP 8 throws `\TypeError` and `\ValueError` exceptions in internal functions (including `method_exists()`) when it encounters an invalid value. For `method_exists`, it expects `string|object`.
See https://php.watch/versions/8.0/internal-function-exceptions

`Assert::methodExists()` and `Assert::methodNotExists()` are updated to make sure the provided argument is string|object before calling `method_exist()` to avoid PHP 8 `\TypeError` exceptions.
@BackEndTea
Copy link
Collaborator

Considering the tests don't need updating, and classes cant be numeric anyways i don't think this is a BC break, so this should be good to merge.

I'm gonna keep the nightly build on allow failure for now, as it could break build if there is a 'bad' version on travis.

@BackEndTea BackEndTea merged commit 69d0ffb into webmozarts:master Jul 5, 2020
@Ayesh
Copy link
Contributor Author

Ayesh commented Jul 5, 2020

Awesome, thanks a lot @BackEndTea .

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.

2 participants