support constant string-unions in substr()#1532
Conversation
There was a problem hiding this comment.
it might be worth adding an assertType for the false return case when asking for invalid length offsets?
also substr in php7 and php8 return different values when given invalid $length or $offsets; 7 returns false whereas 8 returns '':
https://www.php.net/manual/en/function.substr.php#refsect1-function.substr-changelog
There was a problem hiding this comment.
agreed, added a few cases.
71f3506 to
220b57c
Compare
|
I think its good to go. the composer error is pretty exotic, as it is related to some kind of build step involved (files getting copied into different directories) and therefore the related line related composer commit which introduced this exotic logic composer/composer@e087a4a related composer issue which required the workaround in composer: composer/composer#9937 the PrestaShop error relates to these lines in which phpstan seems to be able to infer the constant values involved and therefore now figures out that a if-case will always be false. seems presta shop needs to configured those constants involved to be dynamic in the phpstan.neon |
There was a problem hiding this comment.
You could use ConstantTypeHelper::getTypeFromValue() instead (even though it's deprecated)
There was a problem hiding this comment.
I searched the code-base yesterday for this method .. I knew we had such thing, but wasn't able to find it :)
as you mentioned - the method is deprecated and seems to be a rather big canon for the rather small problem I solve here - I feel the PR should stay as is in this regard.
| } | ||
|
|
||
| $constantStrings = TypeUtils::getConstantStrings($string); | ||
| if (count($constantStrings) > 0 && |
There was a problem hiding this comment.
I format these differently:
if (
count($constantStrings) > 0
&& $offset instanceof ConstantIntegerType
&& ($length === null || $length instanceof ConstantIntegerType)
) {|
Thank you! |
closes phpstan/phpstan#7663