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: fix nested capturing counting #3184

Merged
merged 10 commits into from
Jun 25, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jun 24, 2024

@staabm
Copy link
Contributor Author

staabm commented Jun 24, 2024

Will have another look later

@clxmstaab clxmstaab force-pushed the nested-capturing branch 2 times, most recently from 887cc67 to 778bdb3 Compare June 24, 2024 12:42
// add one capturing group to the end so all capture group keys
// are present in the $matches
// see https://3v4l.org/sOXbn, https://3v4l.org/3SdDM
$captureGroupsRegex = Strings::replace($regex, '~.[a-z\s]*$~i', '|(?<phpstanNamedCaptureGroupLast>)$0');
Copy link
Contributor Author

@staabm staabm Jun 24, 2024

Choose a reason for hiding this comment

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

reworked the matcher to be AST only -> no more php version differences

@@ -172,10 +172,6 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-9499.php');
}

if (PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 70400) {
yield from $this->gatherAssertTypes(__DIR__ . '/data/preg_match_shapes_php73.php');
Copy link
Contributor Author

@staabm staabm Jun 24, 2024

Choose a reason for hiding this comment

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

this test is contained in preg_match_shapes_php.php with proper expectations across all php versions. we no longer need separate expectations for 7.2/7.3

@staabm
Copy link
Contributor Author

staabm commented Jun 24, 2024

I did not yet figure out, how/why the result of the last failling test-case can be different on 7.2/7.3

@staabm
Copy link
Contributor Author

staabm commented Jun 24, 2024

I did not yet figure out, how/why the result of the last failling test-case can be different on 7.2/7.3

I now know what the cause of the problem is:

return in_array(strtolower($functionReflection->getName()), ['preg_match'], true) && $parameter->getName() === 'matches';

in PHP 7.2/7.3 the parameter is named "subpatterns" and in 7.4+ its "matches"

@@ -485,3 +485,20 @@ function ($s, $t): void {
assertType('string', $s);
assertType('string', $t);
};

function fooMatch(string $input): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

expectations have been harmonized across php versions, therefore the 2 variants above removed and the tests moved with same expectations in this common file

@staabm staabm force-pushed the nested-capturing branch from e8cabe0 to ec04a4e Compare June 25, 2024 11:28
@staabm
Copy link
Contributor Author

staabm commented Jun 25, 2024

@ondrejmirtes would be great to get this one merged, as it is now php version independent and in turn would also allow to fix the phpstan-nette build.

it fixes all currently known problems (preg_match bugs-ticket)

@ondrejmirtes
Copy link
Member

Vibing cat

Purrrrfect!

@ondrejmirtes ondrejmirtes merged commit 73c7a86 into phpstan:1.11.x Jun 25, 2024
451 of 454 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the nested-capturing branch June 25, 2024 11:48
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.

preg_match bugs
2 participants