Skip to content

Commit 61c299f

Browse files
committed
Error promotions in SPL
Warning to Error promotion and a Notice to Warning promotion to align with the behaviour specified in the Reclassify Engine Warnings RFC. Closes GH-6072
1 parent 4a438b4 commit 61c299f

9 files changed

+47
-41
lines changed

ext/spl/spl_array.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,9 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o
204204
parent = parent->parent;
205205
inherited = 1;
206206
}
207-
if (!parent) { /* this must never happen */
208-
php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of ArrayObject or ArrayIterator");
209-
}
207+
208+
ZEND_ASSERT(parent);
209+
210210
if (inherited) {
211211
intern->fptr_offset_get = zend_hash_str_find_ptr(&class_type->function_table, "offsetget", sizeof("offsetget") - 1);
212212
if (intern->fptr_offset_get->common.scope == parent) {

ext/spl/spl_directory.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, zend_long cto
732732
intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
733733
if (intern->_path) {
734734
/* object is already initialized */
735-
php_error_docref(NULL, E_WARNING, "Directory object is already initialized");
735+
zend_throw_error(NULL, "Directory object is already initialized");
736736
return;
737737
}
738738
intern->flags = flags;
@@ -1232,10 +1232,10 @@ PHP_METHOD(SplFileInfo, getLinkTarget)
12321232
}
12331233
#if defined(PHP_WIN32) || defined(HAVE_SYMLINK)
12341234
if (intern->file_name == NULL) {
1235-
zend_restore_error_handling(&error_handling);
1236-
php_error_docref(NULL, E_WARNING, "Empty filename");
1237-
RETURN_FALSE;
1238-
} else if (!IS_ABSOLUTE_PATH(intern->file_name, intern->file_name_len)) {
1235+
zend_value_error("Filename cannot be empty");
1236+
RETURN_THROWS();
1237+
}
1238+
if (!IS_ABSOLUTE_PATH(intern->file_name, intern->file_name_len)) {
12391239
char expanded_path[MAXPATHLEN];
12401240
if (!expand_filepath_with_mode(intern->file_name, expanded_path, NULL, 0, CWD_EXPAND )) {
12411241
zend_restore_error_handling(&error_handling);
@@ -1577,6 +1577,7 @@ PHP_METHOD(GlobIterator, count)
15771577
RETURN_LONG(php_glob_stream_get_count(intern->u.dir.dirp, NULL));
15781578
} else {
15791579
/* should not happen */
1580+
// TODO ZEND_ASSERT ?
15801581
php_error_docref(NULL, E_ERROR, "GlobIterator lost glob state");
15811582
}
15821583
}
@@ -2330,6 +2331,7 @@ PHP_METHOD(SplFileObject, fgetcsv)
23302331

23312332
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);
23322333

