support subtracted-mixed in toNumber()#1949
Conversation
c6b2f86 to
8c13b7c
Compare
|
This pull request has been marked as ready for review. |
src/Type/MixedType.php
Outdated
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
8c13b7c to
44894ad
Compare
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
fbd9f6d to
0450d0f
Compare
|
This pull request has been marked as ready for review. |
03f0a1b to
52539e7
Compare
ondrejmirtes
left a comment
There was a problem hiding this comment.
Please return ErrorType if the subtracted type makes sure it can never be a number. Thanks.
|
Oh actually, other types can be cast to number as well 🤦 |
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. |
8a452c0 to
d9d607c
Compare
|
Yeah, at least it was still productive :) Thank you. |
No description provided.