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.

Sign up for GitHub

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

Jump to bottom

Add more precise types for the prophesize method #63

Merged
merged 1 commit into from
Mar 13, 2024
Merged

Add more precise types for the prophesize method #63

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/ProphecyTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ trait ProphecyTrait
* @throws DoubleException
* @throws InterfaceNotFoundException
*
* @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>)
Copy link

@jseparovic1 jseparovic1 Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

*
* @not-deprecated
*/
protected function prophesize(?string $classOrInterface = null): ObjectProphecy
Expand Down