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

RegexArrayShapeMatcher - (\d*) can be empty-string #3257

Merged
merged 3 commits into from
Jul 21, 2024

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
445 of 454 checks passed
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants