Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jul 21, 2024

Starting in small steps, as I think preg_match_all has some edge-cases each worth a separate PR ;)

@staabm
Copy link
Contributor Author

staabm commented Jul 21, 2024

for a green build this PR requires #3257

@Seldaek
Copy link
Contributor

Seldaek commented Jul 21, 2024

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.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

  1. All the flags described here https://www.php.net/manual/en/function.preg-match-all.php need implementation and tests.
  2. 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
@ondrejmirtes
Copy link
Member

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 :)

@staabm staabm force-pushed the match-all branch 3 times, most recently from 2fc473e to 1382805 Compare July 27, 2024 19:11
@clxmstaab clxmstaab force-pushed the match-all branch 2 times, most recently from d4fd3f8 to 83e1bbe Compare August 1, 2024 14:38
@staabm staabm marked this pull request as ready for review August 1, 2024 15:11
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm
Copy link
Contributor Author

staabm commented Aug 1, 2024

I think I am at a point where I don't see any mistakes anymore, because I am so deep into it.
lets see what you gyus think about it

@ondrejmirtes ondrejmirtes merged commit 2229deb into phpstan:1.11.x Aug 1, 2024
@ondrejmirtes
Copy link
Member

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants