phpspec / prophecy

Highly opinionated mocking framework for PHP 5.3+
MIT License
8.53k stars 241 forks source link

Problem with inherited self return types #502

Closed giovannialbero1992 closed 4 years ago

giovannialbero1992 commented 4 years ago

I have this behavior with the new minor version, maybe is a bug.

When I do a prophecy of an extended class that inherits a method with self return type I've this error:

Fatal error: Declaration of Double\Fixtures\Prophecy\ClassExtendAbstractWithMethodWithReturnType\P6::test(?DateTimeInterface $test = NULL): Fixtures\Prophecy\ClassExtendAbstractWithMethodWithReturnType must be compatible with Fixtures\Prophecy\AbstractBaseClassWithMethodWithReturnType::test(?DateTimeInterface $test): Fixtures\Prophecy\AbstractBaseClassWithMethodWithReturnType in /home/travis/build/giovannialbero1992/prophecy/src/Prophecy/Doubler/Generator/ClassCreator.php(49) : eval()'d code on line 2

This error is raised into PHP versions <=7.3

ciaranmcnulty commented 4 years ago

Thanks for reporting, the reproducible test case is really great

lolli42 commented 4 years ago

Also reported in issue #503

alexander-schranz commented 4 years ago

Can confirm this issue we have a mock of our StructureMetadata which implements PropertiesMetadata and now fails with:

PHP Fatal error: Declaration of Double\Sulu\Component\Content\Metadata\StructureMetadata\P23::setResource(string > $resource): Sulu\Component\Content\Metadata\StructureMetadata must be compatible with Sulu\Component\Content> \Metadata\PropertiesMetadata::setResource(string $resource): Sulu\Component\Content\Metadata\PropertiesMetadata in > C:\projects\sulu\vendor\phpspec\prophecy\src\Prophecy\Doubler\Generator\ClassCreator.php(49) : eval()'d code on line 2

ciaranmcnulty commented 4 years ago

Yes we're generating the return type wrongly (we are basically behaving as if it's static) but this is allowed via PHP 7.4's type widening, which is why it's only failing on older versions.

Need to see how the old code coped with it (without a test case) and port the logic over.

ciaranmcnulty commented 4 years ago

Thanks @giovannialbero1992 I've moved your commits to #504 and think it's fixed there