Recognize constants as constant types

[rvanvelzen]

Closes phpstan/phpstan#6160. This was just a wild idea. Probably not the best way forward, but at least it shows the potential

[rvanvelzen]

Obviously this breaks on PHP < 8 💥

[staabm]

you should run this test only on php8, or/and add an php7 alternative test-case with a different expectation.

[staabm]

since A and B or similar short-names might be considered a constant in this case, I think tryResolveConstant should at first try whether there is a class defined with this name. Only if not, try whether its actually a constant.

what do you think?

[ondrejmirtes]

Agreed, constant can be tried only when it's UPPER_CASE and class of that name does not exist.

[staabm]

I love it. this kills 2 birds with one stone.

it implements phpstan/phpstan#6160 and fixes one of the psalm-special-cases within the conditional-type language phpstan/phpstan#3853 (comment)

[ondrejmirtes]

I hate to bring the bad news but:

1. I like to resolve constants here to solve phpstan/phpstan#6160
2. But I don't think it makes sense to support this in conditional types. The code here reports int<5, max>: https://phpstan.org/r/853028b5-3d52-4f51-ae64-1655458ef879 So typehinting PHP_MAJOR_VERSION in a PHPDoc also has to resolve to int<5, max> thanks to dynamicConstantNames (https://phpstan.org/config-reference#constants).

Which makes it useless for conditional types. But it's fine for me - having PHP_MAJOR_VERSION conditional in a stub is useful only for built-in PHP functions, and these can be solved by writing code in an extension. I don't want to have a small cosmetic advantage at a cost of doing some horrible hacks to make this work in stubs :)

[ondrejmirtes]

Also wildcard support would be a great addition, some people expect that to work (phpstan/phpstan#6972). Needs modification in phpdoc-parser too.

[rvanvelzen]

Also wildcard support would be a great addition, some people expect that to work (phpstan/phpstan#6972). Needs modification in phpdoc-parser too.

If you don't mind I'd like to tackle that separately. This PR has become quite large enough as-is 🥲

[ondrejmirtes]

I agree, we can leave the wildcards for a later time :)

[ondrejmirtes]

Perfect! Can you please check that NameScope doesn't contain use function uses as they are useless for it?

[ondrejmirtes]

Checking the code they probably aren't there thanks to TYPE_NORMAL.

[ondrejmirtes]

I think this should just be if ($this->getReflectionProvider()->hasClass($stringName)) {.

Because if something looks like a constant FOO_BAR but there's a class FOO_BAR, I want ObjectType of the class, not the constant.

[rvanvelzen]

This should still always result in the class taking precedence, but does a slightly cheaper heuristic check.

It's covered in the test as well: https://github.com/phpstan/phpstan-src/pull/1163/files#diff-f60365c259ff216f4d228e0c46729817e679144be2a7bf2e7395ace7fd344284R47

[ondrejmirtes]

I don't think it hits that condition:

private function mightBeConstant(string $name): bool	{ return preg_match('((?:^|\\\\)[A-Z_][A-Z0-9_]*$)', $name) > 0;	}

This just matches UPPER_CASE names, right? But the constant in question has CamelCaps namespace.

Because when I read that code in case of FOO_BAR the first return new ObjectType($stringName); is skipped because mightBeConstant() === true. And then tryResolveConstant is successful because the constant exists. So constant takes precedence over class.

[rvanvelzen]

The namespace isn't considered, only the last part of $name (so BAR in that case). If the constant took precedence the type in the test would be 10, instead of the class name (ConstantPhpdocType\BAR).

[ondrejmirtes]

Now it makes sense to me :) I'm ready to merge this, thanks.

[rvanvelzen]

Can you please check that NameScope doesn't contain use function uses as they are useless for it?

It shouldn't because only TYPE_NORMAL and TYPE_CONSTANT are considered, and use function is TYPE_FUNCTION.

[ondrejmirtes]

Thank you! Please send an update to the documentation: https://phpstan.org/writing-php-code/phpdoc-types (add a new section "Global constants").

[vudaltsov]

Did you consider introducing a new constant<MY_CONSTANT> type constructor? That would simplify things a lot and allow to use not only global constants, but any other.

[ondrejmirtes]

@vudaltsov You're commenting on a two year old PR. You should open a new feature request and describe your use case.