Check array key type in PHPdoc

[leongersen]

A take at phpstan/phpstan#3753. I couldn't find an existing way to retain the original type information in a NeverType, but it helps the clarity of the error message. An alternative would be to check the type in InvalidPhpDocVarTagTypeRule, instead of in TypeNodeResolver.

If this is the correct aproach, I'll add the same checks to IncompatiblePhpDocTypeRule.

[ondrejmirtes]

Hi, please read my instructions at phpstan/phpstan#3753 (comment) more carefully and follow them.

1. we need to TypeCombinator::intersect() the key here with new UnionType([new IntegerType(), new StringType()])
2. implement recursive checking for ErrorType/NeverType using TypeTraverser in this rule and others

This PR cannot be accepted because the code is different and doesn't behave as we want it to.

[leongersen]

@ondrejmirtes Could you give a hint on how the original type would be infered from the NeverType (so that it can be used in the error message)?

[ondrejmirtes]

That's out of scope of this PR and too hard. I simply want to achieve this error message: https://phpstan.org/r/ab5ad711-5186-4fae-a52a-3cb4106892c1

Improving this error message would be something for another time and a major version.

[leongersen]

Ok, that's clear. I've applied your suggestions and added support for @param and @return.

[ondrejmirtes]

I imagined something like hasUnresolvableType(Type $type): bool instead.

[ondrejmirtes]

The same logic should be applied in IncompatiblePropertyPhpDocTypeRule as well.

[leongersen]

@ondrejmirtes What would you like me to do with the failing tests in NodeScopeResolverTest?

For example, for the type array<TKey, TValue> $iterator, the added intersection result in the following change:

-'array<TKey (method Bug3266\Foo::iteratorToArray(), argument), TValue (method Bug3266\Foo::iteratorToArray(), argument)>' +'array<(int&TKey (method Bug3266\Foo::iteratorToArray(), argument))|(string&TKey (method Bug3266\Foo::iteratorToArray(), argument)), TValue (method Bug3266\Foo::iteratorToArray(), argument)>' 

Which is correct, but I don't know if changing this output is desirable?

[ondrejmirtes]

@leongersen I think I want array<mixed, X> to keep the mixed untouched, which will also fix the template type issue.

[leongersen]

Sorry, I don't really understand what you mean. Are you saying to check if the array key type is a supertype of the mixed type?

Also, would you mind elaborating on why checking whether int|string->accepts() the array key type is not a valid solution here? I'm guessing I'm misunderstanding what accepts is for.

[ondrejmirtes]

Superseded by 1de5de8