Skip to content

Commit a7968a3

Browse files
GirgiasGirgias
committed
Refactor GregorianCalendar constructor
1 parent 9962aa9 commit a7968a3

File tree

5 files changed

+99
-95
lines changed

5 files changed

+99
-95
lines changed

‎ext/intl/calendar/calendar.stub.php‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,8 @@ class IntlGregorianCalendar extends IntlCalendar
284284
{
285285
/**
286286
* @param DateTimeZone|IntlTimeZone|string|int|null $timezoneOrYear
287-
* @param string|int|null $localeOrMonth
288-
* @param int $day
289-
* @param int $hour
290-
* @param int $minute
291-
* @param int $second
292287
*/
293-
public function __construct($timezoneOrYear = UNKNOWN, $localeOrMonth = UNKNOWN, $day = UNKNOWN, $hour = UNKNOWN, $minute = UNKNOWN, $second = UNKNOWN) {}
288+
public function __construct($timezoneOrYear = null, string|int|null $localeOrMonth = null, ?int $day = null, int $hour = 0, int $minute = 0, int $second = 0) {}
294289

295290
/**
296291
* @tentative-return-type

‎ext/intl/calendar/calendar_arginfo.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 2265c2a4f478d6ccd576ce09a19a158df38a2bdb */
2+
* Stub hash: 556e84d52ad03a5eab6007d45d80ea9301fefd75 */
33

44
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_IntlCalendar___construct, 0, 0, 0)
55
ZEND_END_ARG_INFO()
@@ -157,12 +157,12 @@ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_OBJ_TYPE_MASK_EX(arginfo_class_IntlCalendar
157157
ZEND_END_ARG_INFO()
158158

159159
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_IntlGregorianCalendar___construct, 0, 0, 0)
160-
ZEND_ARG_INFO(0, timezoneOrYear)
161-
ZEND_ARG_INFO(0, localeOrMonth)
162-
ZEND_ARG_INFO(0, day)
163-
ZEND_ARG_INFO(0, hour)
164-
ZEND_ARG_INFO(0, minute)
165-
ZEND_ARG_INFO(0, second)
160+
ZEND_ARG_INFO_WITH_DEFAULT_VALUE(0, timezoneOrYear, "null")
161+
ZEND_ARG_TYPE_MASK(0, localeOrMonth, MAY_BE_STRING|MAY_BE_LONG|MAY_BE_NULL, "null")
162+
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, day, IS_LONG, 1, "null")
163+
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, hour, IS_LONG, 0, "0")
164+
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, minute, IS_LONG, 0, "0")
165+
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, second, IS_LONG, 0, "0")
166166
ZEND_END_ARG_INFO()
167167

168168
#define arginfo_class_IntlGregorianCalendar_setGregorianChange arginfo_class_IntlCalendar_setTime

‎ext/intl/calendar/gregoriancalendar_methods.cpp‎

Lines changed: 81 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -43,48 +43,32 @@ static inline GregorianCalendar *fetch_greg(Calendar_object *co) {
4343
return (GregorianCalendar*)co->ucal;
4444
}
4545

