-
Notifications
You must be signed in to change notification settings - Fork 493
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 PregMatchAllParameterOutTypeExtension #3256
Conversation
for a green build this PR requires #3257 |
Nice! Doesn't look like too much code so far, which is also what I expected. The return type is different because it's an array of arrays of matches, but otherwise the big chunk of the pattern extraction etc is the same really. |
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.
- All the flags described here https://www.php.net/manual/en/function.preg-match-all.php need implementation and tests.
- We also need TypeSpecifyingExtension for code like:
if (preg_match_all($x, $y, $z))
if (preg_match_all($x, $y, $z) > 0)
if (preg_match_all($x, $y, $z) != false)
(loose!=
on purpose)if (preg_match_all($x, $y, $z) == true)
(two==
on purpose)if (preg_match_all($x, $y, $z) === 1)
(any number higher than 0)- Falsy contexts of all of the above
I'm also looking forward to https://www.php.net/manual/en/function.preg-replace-callback.php because I believe that's going to take advantage of the new-ish FunctionParameterClosureTypeExtension :) |
2fc473e
to
1382805
Compare
d4fd3f8
to
83e1bbe
Compare
This pull request has been marked as ready for review. |
I think I am at a point where I don't see any mistakes anymore, because I am so deep into it. |
Thank you. |
Starting in small steps, as I think
preg_match_all
has some edge-cases each worth a separate PR ;)