ext/int: IntlTimeZone converting few intl error to exceptions. #13477
Conversation
| if (ZEND_NUM_ARGS() == 0) { | ||
| se = TimeZone::createEnumeration(); |
There was a problem hiding this comment.
Oh maybe the first arg should then be string|int|null, sorry didn't realize this at first.
There was a problem hiding this comment.
@kocsismate do you think the param should be nullable or not? Or is that not really an issue in regards to the RFC about overloaded methods?
There was a problem hiding this comment.
Uh, thanks for spotting this. The argument should be definitely nullable, otherwise it will be overloaded. @devnexen In most of the cases, using ZEND_NUM_ARGS() is a clear sign of an overloaded signature.
There was a problem hiding this comment.
However, I'm wondering how this parameter can be declared with the string|int type? Currently, its phpdoc says IntlTimeZone|string|int|float|null. 🤔
| argl = zend_dval_to_lval(dval); | ||
| if (!zend_is_long_compatible(argl, dval)) { | ||
| zend_incompatible_double_to_long_error(dval); | ||
| RETURN_FALSE; | ||
| } |
There was a problem hiding this comment.
Add a test for this, and I don't think you should return false here, as the behaviour should be the same, just emitting a deprecation notice :)
devnexen
force-pushed
the
ext_intl_to_typeerror
branch
from
09819d5 to
d9ffb18
Compare
|
ping :) |
No description provided.