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.

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

InvalidThrowsPhpDocValueRule: support @require-extends, @require-implements #2890

Merged
merged 4 commits into from Jan 27, 2024

Conversation

RobertMe
Copy link
Contributor

Handle / support @throws types which are annotated with @phpstan-require-extends or @phpstan-require-implements.

Fixes phpstan/phpstan#10475

Currently this is a WIP as support for intersections and unions is lacking and this is more like a "support request" on how I can support this.

@ondrejmirtes
Copy link
Member

Fixed the implementation, see my commits.

@ondrejmirtes ondrejmirtes marked this pull request as ready for review January 27, 2024 14:35
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes ondrejmirtes merged commit 5ee375e into phpstan:1.10.x Jan 27, 2024
256 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

@RobertMe
Copy link
Contributor Author

Thank you. I more or less had the same in mind for handling unions (check of UnionType, iterate over its types and make a recursive call returning false if one is invalid / true if their all valid). But wanted to do to same for intersections (but obviously being true if any is valid, instead of all). And then "falling back" to what I had in case it was neither of those. But the resolved types (from the annotation) can indeed be combined into an intersection with the given type, and then doing the isSuperTypeOf check on that, dropping the need for an IntersectionType specific path.

@staabm
Copy link
Contributor

staabm commented Jan 30, 2024

does this PR also needs support in DependencyResolver, so types referenced via require-extends/require-implements from a throw-type will invalidate the result cache properly?

(similar how it was done with #2866)

@ondrejmirtes
Copy link
Member

I don't know. Maybe when the class in require-extends starts/stops being a Throwable we get a stale cache.

You can always verify your hypothesis with a failing test 😊

@RobertMe RobertMe deleted the throws-require-extends branch April 8, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants