Handle lowercase string in sprintf #3498
Conversation
|
This pull request has been marked as ready for review. |
There was a problem hiding this comment.
Does this case also need lowercase stuff?
VincentLanglet
force-pushed
the
sprintfLower
branch
from
b051b49 to
69a8b53
Compare
VincentLanglet
force-pushed
the
sprintfLower
branch
from
24ef8c3 to
86ddf78
Compare
| if ($isLowercase) { | ||
| $singlePlaceholderEarlyReturn = new IntersectionType([ | ||
| new StringType(), | ||
| new AccessoryLowercaseStringType(), | ||
| new AccessoryNumericStringType(), | ||
| ]); | ||
| } else { | ||
| $singlePlaceholderEarlyReturn = new IntersectionType([ | ||
| new StringType(), | ||
| new AccessoryNumericStringType(), | ||
| ]); | ||
| } |
There was a problem hiding this comment.
maybe it would make sense to extract this "if lowercase, add lowercase-type" into a separate method, so you don't need to repeat it over and over
There was a problem hiding this comment.
Tried b22c45b
|
This pull request has been marked as ready for review. |
| continue; | ||
| } | ||
|
|
||
| $singlePlaceholderEarlyReturn = $checkArgType->toString(); |
There was a problem hiding this comment.
I wonder this line is also missing lowercase handling
maybe instead of fiddling with this complex loop and all its paths, just add it once here:
if ($singlePlaceholderEarlyReturn !== null) {
return $singlePlaceholderEarlyReturn;
}
There was a problem hiding this comment.
I shouldn't add lowercase handling here and just rely on toString.
I'll open a PR to update IntegerType::toString to fix https://phpstan.org/r/79f89e3e-f19f-4a44-bd58-63f3c77db487
|
Thank you! |
4 participants
No description provided.