Error promotions in SPL
authorGeorge Peter Banyard <girgias@php.net>
Thu, 3 Sep 2020 15:17:46 +0000 (17:17 +0200)
committerGeorge Peter Banyard <girgias@php.net>
Thu, 3 Sep 2020 17:27:02 +0000 (19:27 +0200)
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

ext/spl/spl_array.c
ext/spl/spl_directory.c
ext/spl/spl_dllist.c
ext/spl/spl_fixedarray.c
ext/spl/spl_heap.c
ext/spl/spl_iterators.c
ext/spl/tests/bug61828.phpt
ext/spl/tests/bug65545.phpt
ext/spl/tests/iterator_044.phpt

index 5beddc3391929027ec502a1eb973ece2578c1c2a..ce468267d089ace92bee21b414fcd027e4b1fb02 100644 (file)
@@ -204,9 +204,9 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o
                parent = parent->parent;
                inherited = 1;
        }
-       if (!parent) { /* this must never happen */
-               php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of ArrayObject or ArrayIterator");
-       }
+
+       ZEND_ASSERT(parent);
+
        if (inherited) {
                intern->fptr_offset_get = zend_hash_str_find_ptr(&class_type->function_table, "offsetget", sizeof("offsetget") - 1);
                if (intern->fptr_offset_get->common.scope == parent) {
index e7bc262da5236f99f938c704ccce7d046bcc4346..e782b1e70a462e857fc9a5919a715486ec08ea18 100644 (file)
@@ -732,7 +732,7 @@ void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, zend_long cto
        intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
        if (intern->_path) {
                /* object is already initialized */
-               php_error_docref(NULL, E_WARNING, "Directory object is already initialized");
+               zend_throw_error(NULL, "Directory object is already initialized");
                return;
        }
        intern->flags = flags;
@@ -1232,10 +1232,10 @@ PHP_METHOD(SplFileInfo, getLinkTarget)
        }
 #if defined(PHP_WIN32) || defined(HAVE_SYMLINK)
        if (intern->file_name == NULL) {
-               zend_restore_error_handling(&error_handling);
-               php_error_docref(NULL, E_WARNING, "Empty filename");
-               RETURN_FALSE;
-       } else if (!IS_ABSOLUTE_PATH(intern->file_name, intern->file_name_len)) {
+               zend_value_error("Filename cannot be empty");
+               RETURN_THROWS();
+       }
+       if (!IS_ABSOLUTE_PATH(intern->file_name, intern->file_name_len)) {
                char expanded_path[MAXPATHLEN];
                if (!expand_filepath_with_mode(intern->file_name, expanded_path, NULL, 0, CWD_EXPAND )) {
                        zend_restore_error_handling(&error_handling);
@@ -1577,6 +1577,7 @@ PHP_METHOD(GlobIterator, count)
                RETURN_LONG(php_glob_stream_get_count(intern->u.dir.dirp, NULL));
        } else {
                /* should not happen */
+               // TODO ZEND_ASSERT ?
                php_error_docref(NULL, E_ERROR, "GlobIterator lost glob state");
        }
 }
@@ -2330,6 +2331,7 @@ PHP_METHOD(SplFileObject, fgetcsv)
 
                CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);
 
