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

Add support for isMatch in phpstan type ext #26

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

Seldaek
Copy link
Member

@Seldaek Seldaek commented Jul 11, 2024

@staabm sorry to bug you again but any clue why this does not work?

Ideally we'd have support for:

  • isMatch (like match but returns bool instead of 0|1)
  • matchStrictGroups (like match but makes sure all match groups are non-null if it matched)
  • isMatchStrictGroups (same as above but returns bool instead of 0|1)
  • all the above in matchAll variants

But at least isMatch would be great because that's used all over the place (I never use match as it returns int), and it is API compatible with match so I don't get why this fails.

@Seldaek
Copy link
Member Author

Seldaek commented Jul 11, 2024

The other question I have is that it does array{0: string, 1?: string|null} but as the lib forces PREG_UNMATCHED_AS_NULL I think it should be array{0: string, 1: string|null} i.e. 1 offset is always there, but may be null of course. Is there any way to tweak this?

@staabm
Copy link
Contributor

staabm commented Jul 11, 2024

any clue why this does not work?

I will have a look, but cannot promise today/tomorrow

The other question I have is that it does array{0: string, 1?: string|null} but as the lib forces PREG_UNMATCHED_AS_NULL I think it should be array{0: string, 1: string|null} i.e. 1 offset is always there, but may be null of course. Is there any way to tweak this?

I think thats already fixed - but not yet released - with phpstan/phpstan-src#3219

@@ -27,6 +27,13 @@ function doMatch(string $s): void
assertType('array{}', $matches);
}
assertType('array{}|array{0: string, 1?: string|null}', $matches);

if (Preg::isMatch('/Price: (?P<currency>£|€)\d+/', $s, $matches)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

something is wrong with Hoa parsing the AST for the pattern.

it works as expected with a pattern like

Suggested change
if (Preg::isMatch('/Price: (?P<currency>£|€)\d+/', $s, $matches)) {
if (Preg::isMatch('/Price: (£|€)\d+/', $s, $matches)) {

Copy link
Contributor

Choose a reason for hiding this comment

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

opened a upstream bug report regarding the bogus-pattern:

phpstan/phpstan#11323

@staabm
Copy link
Contributor

staabm commented Jul 11, 2024

  • isMatch (like match but returns bool instead of 0|1)

  • matchStrictGroups (like match but makes sure all match groups are non-null if it matched)

  • isMatchStrictGroups (same as above but returns bool instead of 0|1)

I think these should be doable in a similar way you did in this PR

  • all the above in matchAll variants

match all is on the todo-list and not yet support in phpstan-src

@Seldaek Seldaek merged commit 9f7f3d2 into composer:2.x Jul 11, 2024
11 of 12 checks passed
@Seldaek Seldaek deleted the isMatch-support branch July 11, 2024 13:31
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.

2 participants