ctype_digit() narrows to decimal-int-string

[staabm]

No description provided.

[phpstan-bot]

You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x.

[staabm]

bug before PR: numeric strings exist which ctype_digit does not return true, for see https://3v4l.org/1Qrlg#veol

[VincentLanglet]

Should be fix on 2.1 then ?

[staabm]

Not sure its worth fixing on 2.1.x because noone reported it yet and I think for a good fix we need decimal-int-string type which only exists on 2.2.x

[ondrejmirtes]

I’d put brakes on this a bit. It’s an experimental type, it’s too soon to propagate it around the codebase.

[ondrejmirtes]

My current idea is that there is going to be 3-way setting about “unsafe array string keys”. The default would be current behaviour (unsafe), to not break users’ baselines with more specific types.

[staabm]

I see. lets park it for now and come back after the array-key topic is more mature.

[VincentLanglet]

My current idea is that there is going to be 3-way setting about “unsafe array string keys”. The default would be current behaviour (unsafe), to not break users’ baselines with more specific types.

I also tried #5312 on my side @ondrejmirtes.
I won't spend more time on this and let you do it like you want but here's my thought:

• I have some trouble to see how to have this new type efficient without propagating it to the whole codebase
• decimal-int-string&lowercase-string&uppercase-string seems redundant, a decimal-int-string is ALWAYS lowercase and uppercase ; we should:
  • Simplify the description in IntersectionType
  • Avoid adding both when possible
• Breaking the user baseline with more specific seems ok to me in minor, especially when this certainly already occured without people complaining a lot about it when we introduced lowercase-string or uppercase-string ; but if you really want to avoid this, maybe we could just describe the decimal-int-string as numeric-string&lowercase-string&uppercase-string for people with the toggle off.

[ondrejmirtes]

Yes, decimal-int-string&lowercase-string&uppercase-string is redundant, this should be reduced in TypeCombinator, you can write a test and modify some code in isSuperTypeOf/isSubTypeOf.

[staabm]

@ondrejmirtes can we go ahead here?

the newly reported issue made me remember this PR

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

What about strings with negative integers? AFAIK they are still cast to integers in array keys.

[staabm]

they are cast to integers but do not return true on ctype_digit

https://3v4l.org/vo6gg#veol

[ondrejmirtes]

Thank you!