fix parameter of strval, intval and floatval#1005
fix parameter of strval, intval and floatval#1005Khartir wants to merge 6 commits intophpstan:1.5.xfrom
Conversation
tests/PHPStan/Rules/Functions/StrvalFamilyParametersRuleTest.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I'm not aware of any restrictions on boolval. As far as I know it really accepts mixed. Am I missing something?
Also I think I wiill remove everything but strval. Only strval needs special handling due to Stringable. Everything else should be fixable by changing the function signature in functionMap.
|
Errr, I thought that you want to implement a dynamic return type extension so that the result type of If you need to check the parameter type then sure, a change in |
5ea3402 to
1073a5e
Compare
There was a problem hiding this comment.
Is there a better way, to check if this is an instance of any object?
|
After changing the |
86408aa to
cc28967
Compare
resources/functionMap.php
Outdated
There was a problem hiding this comment.
Is it ever useful to allow the array and resource here?
There was a problem hiding this comment.
I'm also a bit confused - doubleval does not accept object here which should already be picked up by an existing rule (CallToFunctionParametersRule) but apparently isn't. Any idea why?
There was a problem hiding this comment.
Is it ever useful to allow the
arrayandresourcehere?
Probably not. I was attempting to keep (float) and floatval() in line and used the actual PHP behavior as a guideline along with the fact that casting to int is possible.
I'm also a bit confused -
doublevaldoes not acceptobjecthere which should already be picked up by an existing rule (CallToFunctionParametersRule) but apparently isn't. Any idea why?
I have to check again, but if I remember correctly, it does pick up regular objects, but has a special rule to allow objects with a __toString-method. Which in general is the correct behavior. intval and floatval are the special case, in that they accept string but not Stringable:
php > $stringable = new class() {public function __toString(): string {return '';}};
php > echo strlen($stringable);
0
php > echo intval($stringable);
PHP Notice: Object of class class@anonymous could not be converted to int in php shell code on line 1
There was a problem hiding this comment.
I have added a commit to forbid casting resource to float. I'm looking into doing the same with array but that causes a regression for phpstan/phpstan#5162.
There was a problem hiding this comment.
So I looked into phpstan/phpstan#5162. The problem is, that the fix for that issue was to explicitly allow casting array to float. If we forbid it here again, this will obviously fail again. In order to resolve this, the underlying issue would have to be resolved:
/** @var array<string,string|array<string>> $result */
$result = $this->Get();
if ( array_key_exists('a',$result) && ! is_numeric( $result['a'] ) )
{
exit(1);
}
assertType('numeric-string', $result['a']); //this currently reports 'array<string>|string'
/** @var array<string,string|array<string>> $result2 */
$result2 = $this->Get();
if ( ! is_numeric( $result['a'] ) )
{
exit(1);
}
assertType('numeric-string', $result['a']); //this worksAs far as I can tell in the first case the resolved type of $result['a'] is not updated, because it might not even exist. Obviously the type of $result['a'] could also be ERROR if the offset doesn't exist.
However I have no idea how to fix this.
cc28967 to
3ab0c2b
Compare
ddd20b4 to
95d480b
Compare
resolves phpstan/phpstan#6560
I implemented a rule instead of a dynamic return type extension as mentioned in phpstan/phpstan#6560 (comment). I think that was a misunderstanding? After all the problem is the parameter type, not the return type of the function. For the return type StrvalFamilyFunctionReturnTypeExtension already exists.