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

TASK: Phpstan match on class dont /** @phpstan-ignore-next-line */ #4354

Closed
mhsdesign opened this issue Jun 22, 2023 · 3 comments
Closed

TASK: Phpstan match on class dont /** @phpstan-ignore-next-line */ #4354

mhsdesign opened this issue Jun 22, 2023 · 3 comments

Comments

@mhsdesign
Copy link
Member

mhsdesign commented Jun 22, 2023

see

/** @phpstan-ignore-next-line */
return match ($command::class) {
SetNodeProperties::class => $this->handleSetNodeProperties($command, $contentRepository),
SetSerializedNodeProperties::class
=> $this->handleSetSerializedNodeProperties($command, $contentRepository),
where we match on the class, but phpstan doesnt understand that the object is of the matched kind in the match arm, so we /** @phpstan-ignore-next-line */ the code. Which is cruel.

This issue came up again in #4349 (comment)

@mhsdesign
Copy link
Member Author

mhsdesign commented Jun 22, 2023

@grebaldi wrote

I believe that if $destination = $newLocation->destination were established before the match clause, so that we'd actually do:

$destination = $newLocation->destination;
match ($destination::class) {
    /* ... */
};

phpstan would be able to narrow the type of $destination through the match arms. I'm not sure though.

edit, that wouldnt explain the phpstan ignore annotation above, but maybe phpstand got smarter?

@mhsdesign
Copy link
Member Author

A few cases have been cleaned up here: #4455 (comment)

@mhsdesign mhsdesign added the Task label Sep 1, 2023
@mhsdesign
Copy link
Member Author

I tried to fix these matches with #4643 but it turns out that there is no easy way around this.

The command handler are a dynamic construct in regards to canHandle and handle. We dont exhaustively match all cases because we dont handle them all and want an exception to be thrown.

One could use generics to tell the command handler in its template that it can only handle those commands and thus narrow the type of the handle method, but we would effectively gain nothing, as we only call the handle once in the command bus where everything is highly dynamic - so static analysis doesnt help.

@github-project-automation github-project-automation bot moved this from Todo to Done ✅ in Neos 9.0 Release Board Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

1 participant