Skip to content

Added FunctionTypeSpecifyingExtension for array_is_list #770

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

Conversation

canvural
Copy link
Contributor

This PR adds a new FunctionTypeSpecifyingExtension for PHP 8.1's array_is_list function.

I'm not sure if the tests cover every case that it should cover. If you request I can add more.

@canvural canvural force-pushed the array-is-list-type-specifying-extension branch from 205ad66 to 46a8b88 Compare November 15, 2021 15:49
Comment on lines +8 to +10
if (array_is_list($foo)) {
assertType('array<int, mixed>', $foo);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

tabs vs. spaces

Comment on lines +83 to +95
if ($functionName === 'array_is_list' && count($node->getArgs()) === 1) {
$arrayValue = $scope->getType($node->getArgs()[0]->value);

if (!$arrayValue instanceof ArrayType) {
return null;
}

if ((new StringType())->isSuperTypeOf($arrayValue->getKeyType())->yes()) {
return false;
}

return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering whether these changes are required?
isn't the type-specifying extension enough to get it working?

use PHPStan\Type\FunctionTypeSpecifyingExtension;
use PHPStan\Type\IntegerType;

class ArrayIsListFunctionTypeSpecifyingExtension implements FunctionTypeSpecifyingExtension, TypeSpecifierAwareExtension
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be a php-version check somewhere in this extension, since array_is_list is only available with php 8.1+

@ondrejmirtes
Copy link
Member

Hi, thanks! I've reused your tests and implemented it the way I think is right (inspired by the array_values dynamic return type extension): 59d6fab

@canvural canvural deleted the array-is-list-type-specifying-extension branch November 16, 2021 12:56
@canvural
Copy link
Contributor Author

Oh great! So the changes in the extension was enough to get the Call to function array_is_list() with... will always evaluate to false errors. I couldn't manage to do that 😅

@ondrejmirtes
Copy link
Member

Yes, these errors are reported when the type returned from the extension against the current expression type form an impossible intersection. If you return empty SpecifiedTypes when the argument isn't an array, there's nothing to report :)

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.

3 participants