Add more precise types for the prophesize method#63
Conversation
This matches the type used in Prophecy as the method is forwarded to it.
| * @psalm-param class-string|null $classOrInterface | ||
| * @template T of object | ||
| * @phpstan-param class-string<T>|null $classOrInterface | ||
| * @phpstan-return ($classOrInterface is null ? ObjectProphecy<object> : ObjectProphecy<T>) |
There was a problem hiding this comment.
| * @phpstan-return ($classOrInterface is null ? ObjectProphecy<object> : ObjectProphecy<T>) | |
| * @return ($classOrInterface is null ? ObjectProphecy<object> : ObjectProphecy<T>) |
Does it require phpstan specific annotation? This way I assume it could work with both psalm and phpstan.
https://phpstan.org/blog/phpstan-1-6-0-with-conditional-return-types#conditional-return-types
Update
Never mind i see that underlaying method is also using phpstan-return.
There was a problem hiding this comment.
Both phpstan and psalm will read the annotation with the prefix of the other tool if they don't have their own prefixed annotation (but with a different error handling as they will skip an annotation they cannot support instead of reporting an error).
The reason they do that is to avoid forcing all libraries to provide duplicate annotations to let downstream projects benefit from their type when being analyzed.
The general rule is that libraries should apply the prefix corresponding to the tool they use themselves (so that their own analysis report invalid annotations). And as I'm more a phpstan user than a psalm one myself, I would be using phpstan when adding a static analysis setup in this package (which I will probably do at some point).
I'm not using @return here because conditional types are supported by psalm and phpstan but not by other tools dealing with phpdoc, which might then be confused about them.
There was a problem hiding this comment.
Thank you for the explanation! Obviously I wasn't aware on how they approach this but it makes sense as writing two types of anotations would be a maintenance burden.
This is to include a couple of PRs pending release: * Jan0707/phpstan-prophecy#328 * phpspec/prophecy-phpunit#63
|
Can we have a release including this change/fix please? |
|
@stof do you think we could create a release with this one? |
This matches the type used in Prophecy as the method is forwarded to it.