optional array parameter causes incompatible prototype generation

[greg0ire]

Raw error message I get when running phpspec:

- it produces a result item with a path to the work route
      warning: Declaration of Double\EMA\MD\Domain\Model\Work\Work\P5::fromNative(string $contentId, string $originalTitle,
      string $workType, string $birthday, string $durationSpec, string $countryOfOrigin, string $language, string $title,
      ?string $identifier = NULL, array $alternateIdentifiers = Array): EMA\MD\Domain\Model\Work\Work should be compatible
      with EMA\MD\Domain\Model\Work\Work::fromNative(string $contentId, string $originalTitle, string $workType, string
      $birthday, string $durationSpec, string $countryOfOrigin, string $language, string $title, ?string $identifier = NULL,
      ?array $alternateIdentifiers = Array): EMA\MD\Domain\Model\Work\Work in
      /var/www/html/vendor/phpspec/prophecy/src/Prophecy/Doubler/Generator/ClassCreator.php(49)
       0 vendor/phpspec/phpspec/src/PhpSpec/Runner/Maintainer/ErrorMaintainer.php:121
         throw new PhpSpec\Exception\Example\ErrorException("warning: Declaration of D...")
       1 vendor/phpspec/prophecy/src/Prophecy/Doubler/Generator/ClassCreator.php:49
         eval()
       2 vendor/phpspec/prophecy/src/Prophecy/Doubler/Doubler.php:142
         Prophecy\Doubler\Generator\ClassCreator->create("Double\EMA\MD\Domain\Mode...", [obj:Prophecy\Doubler\Generator\Node\ClassNode])
       3 vendor/phpspec/prophecy/src/Prophecy/Doubler/Doubler.php:105
         Prophecy\Doubler\Doubler->createDoubleClass([obj:ReflectionClass], [array:0])
       4 vendor/phpspec/prophecy/src/Prophecy/Doubler/LazyDouble.php:122
         Prophecy\Doubler\Doubler->double([obj:ReflectionClass], [array:0])
       5 vendor/phpspec/prophecy/src/Prophecy/Prophecy/ObjectProphecy.php:114
         Prophecy\Doubler\LazyDouble->getInstance()
       6 vendor/phpspec/prophecy/src/Prophecy/Prophecy/MethodProphecy.php:49
         Prophecy\Prophecy\ObjectProphecy->reveal()
       7 vendor/phpspec/prophecy/src/Prophecy/Prophecy/ObjectProphecy.php:256
         Prophecy\Prophecy\MethodProphecy->__construct([obj:Prophecy\Prophecy\ObjectProphecy], "contentId", [obj:Prophecy\Argument\ArgumentsWildcard])
       8 vendor/phpspec/phpspec/src/PhpSpec/Wrapper/Collaborator.php:69
         Prophecy\Prophecy\ObjectProphecy->__call("contentId", [obj:Prophecy\Argument\ArgumentsWildcard])
       9 [internal]
         spec\App\Infrastructure\Assembler\DoctrineWorkResultItemAssemblerSpec->it_produces_a_result_item_with_a_path_to_the_work_route([obj:PhpSpec\Wrapper\Collaborator], [obj:PhpSpec\Wrapper\Collaborator], [obj:PhpSpec\Wrapper\Collaborator])
      10 vendor/phpspec/phpspec/src/PhpSpec/Runner/ExampleRunner.php:150
         ReflectionMethod->invokeArgs([obj:spec\App\Infrastructure\Assembler\DoctrineWorkResultItemAssemblerSpec], [array:3])
      11 vendor/symfony/console/Application.php:906
         Symfony\Component\Console\Command\Command->run([obj:Symfony\Component\Console\Input\ArgvInput], [obj:Symfony\Component\Console\Output\ConsoleOutput])
      12 vendor/symfony/console/Application.php:224
         Symfony\Component\Console\Application->doRunCommand([obj:PhpSpec\Console\Command\RunCommand], [obj:Symfony\Component\Console\Input\ArgvInput], [obj:Symfony\Component\Console\Output\ConsoleOutput])
      13 vendor/phpspec/phpspec/src/PhpSpec/Console/Application.php:89
         Symfony\Component\Console\Application->doRun([obj:Symfony\Component\Console\Input\ArgvInput], [obj:Symfony\Component\Console\Output\ConsoleOutput])
      14 vendor/phpspec/phpspec/bin/phpspec:25
         Symfony\Component\Console\Application->run()
      15 vendor/phpspec/phpspec/bin/phpspec:27
         {closure}("4.2.0")

Diff between signatures

- fromNative(…, ?string $identifier = NULL, array $alternateIdentifiers = Array): …
+ fromNative(…, ?string $identifier = NULL, ?array $alternateIdentifiers = Array): …

Notice how things are ok for the $identifier parameter, which is of type string.

[ciaranmcnulty]

Probably significant that $identifier's default is NULL

[greg0ire]

Yeah that's indeed probably more significant than the array type

[greg0ire]

Hmmm maybe my signature is stupid... I mean if I pass null, my code doesn't work, I think.

[ciaranmcnulty]

Can you show the actual method signature of your concrete class?

[greg0ire]

    public static function fromNative(
        string $contentId,
        string $originalTitle,
        string $workType,
        string $birthday,
        string $durationSpec,
        string $countryOfOrigin,
        string $language,
        string $title,
        ?string $identifier = null,
        ?array $alternateIdentifiers = []
    ): self {

I now realise it is indeed stupid to do that, but it's valid php, so... should work.

[ciaranmcnulty]

So possibly https://github.com/phpspec/prophecy/blob/master/src/Prophecy/Doubler/Generator/ClassCodeGenerator.php#L98 is receiving the wrong value from isNullable() for some reason?

[greg0ire]

some reason

Some reason being that the default value isn't null

[michalbundyra]

See #377, I think it resolves the issue

[weierophinney]

Just for the record:

We have an class in zend-expressive-router where we are updating it to have the following signature:

public static function fromRouteFailure(?array $allowedMethods = []) : self

This particular change triggers the error reported above.

We have a reason for the defaults:

• null indicates any method is allowed. This is not the default behavior, though!
• An empty array means no method is allowed. This is the default.
• Otherwise, a populated array with the more specific subset of methods will be provided.

We could potentially alter this to make the value required. That said, I don't see this as an exceptional situation; I can think of a few other cases (such as a nullable string with a default argument) where this might be of use.

The problem only occurs once you're testing on PHP 7.1 and above, which is why we just noticed it (as we're just now updating the library to depend on 7.1+).

I can verify #377 fixes the issue, and would love to see a bugfix release we could pin against that includes it! 😁

[phonixor]

booleans also don't work

public function findOneActiveBySystemName(string $system_name, bool $visibility = true): ProductInterface
public function findOneActiveBySystemName(string $system_name, ?bool $visibility = true): ProductInterface

[thatside-zaraffa]

same for ints:
getByIssuerIds(array $ids, int $limit = 100, int $offset = 0): array
getByIssuerIds(array $ids, ?int $limit = 100, ?int $offset = 0): array