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 | 'static' is an invalid class name #527

Closed
Tracked by #34
AlexandruGG opened this issue Mar 18, 2021 · 14 comments
Closed
Tracked by #34

PHP 8 | 'static' is an invalid class name #527

AlexandruGG opened this issue Mar 18, 2021 · 14 comments
Labels

Comments

@AlexandruGG
Copy link

AlexandruGG commented Mar 18, 2021

Hello, I'm seeing a fatal error trying to use Prophecy with PHPUnit in a Symfony 5 project running PHP 8 with the new static return type. Here is the error:

PHP Fatal error:  '\static' is an invalid class name in /Users/alex/dev/project/vendor/bin/.phpunit/phpunit-9.5-0/vendor/phpspec/prophecy/src/Prophecy/Doubler/Generator/ClassCreator.php(49) : eval()'d code on line 11

And a simplified version of the Entity I'm trying to create a prophecy for:

class MyEntity {
    /**
     * @ORM\Column(name="id", type="integer", nullable=false, options={"unsigned"=true})
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    private int $id;

    /**
     * @ORM\Column(name="name", type="string", length=50, nullable=false)
     */
    private string $name;

    public function getId(): int
    {
        return $this->id;
    }

    public function getName(): string
    {
        return $this->name;
    }

    public function setName(string $name): static // <--- if I replace this with 'self' it works
    {
        $this->name = $name;

        return $this;
    }
}

Is this a known issue with PHP 8? Apologies if so but I couldn't find any mention of it.

Or am I doing something wrong with this usage?

Thanks

@stof
Copy link
Member

stof commented Mar 18, 2021

Support for this new return type has not been added yet in Prophecy.

However, I'm not quite sure how to implement that properly regarding the value actually returned by the double (when Prophecy guesses the returned value because it is not set explicitly).

@ciaranmcnulty
Copy link
Member

I think it only needs to be handled the same way void is here:
https://github.com/phpspec/prophecy/blob/master/src/Prophecy/Doubler/Generator/Node/ReturnTypeNode.php#L11-L13

There's no need to resolve the class, as this is valid:


class A {
    function foo() : static {}
}
class B extends A {
    function foo() : static {}
}

@ciaranmcnulty
Copy link
Member

@ciaranmcnulty
Copy link
Member

Oh hm you're right @stof that is tricky

@stof
Copy link
Member

stof commented Mar 18, 2021

@ciaranmcnulty generating the right signature is easy. But generating the right behavior is not.

My own opinion is that Prophecy should drop all that logic, and expect all calls to be explicitly configured (which solves that case for static too, as it is then the responsibility of the dev to specify either ->willReturnSelf() or another valid return value returning another double instance). #526 is already bringing back the immediate UnexpectedCallException. My suggestion would then only mean removing the special case of an ObjectProphecy without anything configured in it. Today, it accepts any call (which stops as soon as you configure one). In my proposal, it would expect having no calls being done on it. I think that this is much less surprising, as Prophecy cannot properly determine what the call should return anyway.

@ciaranmcnulty
Copy link
Member

In the short term (+ in prophecy 1.x) we can have a special case for static and throw an UnexpectedCallException maybe

@RobinHoutevelts
Copy link

I'm hitting this as well..

Any workaround besides dropping the use of the static return type in my code?

@AlexandruGG
Copy link
Author

I'm hitting this as well..

Any workaround besides dropping the use of the static return type in my code?

@RobinHoutevelts this is the workaround I'm using for now. Works with static analysers like PHPStan and Psalm:

/**
* @return static
*/
public function setName(string $name): self
    {
        $this->name = $name;

        return $this;
    }

@RobinHoutevelts
Copy link

RobinHoutevelts commented Jun 5, 2021

irontec@2d17db5 kinda did it for me as wel.

Not sure what impact it has but at least my tests are green.

Thanks for the help!

@alsar
Copy link

alsar commented Oct 14, 2021

The fix from irontec@2d17db5 works. Could this be merged?

@stof
Copy link
Member

stof commented Oct 14, 2021

Well, nobody has submitted a PR with a fix yet

@ciaranmcnulty
Copy link
Member

Yeah we don't scour forks for fixes; if someone can open a PR and write a test to go with it we can consider merging

@dzianis-hrynko
Copy link
Contributor

Here is fix of the issue
I've checked on my project
#545

Jean85 pushed a commit to Jean85/prophecy that referenced this issue Nov 9, 2021
dantleech added a commit to phpbench/phpbench that referenced this issue Dec 4, 2021
dantleech added a commit to phpbench/phpbench that referenced this issue Dec 4, 2021
* Allow Symfony 6

* Switch to phpunit mocking for Process test

Avoid bug in Prophecy phpspec/prophecy#527

* Fix CS

Co-authored-by: Boudry Julien <[email protected]>
stof added a commit that referenced this issue Dec 8, 2021
@stof stof closed this as completed Dec 8, 2021
@stof
Copy link
Member

stof commented Dec 8, 2021

Fixed in #545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants