Skip to content

Add support for wildcards in the middle and beginning of const type annotations #658

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 12, 2021
Merged

Add support for wildcards in the middle and beginning of const type annotations #658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"ondram/ci-detector": "^3.4.0",
"ondrejmirtes/better-reflection": "4.3.67",
"phpstan/php-8-stubs": "^0.1.23",
"phpstan/phpdoc-parser": "^0.5.6",
"phpstan/phpdoc-parser": "^0.5.7",
"react/child-process": "^0.6.1",
"react/event-loop": "^1.1",
"react/http": "^1.1",
Expand Down
14 changes: 7 additions & 7 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/PhpDoc/TypeNodeResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,11 +702,12 @@ private function resolveConstTypeNode(ConstTypeNode $typeNode, NameScope $nameSc
$classReflection = $this->getReflectionProvider()->getClass($className);

$constantName = $constExpr->name;
if (Strings::endsWith($constantName, '*')) {
$constantNameStartsWith = Strings::substring($constantName, 0, Strings::length($constantName) - 1);
if (Strings::contains($constantName, '*')) {
// convert * into .*? and escape everything else so the constants can be matched against the pattern
$pattern = '{^' . str_replace('\\*', '.*?', preg_quote($constantName)) . '$}D';
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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

Copy link
Contributor Author

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 like FOO* => 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 also FOOBAR\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.

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thank you :)

$constantTypes = [];
foreach ($classReflection->getNativeReflection()->getConstants() as $classConstantName => $constantValue) {
if (!Strings::startsWith($classConstantName, $constantNameStartsWith)) {
if (Strings::match($classConstantName, $pattern) === null) {
continue;
}

Expand Down
16 changes: 14 additions & 2 deletions tests/PHPStan/Analyser/data/const-expr-phpdoc-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ class Foo

public const SOME_CONSTANT = 1;
public const SOME_OTHER_CONSTANT = 2;
public const YET_ONE_CONST = 3;
public const YET_ANOTHER_CONST = 4;
public const DUMMY_SOME_CONSTANT = 99; // should only match the *

/**
* @param 'foo'|'bar' $one
Expand All @@ -21,6 +24,9 @@ class Foo
* @param 234 $seven
* @param self::SOME_OTHER_* $eight
* @param self::* $nine
* @param self::*_CONST $ten
* @param self::YET_*_CONST $eleven
* @param self::*OTHER* $twelve
*/
public function doFoo(
$one,
Expand All @@ -31,7 +37,10 @@ public function doFoo(
$six,
$seven,
$eight,
$nine
$nine,
$ten,
$eleven,
$twelve
)
{
assertType("'bar'|'foo'", $one);
Expand All @@ -42,7 +51,10 @@ public function doFoo(
assertType('1.0', $six);
assertType('234', $seven);
assertType('2', $eight);
assertType('1|2', $nine);
assertType('1|2|3|4|99', $nine);
assertType('3|4', $ten);
assertType('3|4', $eleven);
assertType('2|4', $twelve);
}

}