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.0 compatibility #491

Closed
garak opened this issue Jul 15, 2020 · 15 comments
Closed

PHP 8.0 compatibility #491

garak opened this issue Jul 15, 2020 · 15 comments

Comments

@garak
Copy link

garak commented Jul 15, 2020

Can we allow php 8 in composer.json?
I see that some tests are already passing on travis

@Glancu
Copy link

Glancu commented Jul 15, 2020

I add PR - #492

@ciaranmcnulty
Copy link
Member

Not before feature freeze at the very earliest

@GrahamCampbell
Copy link
Contributor

I think revisiting this after PHP 8.0.0 beta 1 and PHPUnit 9.3.0 has been tagged would be a good idea. We might actually be able to install all dependencies without ignoring platform-reqs at that point.

@GrahamCampbell
Copy link
Contributor

I think, that is 10th August, from memory.... something around that date anyway. ;)

@ciaranmcnulty
Copy link
Member

that's what 'feature freeze' is :)

@sebastianbergmann
Copy link
Contributor

PHP 8 is feature-frozen and PHP 8.0.0 Beta 1 was released yesterday. Today PHPUnit 9.3.0 has been released.

@MAXakaWIZARD
Copy link

@sebastianbergmann currently PHPUnit 9.3 cannot be installed under PHP 8.0 because phpspec/prophecy does not allow it. I'm curious how phpunit 9.3 was released despite the fact that dependencies cannot be resolved at the moment. Have you built it under PHP 7.x?

@sebastianbergmann
Copy link
Contributor

Have you built it under PHP 7.x?

Yes.

@TeBoring
Copy link

When can prophecy be released?

@ciaranmcnulty
Copy link
Member

#482 is outstanding. I made some start on it already and may be able to push it over the line in the next few days

If anyone wants to jump in please do

@ciaranmcnulty
Copy link
Member

Feedback on #495 will help

@ciaranmcnulty
Copy link
Member

Review of #495 would be helpful to move forward

@alexpott
Copy link

alexpott commented Sep 29, 2020

EDIT: my bad - had some old code lying around - once I got rid of this everything worked as expected. Apologies.

i don't know whether to create a new issue but there's an issue with creating doubles where the class has a parameter declared as mixed. In this case \Drupal\Core\Php8\Phpspec\Prophecy\ClassMirror::isNullable() returns true because mixed allows null but null !== $this->getTypeHint($parameter). Then \Prophecy\Doubler\Generator\Node\TypeNodeAbstract::guardIsValidType() throws an exception because

        if (\PHP_VERSION_ID >= 80000 && isset($this->types['mixed']) && count($this->types) !== 1) {
            throw new DoubleException('mixed cannot be part of a union');
        }

I think the solution is to change isNullable() to

    private function isNullable(ReflectionParameter $parameter)
    {
        $typehint = $this->getTypeHint($parameter);
        return $parameter->allowsNull() && !(null === $typehint || 'mixed' === $typehint);
    }

Because in PHP 8.0 nullable mixed types are not allowed. See https://php.watch/versions/8.0/mixed-type#nullable

@garak
Copy link
Author

garak commented Dec 4, 2020

Is there anything still blocking this?

@Glancu
Copy link

Glancu commented Dec 4, 2020

We can close. sonata-project/SonataAdminBundle#6476

@stof stof closed this as completed Dec 4, 2020
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

No branches or pull requests

9 participants