-
Notifications
You must be signed in to change notification settings - Fork 819
[phpstorm-stubs] Add logic to resolve and normalize return types from PHPDoc in StubsTypeHintsTest #1826
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
Conversation
… PHPDoc in StubsTypeHintsTest
…ing return types from PHPDoc in StubsTypeHintsTest
…type normalization logic in StubsTypeHintsTest
tests/StubsTypeHintsTest.php
Outdated
| * @param array $types Array of strings with types for processing | ||
| * @return array Normalized types | ||
| */ | ||
| private static function processTypes(array $types): array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, process is quite a generic and not descriptive name here. How do we process types? AFAIU we convert a string of types to array of types and make them unified, right? Would something like convertToArrayOfUnifiedTypes or convertToArrayOfNormalizedTypes be more descriptive here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you are right. Your variants are much more descriptive
tests/StubsTypeHintsTest.php
Outdated
| $result[] = self::replaceArrayNotations($simpleType); | ||
| } | ||
|
|
||
| return array_filter(array_unique($result)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but I'm not sure I get why we need array_filter without a callback here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was made to remove possible falsy elements like null, false, 0 etc.
But since tests pass even without it, I removed it
tests/StubsTypeHintsTest.php
Outdated
| } | ||
|
|
||
| /** | ||
| * @param array $types Array of strings with types for processing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to replace array with string[] here since we pass an array of strings, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we can do that
…tToArrayOfNormalizedTypes` and update references in StubsTypeHintsTest
|
I take responsibility🫡 |
No description provided.