Skip to content

Modify functionMap and add __benevolent<>#2986

Merged
ondrejmirtes merged 3 commits into
phpstan:1.10.xfrom
zonuexe:fix/functionMap-use-benevolent
Mar 22, 2024
Merged

Modify functionMap and add __benevolent<>#2986
ondrejmirtes merged 3 commits into
phpstan:1.10.xfrom
zonuexe:fix/functionMap-use-benevolent

Conversation

@zonuexe

@zonuexe zonuexe commented Mar 22, 2024

Copy link
Copy Markdown
Contributor

Add __benevolent<> to functions whose documentation says "return false", which return false when passed a parameter of the wrong type, or which do not return false upon normal use.

Modify the functionMap for other functions, including those already covered by DynamicFunctionReturnTypeExtension.

@phpstan-bot

Copy link
Copy Markdown
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@zonuexe
zonuexe changed the base branch from 1.11.x to 1.10.x March 22, 2024 19:18
Comment thread resources/functionMap.php
'floatval' => ['float', 'var'=>'scalar|array|resource|null'],
'flock' => ['bool', 'fp'=>'resource', 'operation'=>'int', '&w_wouldblock='=>'int'],
'floor' => ['float|false', 'number'=>'float'],
'floor' => ['__benevolent<float|false>', 'number'=>'float'],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as RoundFunctionReturnTypeExtension.

$defaultReturnType = new FloatType();
} else {
// PHP 7 returns null with a missing parameter.
$noArgsReturnType = new NullType();
// PHP 7 can return either a float or false.
$defaultReturnType = new BenevolentUnionType([
new FloatType(),
new ConstantBooleanType(false),
]);

Comment thread resources/functionMap.php
'get_headers' => ['array|false', 'url'=>'string', 'format='=>'int', 'context='=>'resource'],
'get_html_translation_table' => ['array', 'table='=>'int', 'flags='=>'int', 'encoding='=>'string'],
'get_include_path' => ['string|false'],
'get_include_path' => ['__benevolent<string|false>'],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since the situation where the ini include_path entry does not exist is an abnormal situation, it seems that __benevolent<> can be added.

https://github.com/php/php-src/blob/df017cd0ef9e89d97b64dcf01efc5395fab30f6c/ext/standard/basic_functions.c#L2084-L2099

Comment thread resources/functionMap.php
'memory_get_peak_usage' => ['int', 'real_usage='=>'bool'],
'memory_get_usage' => ['int', 'real_usage='=>'bool'],
'memory_get_peak_usage' => ['positive-int', 'real_usage='=>'bool'],
'memory_get_usage' => ['positive-int', 'real_usage='=>'bool'],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread resources/functionMap.php
'ming_useswfversion' => ['void', 'version'=>'int'],
'mkdir' => ['bool', 'pathname'=>'string', 'mode='=>'int', 'recursive='=>'bool', 'context='=>'resource'],
'mktime' => ['int|false', 'hour='=>'int', 'min='=>'int', 'sec='=>'int', 'mon='=>'int', 'day='=>'int', 'year='=>'int'],
'mktime' => ['__benevolent<int|false>', 'hour='=>'int', 'min='=>'int', 'sec='=>'int', 'mon='=>'int', 'day='=>'int', 'year='=>'int'],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This function returns false only when it receives a value outside the integer range.

https://github.com/php/php-src/blob/df017cd0ef9e89d97b64dcf01efc5395fab30f6c/ext/date/php_date.c#L1209-L1213

PHP Manual says:

mktime() returns the Unix timestamp of the arguments given, or false if the timestamp doesn't fit in a PHP integer.

It seems that even if PHP_INT_MAX is passed for all parameters, this value will never be exceeded.

https://3v4l.org/dnSu2

$defaultReturnType = new BenevolentUnionType([
new FloatType(),
new ConstantBooleanType(false),
]);

@zonuexe zonuexe Mar 22, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This extension change is unnecessary, but it removes the duplicate definition of the type with functionMap.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just stumbled over

if ($argType instanceof MixedType) {
return TypeUtils::toBenevolentUnion($defaultReturnType);
}
which might benefit from a similar simplification.

@ondrejmirtes

Copy link
Copy Markdown
Member

Thank you, changes make sense 😊

@ondrejmirtes
ondrejmirtes merged commit 15dce0c into phpstan:1.10.x Mar 22, 2024
@zonuexe
zonuexe deleted the fix/functionMap-use-benevolent branch June 2, 2024 16:04
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.

4 participants