Add more updates from Psalm and PHPStan #3

muglug · 2019-01-09T03:03:38Z

TysonAndre · 2019-01-09T04:58:31Z

FunctionSignatureMap.php

@@ -300,6 +300,8 @@
 'array_intersect_uassoc\'1' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'arr3'=>'array', 'arg4'=>'array|callable(mixed,mixed):int', '...rest'=>'array|callable(mixed,mixed):int'],
 'array_intersect_ukey' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'key_compare_func'=>'callable(mixed,mixed):int'],
 'array_intersect_ukey\'1' => ['array', 'arr1'=>'array', 'arr2'=>'array', 'arr3'=>'array', 'arg4'=>'array|callable(mixed,mixed):int', '...rest'=>'array|callable(mixed,mixed):int'],
+'array_key_first' => ['int|string|null', 'array'=>'array'],


Speaking of which, I was planning to update this with the new signatures from https://github.com/phan/phan/blob/master/src/Phan/Language/Internal/FunctionSignatureMap_php73_delta.php when 7.3.0 was released, but haven't gotten around to it yet.

TysonAndre · 2019-01-09T05:01:19Z

FunctionSignatureMap.php

 'exif_read_data' => ['array|false', 'filename'=>'string', 'sections_needed='=>'string', 'sub_arrays='=>'bool', 'read_thumbnail='=>'bool'],
 'exif_tagname' => ['string', 'index'=>'int'],
 'exif_thumbnail' => ['string', 'filename'=>'string', '&w_width='=>'int', '&w_height='=>'int', '&w_imagetype='=>'int'],
 'exit' => ['', 'status'=>'string|int'],
 'exp' => ['float', 'number'=>'float'],
 'expect_expectl' => ['int', 'expect'=>'resource', 'cases'=>'array', 'match='=>'array'],
 'expect_popen' => ['resource', 'command'=>'string'],
-'explode' => ['array<int,string>', 'separator'=>'string', 'str'=>'string', 'limit='=>'int'],
+'explode' => ['array<int,string>|false', 'separator'=>'string', 'str'=>'string', 'limit='=>'int'],


In Phan's version of the signature map, this only contains return types that would happen if the input types were valid.
Explode would always return an array given two strings (I think)

So changes such as imap and exif make sense (they depend on the filesystem/network and file contents and can fail), but this doesn't for Phan

