Phpstan: False positive with ext-ds

[enumag]

Bug report

This only started on phpstan 0.12.8.

Code snippet that reproduces the problem

        /** @var \Ds\Map<PaymentGroupId, array<Payment>> $paymentsByGroup */
        $paymentsByGroup = new \Ds\Map();
        $paymentGroup = $paymentsByGroup->get($paymentGroupId, []);
        $paymentGroup[] = Payment::fromValues(...);

The last line triggers this error:

Cannot access an offset on array<App\Module\PaymentRegistry\Domain\PaymentGroup\ValueObject\Payment>|TDefault.

Something is wrong here - TDefault is a generic type in Map::get() so it makes no sense that the error refers to it. Also it is actually an array in this case which means that $paymentGroup will always be an array.

Expected output

no error

[mergeable[bot]]

This bug report is missing a link to reproduction on phpstan.org.
It will most likely be closed after manual review.

[ondrejmirtes]

Please try to reproduce the problem on phpstan.org - should be possible if you annotate your own methods the same way as DsMap::get() is annotated.

[enumag]

Unfortunately I can't reproduce it there.
https://phpstan.org/r/1afd4c85-601c-44a9-9dde-51912b1c2d4b

[enumag]

I think I can't reproduce it because the bug is somehow related to the dynamic return type extension. My guess is that this line has to somehow translate the TDefault to the actual type?

[ondrejmirtes]

Yes, it's wrong. You need to use ParametersAcceptorSelector::selectFromArgs like I did here in phpstan-doctrine: https://github.com/phpstan/phpstan-doctrine/commit/5762a215be4a06206ee4cd62df24da3196ebdcc9

[enumag]

The problem is that this is difficult for me to fix now since I can't simply modify phpstan locally to test the changes on my project. I'm sorry but just as I was afraid, the phar makes it impossible for me to contribute such fixes.

[ondrejmirtes]

You just have to do this in your phpstan.neon:

parameters:
    stubFiles!:
        - stubs/ext-ds.stub

And provide the newer versions of the stubs you want to debug in /yourOwnComputer/project/stubs/ext-ds.stub.

[ondrejmirtes]

If you want to run the project with your own local copy of phpstan-src, do this:

cd project
../phpstan-src/bin/phpstan analyse -l 8 -c phpstan.neon src tests

[enumag]

Well phpstan.neon is default config and level and directories are specified in the neon so I'm only using ../../phpstan/phpstan-src/bin/phpstan analyse

First I had some issues that I eventually solved by running composer update in the phpstan-src repo.

Then it failed with this:

File 'phar://phpstan.phar/conf/bleedingEdge.neon' is missing or is not readable.

Then I commented out this line in phpstan.neon:

- vendor/phpstan/phpstan/conf/bleedingEdge.neon

And now it fails with this:

In Resolver.php line 538:

  Service 'rules.103' (type of PHPStan\Rules\Cast\UselessCastRule): Parameter $treatPhpDocTypesAsCertain in __construct() has   
  no class type hint or default value, so its value must be specified.                                                                                                                                   

Sorry but it clearly doesn't work as easily as you believe. I don't have the time to debug such problems.

[ondrejmirtes]

Looks like you're somehow combining incompatible PHPStan versions.

It's fine. Others can pick up the problem if they want to.

[enumag]

I doubt someone will though. So far the only active contributors who're using ext-ds are me and @simPod.

[simPod]

@enumag I use Baseline feature to exclude errors caused by potential bugs in phpstan. I encountered these errors as well (message: "#^Argument of an invalid type Ds\\\\Set\\<array\\>\\|TDefault supplied for foreach, only iterables are supported\\.$#").

Looking into VCS I excluded these errors when upgrading from 0.12.3 to 0.12.5, had no time to investigate it yet 🤔

[ondrejmirtes]

@simPod @enumag Then it's in your interest to fix this bug :) I basically told you how to do it.

[enumag]

@simPod Yeah I'm using that too ofc. I just would like to help fixing it but seems my hands are tied. With the phar version phpstan is getting hard to contribute to. :-(

@ondrejmirtes Yeah I know and I appreciate your help but I can hardly apply your advice without a way to test the result.

[simPod]

@ondrejmirtes as we're talking about it already, what is the preferred way to run phpstan fork within my project? I remember I wanted to debug something month ago or so but had no idea how to require my version of phpstan since it uses phar?

EDIT: @enumag I see we have the same issue :D

[enumag]

I tried to composer require phpstan/phpstan-src to see if I could debug the problem with that instead of the phar but no such package exists on packagist.

[ondrejmirtes]

1) You can create a reproducing case in phpstan-src and debug on that. Which you have to do to create a PR anyway.
2) You can also do this (link from this thread): https://github.com/phpstan/phpstan/issues/2909#issuecomment-581388881

Both seems good enough given the tremendous benefits PHAR is giving to us.

@enumag Yeah, I really don't want anyone to use phpstan-src package, it would fragment the userbase.

[enumag]

@ondrejmirtes You could add it but mark it as abandoned to have a warning thrown if someone uses it. So it would only be used by those who know what they're doing (like us for debugging).

[ondrejmirtes]

1) People don't read warnings.
2) It wouldn't work as extensions do not require phpstan/phpstan-src.

[enumag]

People don't read warnings.

Hmm... then have an extra error thrown by phpstan if it is not run as phar. Would still allow debugging just always fail, making it useless for anyone's CI.

It wouldn't work as extensions do not require phpstan/phpstan-src.

phpstan/phpstan-src could provide phpstan/phpstan

[ondrejmirtes]

I did the replace/provide dance previously with phpstan-shim and it's something I don't want to ever touch again, it's a huge can of worms.

[enumag]

Is there a way to disable the buggy DsMapDynamicReturnTypeExtension in my phpstan.neon? Then I could try with my local implementation.

[ondrejmirtes]

I don't think so. If it weas I'd make it one of my suggestions, but the only two things you can do I described above: https://github.com/phpstan/phpstan/issues/2909#issuecomment-581421478

[enumag]

Meh. I forked phpstan-src, added it to packagist and debugged with that. Your suggested fix works just fine. I added it to https://github.com/phpstan/phpstan-src/pull/126.

[enumag]

phpstan/phpstan-src already has a replace for phpstan/phpstan in it's composer.json and it works perfectly fine. I still think my suggestion is perfectly valid and the way to go to make contributing to phpstan a bit easier.

[ondrejmirtes]

PR merged.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.