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

RegexExpressionHelper - Support all bracket style delimiters #3273

Merged
merged 2 commits into from
Jul 28, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jul 27, 2024

refs #3271 (comment)

I was not aware these variants exist

@staabm
Copy link
Contributor Author

staabm commented Jul 27, 2024

//cc @Seldaek

I was not able to add a test for [(\d+)(?P<num>\d+)]n

function (string $s): void {
	if (preg_match('[(\d+)(?P<num>\d+)]n', $s, $matches)) {
		assertType('array{0: string, num: numeric-string, 1: numeric-string}', $matches);
	}
};

because the hoa ast ist different for it in comparison to the other bracket style delims

<?php

require __DIR__.'/vendor/autoload.php';

// 1. Read the grammar.
$grammar  = new Hoa\File\Read(__DIR__.'/resources/RegexGrammar.pp');

// 2. Load the compiler.
$compiler = Hoa\Compiler\Llk\Llk::load($grammar);

// 3. Lex, parse and produce the AST.
$ast      = $compiler->parse('[(\d+)(?P<num>\d+)]n');

// 4. Dump the result.
$dump     = new Hoa\Compiler\Visitor\Dump();
echo $dump->visit($ast);

@staabm staabm marked this pull request as ready for review July 27, 2024 19:06
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@Seldaek
Copy link
Contributor

Seldaek commented Jul 27, 2024

Yeah that's because the parser has no concept of delimiters.. And supporting those delimiters would probably be a pain tbh. So I guess let's leave it as is for now.

@Seldaek
Copy link
Contributor

Seldaek commented Jul 27, 2024

On second thought I guess actually what you should do is probably parse delimiters first, remove delimiters/modifiers, then pass the actual pattern to the parser. That'd probably be the more accurate way to use it. Because otherwise when using () as delimiters, aren't you ending up with one match group too many?

@staabm
Copy link
Contributor Author

staabm commented Jul 27, 2024

On second thought I guess actually what you should do is probably parse delimiters first, remove delimiters/modifiers, then pass the actual pattern to the parser.

nice catch. looking at hoa-regex README the example actually also doesn't use modifiers/delimiters

@ondrejmirtes ondrejmirtes merged commit 390fe08 into phpstan:1.11.x Jul 28, 2024
446 of 455 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

BTW: Could you please look into \Q...\E? https://stackoverflow.com/questions/20518758/how-the-preg-match-handles-the-delimiter-when-q-e-used

Can't find the issue right now but someone tried to use it in ignoreErrors too, and the validator rejected it.

@staabm staabm deleted the del branch July 29, 2024 08:13
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.

4 participants