support subtracted-mixed in toNumber() #1949
Conversation
staabm
force-pushed
the
mixed-to-number
branch
2 times, most recently
from
c6b2f86
to
8c13b7c
Compare
|
This pull request has been marked as ready for review. |
src/Type/MixedType.php
Outdated
| if ($float->isSuperTypeOf($this->subtractedType)->yes()) { | ||
| return $int; | ||
| } | ||
| if ($int->isSuperTypeOf($this->subtractedType)->yes()) { | ||
| return $float; | ||
| } |
There was a problem hiding this comment.
What if both float and int are subtracted?
There was a problem hiding this comment.
sorry, can someone explain to me how this works in general? if something is e.g. not a float it could be e.g. an object still and objects can't be incremented?
There was a problem hiding this comment.
What if both
floatandintare subtracted?
yeah, we could return a NeverType then.
sorry, can someone explain to me how this works in general? if something is e.g. not a float it could be e.g. an object still and objects can't be incremented?
the incrementing test-case is kind of synthetic.. I "reverse engineered" it from looking at toNumber() uses in the codebase:
phpstan-src/src/Analyser/MutatingScope.php
Lines 975 to 977 in 213863e
decting invalid cases like $object++ is the job of a rule, not the narrowing done here I think
There was a problem hiding this comment.
Even if float is subtracted, the result can still be a float... Because MixedType without FloatType can still be numeric-string for example... https://3v4l.org/StqSP
staabm
force-pushed
the
mixed-to-number
branch
from
8c13b7c
to
44894ad
Compare
| if (!is_int($mixed)) { | ||
| assertType('mixed~float|int', $mixed); | ||
| $mixed++; | ||
| assertType('*NEVER*', $mixed); |
There was a problem hiding this comment.
Could still be a string though? https://3v4l.org/D4BHP
There was a problem hiding this comment.
yeah,.. I think the src/ changes actually makes sense .. but my test is to synthetic and your example shows that we cannot return a Never type for toNumber
staabm
force-pushed
the
mixed-to-number
branch
2 times, most recently
from
fbd9f6d
to
0450d0f
Compare
|
This pull request has been marked as ready for review. |
staabm
force-pushed
the
mixed-to-number
branch
3 times, most recently
from
03f0a1b
to
52539e7
Compare
|
Oh actually, other types can be cast to number as well 🤦 |
staabm
force-pushed
the
mixed-to-number
branch
from
49209d6
to
8a452c0
Compare
|
thank you guys for all the examples and ideas. I think we / I learnt from it, that we can't take subtractable into account in I put all the examples into a unit test and fixed a edge case with the PR no longer does what its title suggested, but at least our time discussing it is not lost, but manifested in more testcoverage. thanks again. |
staabm
force-pushed
the
mixed-to-number
branch
from
8a452c0
to
d9d607c
Compare
|
Yeah, at least it was still productive :) Thank you. |
No description provided.