2334+
// TODO Align behaviour on normal fgetcsv()
23332335
switch(ZEND_NUM_ARGS())
23342336
{
23352337
case 3:
@@ -2377,6 +2379,8 @@ PHP_METHOD(SplFileObject, fputcsv)
23772379
zval *fields = NULL;
23782380

23792381
if (zend_parse_parameters(ZEND_NUM_ARGS(), "a|sss", &fields, &delim, &d_len, &enclo, &e_len, &esc, &esc_len) == SUCCESS) {
2382+
2383+
// TODO Align behaviour on normal fputcsv()
23802384
switch(ZEND_NUM_ARGS())
23812385
{
23822386
case 4:
@@ -2429,6 +2433,7 @@ PHP_METHOD(SplFileObject, setCsvControl)
24292433
size_t d_len = 0, e_len = 0, esc_len = 0;
24302434

24312435
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|sss", &delim, &d_len, &enclo, &e_len, &esc, &esc_len) == SUCCESS) {
2436+
// TODO Align behaviour on normal fgetcsv()
24322437
switch(ZEND_NUM_ARGS())
24332438
{
24342439
case 3:
@@ -2685,8 +2690,8 @@ PHP_METHOD(SplFileObject, fread)
26852690
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);
26862691

26872692
if (length <= 0) {
2688-
php_error_docref(NULL, E_WARNING, "Length parameter must be greater than 0");
2689-
RETURN_FALSE;
2693+
zend_argument_value_error(1, "must be greater than 0");
2694+
RETURN_THROWS();
26902695
}
26912696

26922697
str = php_stream_read_to_str(intern->u.file.stream, length);

ext/spl/spl_dllist.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,9 +413,8 @@ static zend_object *spl_dllist_object_new_ex(zend_class_entry *class_type, zend_
413413
inherited = 1;
414414
}
415415

416-
if (!parent) { /* this must never happen */
417-
php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of SplDoublyLinkedList");
418-
}
416+
ZEND_ASSERT(parent);
417+
419418
if (inherited) {
420419
intern->fptr_offset_get = zend_hash_str_find_ptr(&class_type->function_table, "offsetget", sizeof("offsetget") - 1);
421420
if (intern->fptr_offset_get->common.scope == parent) {

ext/spl/spl_fixedarray.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,7 @@ static zend_object *spl_fixedarray_object_new_ex(zend_class_entry *class_type, z
234234
inherited = 1;
235235
}
236236

237-
if (!parent) { /* this must never happen */
238-
php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of SplFixedArray");
239-
}
237+
ZEND_ASSERT(parent);
240238

241239
funcs_ptr = class_type->iterator_funcs_ptr;
242240
if (!funcs_ptr->zf_current) {

ext/spl/spl_heap.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -433,9 +433,7 @@ static zend_object *spl_heap_object_new_ex(zend_class_entry *class_type, zend_ob
433433
inherited = 1;
434434
}
435435

436-
if (!parent) { /* this must never happen */
437-
php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of SplHeap");
438-
}
436+
ZEND_ASSERT(parent);
439437

440438
if (inherited) {
441439
intern->fptr_cmp = zend_hash_str_find_ptr(&class_type->function_table, "compare", sizeof("compare") - 1);

ext/spl/spl_iterators.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,8 @@ static zend_object_iterator *spl_recursive_it_get_iterator(zend_class_entry *ce,
454454
iterator = emalloc(sizeof(spl_recursive_it_iterator));
455455
object = Z_SPLRECURSIVE_IT_P(zobject);
456456
if (object->iterators == NULL) {
457-
zend_error(E_ERROR, "The object to be iterated is in an invalid state: "
458-
"the parent constructor has not been called");
457+
zend_throw_error(NULL, "Object is not initialized");
458+
return NULL;
459459
}
460460

461461
zend_iterator_init((zend_object_iterator*)iterator);
@@ -2486,7 +2486,7 @@ PHP_METHOD(CachingIterator, offsetGet)
24862486
}
24872487

24882488
if ((value = zend_symtable_find(Z_ARRVAL(intern->u.caching.zcache), key)) == NULL) {
2489-
zend_error(E_NOTICE, "Undefined array key \"%s\"", ZSTR_VAL(key));
2489+
zend_error(E_WARNING, "Undefined array key \"%s\"", ZSTR_VAL(key));
24902490
return;
24912491
}
24922492

ext/spl/tests/bug61828.phpt

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ Bug #61828 (Memleak when calling Directory(Recursive)Iterator/Spl(Temp)FileObjec
33
--FILE--
44
<?php
55
$x = new DirectoryIterator('.');
6-
$x->__construct('/tmp');
7-
echo "Okey";
6+
7+
try {
8+
$x->__construct('/tmp');
9+
} catch (\Error $e) {
10+
echo $e->getMessage() . \PHP_EOL;
11+
}
12+
813
?>
9-
--EXPECTF--
10-
Warning: DirectoryIterator::__construct(): Directory object is already initialized in %sbug61828.php on line 3
11-
Okey
14+
--EXPECT--
15+
Directory object is already initialized

ext/spl/tests/bug65545.phpt

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,19 @@ $obj = new SplFileObject(__FILE__, 'r');
66
$data = $obj->fread(5);
77
var_dump($data);
88

9-
$data = $obj->fread(0);
10-
var_dump($data);
9+
try {
10+
$data = $obj->fread(0);
11+
var_dump($data);
12+
} catch (\ValueError $e) {
13+
echo $e->getMessage() . \PHP_EOL;
14+
}
1115

1216
// read more data than is available
1317
$data = $obj->fread(filesize(__FILE__) + 32);
1418
var_dump(strlen($data) === filesize(__FILE__) - 5);
1519

1620
?>
17-
--EXPECTF--
21+
--EXPECT--
1822
string(5) "<?php"
19-
20-
Warning: SplFileObject::fread(): Length parameter must be greater than 0 in %s on line %d
21-
bool(false)
23+
SplFileObject::fread(): Argument #1 ($length) must be greater than 0
2224
bool(true)

ext/spl/tests/iterator_044.phpt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ Exception: MyCachingIterator does not use a full cache (see CachingIterator::__c
7878
int(0)
7979
bool(false)
8080

81-
Notice: Undefined array key "0" in %s on line %d
81+
Warning: Undefined array key "0" in %s on line %d
8282
NULL
8383
===1===
8484
object(stdClass)#%d (0) {
@@ -90,31 +90,31 @@ object(MyFoo)#%d (0) {
9090
}
9191
bool(false)
9292

93-
Notice: Undefined array key "foo" in %s on line %d
93+
Warning: Undefined array key "foo" in %s on line %d
9494
NULL
9595
===3===
9696
NULL
9797
bool(false)
9898

99-
Notice: Undefined array key "" in %s on line %d
99+
Warning: Undefined array key "" in %s on line %d
100100
NULL
101101
===4===
102102
int(2)
103103
bool(false)
104104

105-
Notice: Undefined array key "2" in %s on line %d
105+
Warning: Undefined array key "2" in %s on line %d
106106
NULL
107107
===5===
108108
string(3) "foo"
109109
bool(false)
110110

111-
Notice: Undefined array key "foo" in %s on line %d
111+
Warning: Undefined array key "foo" in %s on line %d
112112
NULL
113113
===6===
114114
int(3)
115115
bool(false)
116116

117-
Notice: Undefined array key "3" in %s on line %d
117+
Warning: Undefined array key "3" in %s on line %d
118118
NULL
119119
===FILL===
120120
===0===
@@ -135,7 +135,7 @@ int(1)
135135
NULL
136136
bool(false)
137137

138-
Notice: Undefined array key "" in %s on line %d
138+
Warning: Undefined array key "" in %s on line %d
139139
NULL
140140
===4===
141141
int(2)
@@ -149,5 +149,5 @@ int(1)
149149
int(3)
150150
bool(false)
151151

152-
Notice: Undefined array key "3" in %s on line %d
152+
Warning: Undefined array key "3" in %s on line %d
153153
NULL

0 commit comments

Comments
 (0)