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

Implement conditional parameter types #1264

Merged
merged 6 commits into from May 3, 2022

Conversation

rvanvelzen
Copy link
Contributor

Resolves phpstan/phpstan#7124. Parameter types could in fact already be conditional when using template types, this just implements the same logic as used for return types.

To improve error messages for conditional types in general they're resolved immediately now (when possible)

}

$errors = $this->runAnalyse(__DIR__ . '/data/discussion-7124.php');
$this->assertCount(4, $errors);
Copy link
Member

Choose a reason for hiding this comment

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

Please assert which errors are happening here.

$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(int): bool, Closure(bool): bool given', $errors[0]->getMessage());
$this->assertSame(52, $errors[0]->getMessage());
$this->assertSame('Parameter #2 $callback of function Discussion7124\filter expects callable(bool): bool, Closure(int): bool given', $errors[0]->getMessage());
$this->assertSame(59, $errors[0]->getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

What? :) You're asserting the same thing to different values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I was day dreaming a bit when moving some code around 😅

@rvanvelzen
Copy link
Contributor Author

I've refactored "late resolvable types" a bit to make it more future-proof. Stuff like offset access types can reuse much of the logic.

@@ -19,6 +19,7 @@
"Reflection",
"SPL",
"standard",
"pcntl",
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you re-add this stuff intentionally or was it a bad merge/rebase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely a bad merge

@ondrejmirtes
Copy link
Member

Nice cleanup, I really like it :) Thank you!

@ondrejmirtes ondrejmirtes merged commit 8ad35b3 into phpstan:1.6.x May 3, 2022
@rvanvelzen rvanvelzen deleted the conditional-parameter branch May 17, 2022 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants