Skip to content

Commit 7e86c1d

Browse files
committed
Promote some IntlBreakIterator internal error handling to standard ValueError
1 parent ea2b0d3 commit 7e86c1d

File tree

3 files changed

+28
-36
lines changed

3 files changed

+28
-36
lines changed

ext/intl/breakiterator/breakiterator.stub.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function current() {}
3030
/** @return int */
3131
public function first() {}
3232

33-
/** @return int|false */
33+
/** @return int */
3434
public function following(int $offset) {}
3535

3636
/** @return int */
@@ -42,7 +42,7 @@ public function getErrorMessage() {}
4242
/** @return string */
4343
public function getLocale(int $locale_type) {}
4444

45-
/** @return IntlPartsIterator|false */
45+
/** @return IntlPartsIterator */
4646
public function getPartsIterator(string $key_type = IntlPartsIterator::KEY_SEQUENTIAL) {}
4747

4848
/** @return string|null */
@@ -54,10 +54,10 @@ public function isBoundary(int $offset) {}
5454
/** @return int */
5555
public function last() {}
5656

57-
/** @return int|false */
57+
/** @return int */
5858
public function next(?int $offset = null) {}
5959

60-
/** @return int|false */
60+
/** @return int */
6161
public function preceding(int $offset) {}
6262

6363
/** @return int */

ext/intl/breakiterator/breakiterator_methods.cpp

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ static void _breakiter_factory(const char *func_name,
6262

6363
biter = func(Locale::createFromName(locale_str), status);
6464
intl_error_set_code(NULL, status);
65+
// Todo check if this can happen?
6566
if (U_FAILURE(status)) {
6667
spprintf(&msg, 0, "%s: error creating BreakIterator",
6768
func_name);
@@ -169,7 +170,6 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, setText)
169170
}
170171

171172
static void _breakiter_no_args_ret_int32(
172-
const char *func_name,
173173
int32_t (BreakIterator::*func)(),
174174
INTERNAL_FUNCTION_PARAMETERS)
175175
{
@@ -189,11 +189,9 @@ static void _breakiter_no_args_ret_int32(
189189
}
190190

191191
static void _breakiter_int32_ret_int32(
192-
const char *func_name,
193192
int32_t (BreakIterator::*func)(int32_t),
194193
INTERNAL_FUNCTION_PARAMETERS)
195194
{
196-
char *msg;
197195
zend_long arg;
198196
BREAKITER_METHOD_INIT_VARS;
199197
object = ZEND_THIS;
@@ -205,11 +203,8 @@ static void _breakiter_int32_ret_int32(
205203
BREAKITER_METHOD_FETCH_OBJECT;
206204

207205
if (arg < INT32_MIN || arg > INT32_MAX) {
208-
spprintf(&msg, 0, "%s: offset argument is outside bounds of "
209-
"a 32-bit wide integer", func_name);
210-
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR, msg, 1);
211-
efree(msg);
212-
RETURN_FALSE;
206+
zend_argument_value_error(1, "must be between %d and %d", INT32_MIN, INT32_MAX);
207+
RETURN_THROWS();
213208
}
214209

215210
int32_t res = (bio->biter->*func)((int32_t)arg);
@@ -219,22 +214,19 @@ static void _breakiter_int32_ret_int32(
219214

220215
U_CFUNC PHP_METHOD(IntlBreakIterator, first)
221216
{
222-
_breakiter_no_args_ret_int32("breakiter_first",
223-
&BreakIterator::first,
217+
_breakiter_no_args_ret_int32(&BreakIterator::first,
224218
INTERNAL_FUNCTION_PARAM_PASSTHRU);
225219
}
226220

227221
U_CFUNC PHP_METHOD(IntlBreakIterator, last)
228222
{
229-
_breakiter_no_args_ret_int32("breakiter_last",
230-
&BreakIterator::last,
223+
_breakiter_no_args_ret_int32(&BreakIterator::last,
231224
INTERNAL_FUNCTION_PARAM_PASSTHRU);
232225
}
233226

234227
U_CFUNC PHP_METHOD(IntlBreakIterator, previous)
235228
{
236-
_breakiter_no_args_ret_int32("breakiter_previous",
237-
&BreakIterator::previous,
229+
_breakiter_no_args_ret_int32(&BreakIterator::previous,
238230
INTERNAL_FUNCTION_PARAM_PASSTHRU);
239231
}
240232

@@ -252,12 +244,10 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, next)
252244
if (arg == NULL) {
253245
ZEND_NUM_ARGS() = 0; /* pretend we don't have any argument */
254246
no_arg_version:
255-
_breakiter_no_args_ret_int32("breakiter_next",
256-
&BreakIterator::next,
247+
_breakiter_no_args_ret_int32(&BreakIterator::next,
257248
INTERNAL_FUNCTION_PARAM_PASSTHRU);
258249
} else {
259-
_breakiter_int32_ret_int32("breakiter_next",
260-
&BreakIterator::next,
250+
_breakiter_int32_ret_int32(&BreakIterator::next,
261251
INTERNAL_FUNCTION_PARAM_PASSTHRU);
262252
}
263253
}
@@ -280,14 +270,14 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, current)
280270

281271
U_CFUNC PHP_METHOD(IntlBreakIterator, following)
282272
{
283-
_breakiter_int32_ret_int32("breakiter_following",
273+
_breakiter_int32_ret_int32(
284274
&BreakIterator::following,
285275
INTERNAL_FUNCTION_PARAM_PASSTHRU);
286276
}
287277

288278
U_CFUNC PHP_METHOD(IntlBreakIterator, preceding)
289279
{
290-
_breakiter_int32_ret_int32("breakiter_preceding",
280+
_breakiter_int32_ret_int32(
291281
&BreakIterator::preceding,
292282
INTERNAL_FUNCTION_PARAM_PASSTHRU);
293283
}
@@ -304,10 +294,8 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, isBoundary)
304294
}
305295

306296
if (offset < INT32_MIN || offset > INT32_MAX) {
307-
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
308-
"breakiter_is_boundary: offset argument is outside bounds of "
309-
"a 32-bit wide integer", 0);
310-
RETURN_FALSE;
297+
zend_argument_value_error(1, "must be between %d and %d", INT32_MIN, INT32_MAX);
298+
RETURN_THROWS();
311299
}
312300

313301
BREAKITER_METHOD_FETCH_OBJECT;
@@ -327,6 +315,7 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, getLocale)
327315
RETURN_THROWS();
328316
}
329317

318+
/* Change to ValueError? */
330319
if (locale_type != ULOC_ACTUAL_LOCALE && locale_type != ULOC_VALID_LOCALE) {
331320
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
332321
"breakiter_get_locale: invalid locale type", 0);
@@ -356,9 +345,9 @@ U_CFUNC PHP_METHOD(IntlBreakIterator, getPartsIterator)
356345
if (key_type != PARTS_ITERATOR_KEY_SEQUENTIAL
357346
&& key_type != PARTS_ITERATOR_KEY_LEFT
358347
&& key_type != PARTS_ITERATOR_KEY_RIGHT) {
359-
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR,
360-
"breakiter_get_parts_iterator: bad key type", 0);
361-
RETURN_FALSE;
348+
zend_argument_value_error(1, "must be one of IntlPartsIterator::KEY_SEQUENTIAL, "
349+
"IntlPartsIterator::KEY_LEFT, or IntlPartsIterator::KEY_RIGHT");
350+
RETURN_THROWS();
362351
}
363352

364353
BREAKITER_METHOD_FETCH_OBJECT;

ext/intl/tests/breakiter_getPartsIterator_error.phpt

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@ if (!extension_loaded('intl'))
66
die('skip intl extension not enabled');
77
--FILE--
88
<?php
9-
ini_set("intl.error_level", E_WARNING);
109
ini_set("intl.default_locale", "pt_PT");
1110

1211
$it = IntlBreakIterator::createWordInstance(NULL);
13-
var_dump($it->getPartsIterator(-1));
12+
13+
try {
14+
var_dump($it->getPartsIterator(-1));
15+
} catch(\ValueError $e) {
16+
echo $e->getMessage() . \PHP_EOL;
17+
}
1418

1519
?>
16-
--EXPECTF--
17-
Warning: IntlBreakIterator::getPartsIterator(): breakiter_get_parts_iterator: bad key type in %s on line %d
18-
bool(false)
20+
--EXPECT--
21+
IntlBreakIterator::getPartsIterator(): Argument #1 ($key_type) must be one of IntlPartsIterator::KEY_SEQUENTIAL, IntlPartsIterator::KEY_LEFT, or IntlPartsIterator::KEY_RIGHT

0 commit comments

Comments
 (0)