46-
static void _php_intlgregcal_constructor_body(
47-
INTERNAL_FUNCTION_PARAMETERS, bool is_constructor)
46+
static void _php_intlgregcal_constructor_body(INTERNAL_FUNCTION_PARAMETERS, bool is_constructor)
4847
{
49-
zval *tz_object = NULL;
50-
zval args_a[6],
51-
*args = &args_a[0];
52-
char *locale = NULL;
53-
size_t locale_len;
54-
zend_long largs[6];
55-
UErrorCode status = U_ZERO_ERROR;
56-
int variant;
57-
intl_error_reset(NULL);
58-
59-
// parameter number validation / variant determination
60-
if (ZEND_NUM_ARGS() > 6 ||
61-
zend_get_parameters_array_ex(ZEND_NUM_ARGS(), args) == FAILURE) {
62-
zend_argument_count_error("Too many arguments");
63-
RETURN_THROWS();
64-
}
65-
66-
for (variant = ZEND_NUM_ARGS();
67-
variant > 0 && Z_TYPE(args[variant - 1]) == IS_NULL;
68-
variant--) {}
69-
if (variant == 4) {
70-
zend_argument_count_error("No variant with 4 arguments (excluding trailing NULLs)");
71-
RETURN_THROWS();
72-
}
48+
UErrorCode status = U_ZERO_ERROR;
49+
zval *timezone_or_year = NULL;
50+
zend_string *locale = NULL;
51+
zend_long month;
52+
bool is_month_null = true;
53+
zend_long day_of_month = 0;
54+
bool is_dom_null = true;
55+
zend_long hour = 0;
56+
zend_long minutes = 0;
57+
zend_long seconds = 0;
58+
59+
ZEND_PARSE_PARAMETERS_START(0, 6)
60+
Z_PARAM_OPTIONAL
61+
Z_PARAM_ZVAL(timezone_or_year)
62+
Z_PARAM_STR_OR_LONG_OR_NULL(locale, month, is_month_null)
63+
Z_PARAM_LONG_OR_NULL(day_of_month, is_dom_null)
64+
Z_PARAM_LONG(hour)
65+
Z_PARAM_LONG(minutes)
66+
Z_PARAM_LONG(seconds)
67+
ZEND_PARSE_PARAMETERS_END();
7368

74-
// argument parsing
75-
if (variant <= 2) {
76-
if (zend_parse_parameters(MIN(ZEND_NUM_ARGS(), 2),
77-
"|z!s!", &tz_object, &locale, &locale_len) == FAILURE) {
78-
RETURN_THROWS();
79-
}
80-
}
81-
if (variant > 2 && zend_parse_parameters(ZEND_NUM_ARGS(),
82-
"lll|lll", &largs[0], &largs[1], &largs[2], &largs[3], &largs[4],
83-
&largs[5]) == FAILURE) {
84-
RETURN_THROWS();
85-
}
69+
intl_error_reset(NULL);
8670

87-
// instantion of ICU object
71+
// instantiation of ICU object
8872
Calendar_object *co = Z_INTL_CALENDAR_P(return_value);
8973
GregorianCalendar *gcal = NULL;
9074

@@ -93,27 +77,33 @@ static void _php_intlgregcal_constructor_body(
9377
RETURN_THROWS();
9478
}
9579

96-
if (variant <= 2) {
97-
// From timezone and locale (0 to 2 arguments)
98-
TimeZone *tz = timezone_process_timezone_argument(tz_object, NULL,
80+
if (!timezone_or_year || Z_TYPE_P(timezone_or_year) != IS_LONG) {
81+
if (!locale && !is_month_null) {
82+
zend_argument_value_error(2, "must be ?string if argument 1 is a timezone");
83+
RETURN_THROWS();
84+
}
85+
86+
TimeZone *tz = timezone_process_timezone_argument(timezone_or_year, NULL,
9987
"intlgregcal_create_instance");
10088
if (tz == NULL) {
10189
if (!EG(exception)) {
10290
zend_throw_exception(IntlException_ce_ptr, "Constructor failed", 0);
10391
}
10492
if (!is_constructor) {
10593
zval_ptr_dtor(return_value);
106-
RETVAL_NULL();
94+
RETURN_NULL();
10795
}
108-
return;
96+
RETURN_THROWS();
10997
}
110-
if (!locale) {
111-
locale = const_cast<char*>(intl_locale_get_default());
98+
char *intl_locale;
99+
if (locale) {
100+
intl_locale = ZSTR_VAL(locale);
101+
} else {
102+
intl_locale = const_cast<char*>(intl_locale_get_default());
112103
}
113104

114-
gcal = new GregorianCalendar(tz, Locale::createFromName(locale),
115-
status);
116-
// Should this throw?
105+
gcal = new GregorianCalendar(tz, Locale::createFromName(intl_locale), status);
106+
// Should this always throw?
117107
if (U_FAILURE(status)) {
118108
intl_error_set(NULL, status, "intlgregcal_create_instance: error "
119109
"creating ICU GregorianCalendar from time zone and locale", 0);
@@ -123,31 +113,50 @@ static void _php_intlgregcal_constructor_body(
123113
delete tz;
124114
if (!is_constructor) {
125115
zval_ptr_dtor(return_value);
126-
RETVAL_NULL();
116+
RETURN_NULL();
127117
}
128-
return;
118+
RETURN_THROWS();
129119
}
130120
} else {
131-
// From date/time (3, 5 or 6 arguments)
132-
for (int i = 0; i < variant; i++) {
133-
if (largs[i] < INT32_MIN || largs[i] > INT32_MAX) {
134-
zend_argument_value_error(getThis() ? (i-1) : i,
135-
"must be between %d and %d", INT32_MIN, INT32_MAX);
136-
RETURN_THROWS();
137-
}
121+
ZEND_ASSERT(Z_TYPE_P(timezone_or_year) == IS_LONG);
122+
if (locale || is_month_null) {
123+
zend_argument_value_error(2, "must be int if argument 1 is an int");
124+
RETURN_THROWS();
125+
}
126+
if (is_dom_null) {
127+
zend_argument_value_error(3, "must be provided");
128+
RETURN_THROWS();
138129
}
139130

140-
if (variant == 3) {
141-
gcal = new GregorianCalendar((int32_t)largs[0], (int32_t)largs[1],
142-
(int32_t)largs[2], status);
143-
} else if (variant == 5) {
144-
gcal = new GregorianCalendar((int32_t)largs[0], (int32_t)largs[1],
145-
(int32_t)largs[2], (int32_t)largs[3], (int32_t)largs[4], status);
146-
} else if (variant == 6) {
147-
gcal = new GregorianCalendar((int32_t)largs[0], (int32_t)largs[1],
148-
(int32_t)largs[2], (int32_t)largs[3], (int32_t)largs[4], (int32_t)largs[5],
149-
status);
131+
zend_long year = Z_LVAL_P(timezone_or_year);
132+
if (year < INT32_MIN || year > INT32_MAX) {
133+
zend_argument_value_error(3, "must be between %d and %d", INT32_MIN, INT32_MAX);
134+
RETURN_THROWS();
135+
}
136+
if (month < INT32_MIN || month > INT32_MAX) {
137+
zend_argument_value_error(3, "must be between %d and %d", INT32_MIN, INT32_MAX);
138+
RETURN_THROWS();
139+
}
140+
if (day_of_month < INT32_MIN || day_of_month > INT32_MAX) {
141+
zend_argument_value_error(3, "must be between %d and %d", INT32_MIN, INT32_MAX);
142+
RETURN_THROWS();
143+
}
144+
if (hour < INT32_MIN || hour > INT32_MAX) {
145+
zend_argument_value_error(4, "must be between %d and %d", INT32_MIN, INT32_MAX);
146+
RETURN_THROWS();
147+
}
148+
if (minutes < INT32_MIN || minutes > INT32_MAX) {
149+
zend_argument_value_error(5, "must be between %d and %d", INT32_MIN, INT32_MAX);
150+
RETURN_THROWS();
151+
}
152+
if (seconds < INT32_MIN || seconds > INT32_MAX) {
153+
zend_argument_value_error(6, "must be between %d and %d", INT32_MIN, INT32_MAX);
154+
RETURN_THROWS();
150155
}
156+
157+
gcal = new GregorianCalendar((int32_t)year, (int32_t)month, (int32_t)day_of_month,
158+
(int32_t)hour, (int32_t)minutes, (int32_t)seconds, status);
159+
151160
if (U_FAILURE(status)) {
152161
intl_error_set(NULL, status, "intlgregcal_create_instance: error "
153162
"creating ICU GregorianCalendar from date", 0);
@@ -156,9 +165,9 @@ static void _php_intlgregcal_constructor_body(
156165
}
157166
if (!is_constructor) {
158167
zval_ptr_dtor(return_value);
159-
RETVAL_NULL();
168+
RETURN_NULL();
160169
}
161-
return;
170+
RETURN_THROWS();
162171
}
163172

164173
timelib_tzinfo *tzinfo = get_timezone_info();
@@ -171,9 +180,9 @@ static void _php_intlgregcal_constructor_body(
171180
delete gcal;
172181
if (!is_constructor) {
173182
zval_ptr_dtor(return_value);
174-
RETVAL_NULL();
183+
RETURN_NULL();
175184
}
176-
return;
185+
RETURN_THROWS();
177186
}
178187

179188
TimeZone *tz = TimeZone::createTimeZone(tzstr);

‎ext/intl/tests/gregoriancalendar___construct_basic.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ $intlcal = new IntlGregorianCalendar('Europe/Lisbon', 'pt_PT');
2525
var_dump($intlcal->getTimeZone()->getId());
2626
var_dump($intlcal->getLocale(1));
2727

28-
$intlcal = new IntlGregorianCalendar('Europe/Paris', 'fr_CA', NULL, NULL, NULL, NULL);
28+
$intlcal = new IntlGregorianCalendar('Europe/Paris', 'fr_CA');
2929
var_dump($intlcal->getTimeZone()->getId());
3030
var_dump($intlcal->getLocale(1));
3131

‎ext/intl/tests/gregoriancalendar___construct_error.phpt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,18 @@ try {
1212
echo $e->getMessage(), "\n";
1313
}
1414
try {
15-
var_dump(intlgregcal_create_instance(1,2,3,4,5,6,7,8));
16-
} catch (ArgumentCountError $e) {
15+
var_dump(new IntlGregorianCalendar(null, 1));
16+
} catch (ValueError $e) {
1717
echo $e->getMessage(), "\n";
1818
}
1919
try {
20-
var_dump(intlgregcal_create_instance(1,2,3,4));
21-
} catch (ArgumentCountError $e) {
20+
var_dump(new IntlGregorianCalendar(1, null));
21+
} catch (ValueError $e) {
2222
echo $e->getMessage(), "\n";
2323
}
2424
try {
2525
var_dump(new IntlGregorianCalendar(1,2,NULL,4));
26-
} catch (ArgumentCountError $e) {
26+
} catch (ValueError $e) {
2727
echo $e->getMessage(), "\n";
2828
}
2929
try {
@@ -40,9 +40,9 @@ try {
4040
}
4141
?>
4242
--EXPECT--
43-
Too many arguments
44-
Too many arguments
45-
No variant with 4 arguments (excluding trailing NULLs)
46-
No variant with 4 arguments (excluding trailing NULLs)
43+
intlgregcal_create_instance() expects at most 6 arguments, 7 given
44+
IntlGregorianCalendar::__construct(): Argument #2 ($localeOrMonth) must be ?string if argument 1 is a timezone
45+
IntlGregorianCalendar::__construct(): Argument #2 ($localeOrMonth) must be int if argument 1 is an int
46+
IntlGregorianCalendar::__construct(): Argument #3 ($day) must be provided
4747
IntlGregorianCalendar::__construct(): Argument #6 ($second) must be of type int, array given
4848
IntlGregorianCalendar object is already constructed

0 commit comments

Comments
 (0)