+               // TODO Align behaviour on normal fgetcsv()
                switch(ZEND_NUM_ARGS())
                {
                case 3:
@@ -2377,6 +2379,8 @@ PHP_METHOD(SplFileObject, fputcsv)
        zval *fields = NULL;
 
        if (zend_parse_parameters(ZEND_NUM_ARGS(), "a|sss", &fields, &delim, &d_len, &enclo, &e_len, &esc, &esc_len) == SUCCESS) {
+
+               // TODO Align behaviour on normal fputcsv()
                switch(ZEND_NUM_ARGS())
                {
                case 4:
@@ -2429,6 +2433,7 @@ PHP_METHOD(SplFileObject, setCsvControl)
        size_t d_len = 0, e_len = 0, esc_len = 0;
 
        if (zend_parse_parameters(ZEND_NUM_ARGS(), "|sss", &delim, &d_len, &enclo, &e_len, &esc, &esc_len) == SUCCESS) {
+               // TODO Align behaviour on normal fgetcsv()
                switch(ZEND_NUM_ARGS())
                {
                case 3:
@@ -2685,8 +2690,8 @@ PHP_METHOD(SplFileObject, fread)
        CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);
 
        if (length <= 0) {
-               php_error_docref(NULL, E_WARNING, "Length parameter must be greater than 0");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "must be greater than 0");
+               RETURN_THROWS();
        }
 
        str = php_stream_read_to_str(intern->u.file.stream, length);
index 9d066ecf13216fa75877ddb8456d692e14d1c214..eb01c09d80fc71375e690aa53b0e623efedc1ec9 100644 (file)
@@ -413,9 +413,8 @@ static zend_object *spl_dllist_object_new_ex(zend_class_entry *class_type, zend_
                inherited = 1;
        }
 
-       if (!parent) { /* this must never happen */
-               php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of SplDoublyLinkedList");
-       }
+       ZEND_ASSERT(parent);
+
        if (inherited) {
                intern->fptr_offset_get = zend_hash_str_find_ptr(&class_type->function_table, "offsetget", sizeof("offsetget") - 1);
                if (intern->fptr_offset_get->common.scope == parent) {
index 866a0a9a7869c7e638aadda48f8269021e7d59b7..558514eec1e5035cf066bf4c851a704764174d15 100644 (file)
@@ -234,9 +234,7 @@ static zend_object *spl_fixedarray_object_new_ex(zend_class_entry *class_type, z
                inherited = 1;
        }
 
-       if (!parent) { /* this must never happen */
-               php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of SplFixedArray");
-       }
+       ZEND_ASSERT(parent);
 
        funcs_ptr = class_type->iterator_funcs_ptr;
        if (!funcs_ptr->zf_current) {
index e23b0f242138d197d48f928d3e1a11ecb0609c84..0beea11da078094737068f6165fe35460138b4d7 100644 (file)
@@ -433,9 +433,7 @@ static zend_object *spl_heap_object_new_ex(zend_class_entry *class_type, zend_ob
                inherited = 1;
        }
 
-       if (!parent) { /* this must never happen */
-               php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of SplHeap");
-       }
+       ZEND_ASSERT(parent);
 
        if (inherited) {
                intern->fptr_cmp = zend_hash_str_find_ptr(&class_type->function_table, "compare", sizeof("compare") - 1);
index 242fcfbfae88fe93d879581f06e1b71a657d43fc..d245cc356731bdddf5ba49958308a978c8c1df6d 100644 (file)
@@ -454,8 +454,8 @@ static zend_object_iterator *spl_recursive_it_get_iterator(zend_class_entry *ce,
        iterator = emalloc(sizeof(spl_recursive_it_iterator));
        object   = Z_SPLRECURSIVE_IT_P(zobject);
        if (object->iterators == NULL) {
-               zend_error(E_ERROR, "The object to be iterated is in an invalid state: "
-                               "the parent constructor has not been called");
+               zend_throw_error(NULL, "Object is not initialized");
+               return NULL;
        }
 
        zend_iterator_init((zend_object_iterator*)iterator);
@@ -2486,7 +2486,7 @@ PHP_METHOD(CachingIterator, offsetGet)
        }
 
        if ((value = zend_symtable_find(Z_ARRVAL(intern->u.caching.zcache), key)) == NULL) {
-               zend_error(E_NOTICE, "Undefined array key \"%s\"", ZSTR_VAL(key));
+               zend_error(E_WARNING, "Undefined array key \"%s\"", ZSTR_VAL(key));
                return;
        }
 
index 04d435e6d6edd6ae61f0aeeef70c0beed5e19eb2..2a11b760bb353764f5981300cbdcafb9b1e076ce 100644 (file)
@@ -3,9 +3,13 @@ Bug #61828 (Memleak when calling Directory(Recursive)Iterator/Spl(Temp)FileObjec
 --FILE--
 <?php
 $x = new DirectoryIterator('.');
-$x->__construct('/tmp');
-echo "Okey";
+
+try {
+    $x->__construct('/tmp');
+} catch (\Error $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+
 ?>
---EXPECTF--
-Warning: DirectoryIterator::__construct(): Directory object is already initialized in %sbug61828.php on line 3
-Okey
+--EXPECT--
+Directory object is already initialized
index bd5a7f06dbfd8ed6bbe6ca2c4feb92e494db8bc8..8ebbf648c97c58ae3274f1d5933e4bb0ada1af4c 100644 (file)
@@ -6,17 +6,19 @@ $obj = new SplFileObject(__FILE__, 'r');
 $data = $obj->fread(5);
 var_dump($data);
 
-$data = $obj->fread(0);
-var_dump($data);
+try {
+    $data = $obj->fread(0);
+    var_dump($data);
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 
 // read more data than is available
 $data = $obj->fread(filesize(__FILE__) + 32);
 var_dump(strlen($data) === filesize(__FILE__) - 5);
 
 ?>
---EXPECTF--
+--EXPECT--
 string(5) "<?php"
-
-Warning: SplFileObject::fread(): Length parameter must be greater than 0 in %s on line %d
-bool(false)
+SplFileObject::fread(): Argument #1 ($length) must be greater than 0
 bool(true)
index babce8a43ed28c657f199d03c82c585e6293543d..bc2e4896b0b58414da160aa1d834b6710daf99ac 100644 (file)
@@ -78,7 +78,7 @@ Exception: MyCachingIterator does not use a full cache (see CachingIterator::__c
 int(0)
 bool(false)
 
-Notice: Undefined array key "0" in %s on line %d
+Warning: Undefined array key "0" in %s on line %d
 NULL
 ===1===
 object(stdClass)#%d (0) {
@@ -90,31 +90,31 @@ object(MyFoo)#%d (0) {
 }
 bool(false)
 
-Notice: Undefined array key "foo" in %s on line %d
+Warning: Undefined array key "foo" in %s on line %d
 NULL
 ===3===
 NULL
 bool(false)
 
-Notice: Undefined array key "" in %s on line %d
+Warning: Undefined array key "" in %s on line %d
 NULL
 ===4===
 int(2)
 bool(false)
 
-Notice: Undefined array key "2" in %s on line %d
+Warning: Undefined array key "2" in %s on line %d
 NULL
 ===5===
 string(3) "foo"
 bool(false)
 
-Notice: Undefined array key "foo" in %s on line %d
+Warning: Undefined array key "foo" in %s on line %d
 NULL
 ===6===
 int(3)
 bool(false)
 
-Notice: Undefined array key "3" in %s on line %d
+Warning: Undefined array key "3" in %s on line %d
 NULL
 ===FILL===
 ===0===
@@ -135,7 +135,7 @@ int(1)
 NULL
 bool(false)
 
-Notice: Undefined array key "" in %s on line %d
+Warning: Undefined array key "" in %s on line %d
 NULL
 ===4===
 int(2)
@@ -149,5 +149,5 @@ int(1)
 int(3)
 bool(false)
 
-Notice: Undefined array key "3" in %s on line %d
+Warning: Undefined array key "3" in %s on line %d
 NULL