Skip to content

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

Merged
ondrejmirtes merged 4 commits intophpstan:1.10.xfrom
RobertMe:throws-require-extends
Jan 27, 2024
Merged

InvalidThrowsPhpDocValueRule: support @require-extends, @require-implements#2890
ondrejmirtes merged 4 commits intophpstan:1.10.xfrom
RobertMe:throws-require-extends

Conversation

@RobertMe
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

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

Thank you.

@RobertMe
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Development

Successfully merging this pull request may close these issues.

4 participants