Skip to content

Conversation

@egst
Copy link

@egst egst commented Jan 22, 2023

The current return annotation of the ServiceContainer::get method @return T|ServiceContainer is unnecessarily generic - even when a static analyzer (PHPStan, Psalm) is able to recognize the type T, the ServiceContainer type is forced into the result.

Ideally @return T would be enough (it already includes the T = ServiceContainer case) but it looks like there's an issue in PHPStan that it doesn't see ServiceContainer as a subtype of T in this case. I've reported the issue at phpstan/phpstan#8756.

Until the PHPStan issue is solved, we can use a workaround that utilizes the conditional type declaration (as demonstrated here: https://phpstan.org/r/f2c3eb91-c6b0-45ca-9dd5-d434274f82af).

There's no such issue with Psalm (as shown here: https://psalm.dev/r/85f8d8262c) so I think we should only introduce the workaround with a PHPStan-specific annotation (@phpstan-return).

Also note that Intelephense doesn't recognize templates, so @return T won't work well with it too. However, neither did @return T|ServiceContainer. So if we wanted to make it work well even with Intelephense, we should probaly use @return mixed, @psalm-return T and @phpstan-return (T is static ? static : T).

@egst
Copy link
Author

egst commented Jan 22, 2023

PHPStan update: ondrejmirtes closed my issue report with words "I think it's fine" so I don't think we can expect a fix any time soon (unless the underlying issue causes other reported troubles with PHPStan).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant