Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jul 21, 2024

No description provided.

@staabm
Copy link
Contributor Author

staabm commented Jul 21, 2024

//cc @Seldaek


function (string $s): void {
preg_match('/%a(\d*)?/', $s, $matches, PREG_UNMATCHED_AS_NULL);
assertType("array{0?: string, 1?: ''|numeric-string|null}", $matches); // could be array{0?: string, 1?: ''|numeric-string}
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! What's the matter here tho?

Copy link
Contributor Author

@staabm staabm Jul 21, 2024

Choose a reason for hiding this comment

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

in this case php-src does not return NULL for the unmatched group even if PREG_UNMATCHED_AS_NULL.

that would be another special case we need to hardcode - therefore I went with leaving it less precise and have this unnecessary null in here.

I have a feeling that the ArrayShapeMatcher will get enough special cases with preg_match_all support ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes ok, it should be ''|numeric-string, or unset if outside the true-match-arm.

https://3v4l.org/Tgk4m

PREG_UNMATCHED_AS_NULL has no impact here because (\d*) matches an empty string too, so it always matches. It's technically not optional despite the ? that follows, as long as it is not wrapped in something that contains some content.. e.g. the third example here https://3v4l.org/92geX

So I guess one rule could be that if the inside of a match group resolves to MAYBE empty string, then the match group should never be nullable as long as its parents are also not nullable.

@ondrejmirtes ondrejmirtes merged commit 2223dd9 into phpstan:1.11.x Jul 21, 2024
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the numeric branch July 21, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants