-
Notifications
You must be signed in to change notification settings - Fork 428
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 wildcards in the middle and beginning of const type annotations #658
Conversation
16f8dde
to
0856f4c
Compare
if (Strings::endsWith($constantName, '*')) { | ||
$constantNameStartsWith = Strings::substring($constantName, 0, Strings::length($constantName) - 1); | ||
if (Strings::contains($constantName, '*')) { | ||
$pattern = '{^' . str_replace('\\*', '.*?', preg_quote($constantName)) . '$}D'; |
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.
Calling https://www.php.net/manual/en/function.fnmatch.php might be an easier way to do this?
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.
Not so sure.. Because that also matches []
blocks etc, might do more than we want it to, although I guess the lexer won't ever give you those characters in the constant name. This preg way is easy and works, but if you really want I can do it with fnmatch.
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.
Yeah, this is fine if it works, but I have to admit I'm not a regex expert so I have no idea what's going on in the current code :)
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.
Added a comment if it helps ;) And more tests to check multiple *
s.
Just to explain in more details what this does:
- preg_quote escapes all regex metacharacters, so matching $candidate against
'{^' . preg_quote($constantName) . '$}D'
would simply be equivalent to doing$constantName === $candidate
- That's not so helpful of course so adding the str_replace turns back
\*
(i.e. escaped wildcards into.*?
so you end up with a pattern likeFOO*
=>FOO.*?
which allows for anything where the wildcard was - The complete pattern is{^FOO.*?$}D
though to make sure it does not match a part of the string, the anchors ^/$ make sure it's matching the whole candidate name. - The
D
modifier ensures that$
cannot match if there is a newline, without it^FOO.*?$
would match alsoFOOBAR\n
which we don't want even though here I guess it is not possible to get such a string to be matched against anyway.
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.
Awesome, thank you :)
0856f4c
to
0525467
Compare
0525467
to
0197c3a
Compare
if (Strings::endsWith($constantName, '*')) { | ||
$constantNameStartsWith = Strings::substring($constantName, 0, Strings::length($constantName) - 1); | ||
if (Strings::contains($constantName, '*')) { | ||
$pattern = '{^' . str_replace('\\*', '.*?', preg_quote($constantName)) . '$}D'; |
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.
Awesome, thank you :)
Force-pushed the phpdoc-parser update here, I'm gonna wait for the build and merge it. Thank you, I appreciate it, it was a pleasure! You get how open-source works (which isn't surprising from you :)) |
Fixes phpstan/phpstan#5534
Depends on phpstan/phpdoc-parser#81 (hence the build failures)