This is so that --strict-types would correctly analyze uses of the return types. (It'd warn about param types with the right settings)

Explode would always return an array given two strings (I think)

Nah, not quite:

If delimiter is an empty string (""), explode() will return FALSE.

Should really create a plugin to check for that LiteralStringType as an argument to explode (and any other surprising edge cases in other internal functions)

Yeah, I’ve done that in Psalm

TysonAndre · 2019-01-09T05:04:22Z

FunctionSignatureMap.php

@@ -6074,7 +6078,7 @@
 'kadm5_get_principals' => ['array', 'handle'=>'resource'],
 'kadm5_init_with_password' => ['resource', 'admin_server'=>'string', 'realm'=>'string', 'principal'=>'string', 'password'=>'string'],
 'kadm5_modify_principal' => ['bool', 'handle'=>'resource', 'principal'=>'string', 'options'=>'array'],
-'key' => ['int|string|null', 'array_arg'=>'array'],
+'key' => ['int|string|null', 'array_arg'=>'array|object'],


I did not know about this, but key works. Thankfully, array_key_first only takes arrays.

TysonAndre · 2019-01-09T05:08:49Z

FunctionSignatureMap.php

-'OCI_Collection::max' => ['int'],
-'OCI_Collection::size' => ['int'],
-'OCI_Collection::trim' => ['bool', 'num'=>'int'],
+'OCI-Collection::append' => ['bool', 'value'=>'mixed'],


I might just remove OCI-Collection's signatures from Phan entirely, because it can't analyze them. (or change everything added elsewhere from OCI-Lob to OCI_Lob

It assumes internal modules declares classes with valid FQSENs, which is true for everything except the oci module.

phan/phan#2222 (comment)

Yeah I don't think Psalm can analyse them either (but nobody's ever asked, and I'm hoping nobody ever does)

And it seems like it'd cause a crash in Psalm as well

It might be analyzable with the correct stub files for the underscore version (but users might be mislead about what the class name is).

if (stripos($return_type, 'oci-') !== false) { return; }

Now it doesn’t cause a crash in Psalm

Added a comment suggesting how to migrate PHP away from the name oci-collection in https://bugs.php.net/bug.php?id=58319

TysonAndre · 2019-01-09T05:09:45Z

FunctionSignatureMap.php

-'OCI_Collection::size' => ['int'],
-'OCI_Collection::trim' => ['bool', 'num'=>'int'],
+'OCI-Collection::append' => ['bool', 'value'=>'mixed'],
+'OCI-Collection::assign' => ['bool', 'from'=>'OCI_Collection'],


nit: If this is in other analyzers, the 'from' should be OCI-Collection.

TysonAndre · 2019-01-09T05:11:34Z

FunctionSignatureMap.php

-'preg_replace_callback_array' => ['string|array', 'pattern'=>'array<string,callable(array):string>', 'subject'=>'string|array', 'limit='=>'int', '&w_count='=>'int'],
-'preg_split' => ['array<int, string>', 'pattern'=>'string', 'subject'=>'string', 'limit='=>'?int', 'flags='=>'int'],
-'prev' => ['mixed', '&rw_array_arg'=>'array'],
+'preg_replace_callback' => ['string|array|null', 'regex'=>'string|array', 'callback'=>'callable(array):string', 'subject'=>'string|array', 'limit='=>'int', '&w_count='=>'int'],


Same comment about mb_ereg and preg_replace_callback only including return types for valid regexes and argument types. (to reduce false positives with --strict-types)

Phan's at least able to detect invalid regexes through https://github.com/phan/phan/blob/master/.phan/plugins/PregRegexCheckerPlugin.php (but not mb_ereg)

preg_split etc. can return null if there are invalid UTF-8 characters in the mix. Dunno what --strict-types does, but I've added a specific config in Psalm to allow people to care about internal function null returns (by default they're ignored).

TysonAndre · 2019-01-09T05:11:53Z

FunctionSignatureMap.php

@@ -10205,12 +10210,12 @@
 'Redis::getTimeout' => ['float|false'],
 'Redis::hDel' => ['int|false', 'key'=>'string', 'hashKey1'=>'string', 'hashKey2='=>'string', 'hashKeyN='=>'string'],
 'Redis::hExists' => ['bool', 'key'=>'string', 'hashKey'=>'string'],
-'Redis::hGet' => ['string', 'key'=>'string', 'hashKey'=>'string'],
+'Redis::hGet' => ['string|false', 'key'=>'string', 'hashKey'=>'string'],


TysonAndre · 2019-01-09T05:12:27Z

FunctionSignatureMap.php

 'Redis::select' => ['bool', 'dbindex'=>'int'],
 'Redis::sendEcho' => ['string', 'msg'=>'string'],
 'Redis::set' => ['bool', 'key'=>'string', 'value'=>'string', 'options='=>'array'],
 'Redis::set\'1' => ['bool', 'key'=>'string', 'value'=>'string', 'timeout='=>'int'],
 'Redis::setBit' => ['int', 'key'=>'string', 'offset'=>'int', 'value'=>'int'],
 'Redis::setEx' => ['bool', 'key'=>'string', 'ttl'=>'int', 'value'=>'string'],
+'Redis::setex' => ['bool', 'key'=>'string', 'ttl'=>'int', 'value'=>'string'],


Remove this, this is the same case-insensitive method name as setEx. Same for setnx

TysonAndre · 2019-01-09T05:13:29Z

FunctionSignatureMap.php

@@ -13717,7 +13724,7 @@
 'Threaded::unlock' => ['bool'],
 'Threaded::wait' => ['bool', 'timeout='=>'int'],
 'Throwable::__toString' => ['string'],
-'Throwable::getCode' => ['int'],
+'Throwable::getCode' => ['int|string'],


Returns the exception code as integer in Exception but possibly as other type in Exception descendants (for example as string in PDOException).

Ok, then this makes sense

TysonAndre · 2019-01-09T05:15:47Z

FunctionSignatureMap.php

@@ -12784,7 +12791,7 @@
 'sqlsrv_server_info' => ['array', 'conn'=>'resource'],
 'sqrt' => ['float', 'number'=>'float'],
 'srand' => ['void', 'seed='=>'int', 'mode='=>'int'],
-'sscanf' => ['mixed', 'str'=>'string', 'format'=>'string', '&...w_vars='=>'string|int|float'],
+'sscanf' => ['mixed', 'str'=>'string', 'format'=>'string', '&...w_vars='=>'string|int|float|null'],


Again, that seems like it would increase false positives. What's the motiviation for this?

Also, &...w_vars= means that this is write-only, and Phan completely ignores the input types.

Are there scanf format strings that would set arguments to null (If all arguments were read successfully and argument counts were correct)?

What's the motiviation for this?

Just because it can be null.

Conflicts: FunctionSignatureMap.php This was amended to follow what was typical for Phan's choices of signatures. Source: #3

TysonAndre · 2019-01-16T03:00:37Z

Thanks, this was amended and merged

From phan/PHPSignatures#3

Add more updates

d41bbf9

muglug mentioned this pull request Jan 9, 2019

update the "CallMap" vimeo/psalm#1186

Closed

TysonAndre reviewed Jan 9, 2019

View reviewed changes

Remove dupe entry

8f49cb0

TysonAndre merged commit 8f49cb0 into phan:master Jan 16, 2019

TysonAndre added a commit that referenced this pull request Jan 16, 2019

Merge remote-tracking branch 'psalm/more-fixes'

cf65357

Conflicts: FunctionSignatureMap.php This was amended to follow what was typical for Phan's choices of signatures. Source: #3

TysonAndre added a commit to TysonAndre/phan that referenced this pull request Jan 16, 2019

Add updates to the signature map from Psalm and PHPStan

1ceace6

From phan/PHPSignatures#3

TysonAndre mentioned this pull request Jan 16, 2019

Add updates to the signature map from Psalm and PHPStan phan/phan#2334

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more updates from Psalm and PHPStan #3

Add more updates from Psalm and PHPStan #3

muglug commented Jan 9, 2019

TysonAndre Jan 9, 2019

TysonAndre Jan 13, 2019

TysonAndre Jan 9, 2019

muglug Jan 9, 2019

TysonAndre Jan 13, 2019

muglug Jan 13, 2019

TysonAndre Jan 9, 2019

TysonAndre Jan 9, 2019

muglug Jan 9, 2019

TysonAndre Jan 9, 2019

muglug Jan 13, 2019 •

edited

Loading

TysonAndre Feb 17, 2019

TysonAndre Jan 9, 2019

TysonAndre Jan 9, 2019

muglug Jan 9, 2019 •

edited

Loading

TysonAndre Jan 9, 2019

TysonAndre Jan 9, 2019

muglug Jan 9, 2019

TysonAndre Jan 9, 2019

TysonAndre Jan 9, 2019

muglug Jan 9, 2019 •

edited

Loading

TysonAndre commented Jan 16, 2019

Add more updates from Psalm and PHPStan #3

Add more updates from Psalm and PHPStan #3

Conversation

muglug commented Jan 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muglug Jan 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muglug Jan 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muglug Jan 9, 2019 • edited Loading

Choose a reason for hiding this comment

TysonAndre commented Jan 16, 2019

muglug Jan 13, 2019 •

edited

Loading

muglug Jan 9, 2019 •

edited

Loading

muglug Jan 9, 2019 •

edited

Loading