Fix bug #80945: Don't throw undefined array key warning in ArrayObject unset()

nikic · nikic · commit 71fb83567f3a · 2021-06-14T10:14:38.000+02:00
This makes the behavior of ArrayObject the same as far plain
arrays, which don't throw a warning when unsetting a key that
already doesn't exit.
diff --git a/NEWS b/NEWS
@@ -13,7 +13,9 @@ PHP                                                                        NEWS
   . Fixed bug #81135 (unknown help topic causes assertion failure). (krakjoe)
 
 - SPL:
-  . Fixed buf #81112 (Special json_encode behavior for SplFixedArray). (Nikita)
+  . Fixed bug #81112 (Special json_encode behavior for SplFixedArray). (Nikita)
+  . Fixed bug #80945 ("Notice: Undefined index" on unset() ArrayObject
+    non-existing key). (Nikita)
 
 10 Jun 2021, PHP 8.1.0alpha1
 
diff --git a/Zend/tests/unset_cv10.phpt b/Zend/tests/unset_cv10.phpt
@@ -10,9 +10,7 @@ $a->offsetUnset('x');
 echo $x;
 echo "ok\n";
 ?>
---EXPECTF--
+--EXPECT--
 ok
-
-Warning: Undefined array key "x" in %s on line %d
 ok
 ok
diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
@@ -554,9 +554,7 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
 		if (data) {
 			if (Z_TYPE_P(data) == IS_INDIRECT) {
 				data = Z_INDIRECT_P(data);
-				if (Z_TYPE_P(data) == IS_UNDEF) {
-					zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
-				} else {
+				if (Z_TYPE_P(data) != IS_UNDEF) {
 					zval_ptr_dtor(data);
 					ZVAL_UNDEF(data);
 					HT_FLAGS(ht) |= HASH_FLAG_HAS_EMPTY_IND;
@@ -565,17 +563,13 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
 						spl_array_skip_protected(intern, ht);
 					}
 				}
-			} else if (zend_hash_del(ht, key.key) == FAILURE) {
-				zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
+			} else {
+				zend_hash_del(ht, key.key);
 			}
-		} else {
-			zend_error(E_WARNING,"Undefined array key \"%s\"", ZSTR_VAL(key.key));
 		}
 		spl_hash_key_release(&key);
 	} else {
-		if (zend_hash_index_del(ht, key.h) == FAILURE) {
-			zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, key.h);
-		}
+		zend_hash_index_del(ht, key.h);
 	}
 } /* }}} */
 
diff --git a/ext/spl/tests/arrayObject___construct_basic4.phpt b/ext/spl/tests/arrayObject___construct_basic4.phpt
@@ -63,8 +63,6 @@ bool(true)
 
 Warning: Undefined array key "prop" in %s on line %d
 
-Warning: Undefined array key "prop" in %s on line %d
-
 Warning: Undefined array key "prop" in %s on line %d
 NULL
 NULL
diff --git a/ext/spl/tests/arrayObject___construct_basic5.phpt b/ext/spl/tests/arrayObject___construct_basic5.phpt
@@ -63,8 +63,6 @@ bool(true)
 
 Warning: Undefined array key "prop" in %s on line %d
 
-Warning: Undefined array key "prop" in %s on line %d
-
 Warning: Undefined array key "prop" in %s on line %d
 NULL
 NULL
diff --git a/ext/spl/tests/arrayObject_magicMethods1.phpt b/ext/spl/tests/arrayObject_magicMethods1.phpt
@@ -170,8 +170,6 @@ object(ArrayObject)#2 (1) {
 }
 
 --> Unset existent, non-existent and dynamic:
-
-Warning: Undefined array key "nonexistent" in %s on line %d
   Original wrapped object:
 object(UsesMagic)#1 (3) {
   ["b"]=>
diff --git a/ext/spl/tests/arrayObject_magicMethods3.phpt b/ext/spl/tests/arrayObject_magicMethods3.phpt
@@ -170,8 +170,6 @@ object(ArrayObject)#2 (1) {
 }
 
 --> Unset existent, non-existent and dynamic:
-
-Warning: Undefined array key "nonexistent" in %s on line %d
   Original wrapped object:
 object(UsesMagic)#1 (3) {
   ["b"]=>
diff --git a/ext/spl/tests/arrayObject_magicMethods4.phpt b/ext/spl/tests/arrayObject_magicMethods4.phpt
@@ -179,8 +179,6 @@ object(UsesMagic)#2 (2) {
 }
 
 --> Unset existent, non-existent and dynamic:
-
-Warning: Undefined array key "nonexistent" in %s on line %d
   Original wrapped object:
 object(C)#1 (3) {
   ["b"]=>
diff --git a/ext/spl/tests/arrayObject_magicMethods6.phpt b/ext/spl/tests/arrayObject_magicMethods6.phpt
@@ -179,8 +179,6 @@ object(UsesMagic)#2 (2) {
 }
 
 --> Unset existent, non-existent and dynamic:
-
-Warning: Undefined array key "nonexistent" in %s on line %d
   Original wrapped object:
 object(C)#1 (3) {
   ["b"]=>
diff --git a/ext/spl/tests/array_001.phpt b/ext/spl/tests/array_001.phpt
@@ -82,10 +82,6 @@ NULL
 
 Warning: Undefined array key "b" in %s on line %d
 NULL
-
-Warning: Undefined array key 7 in %s on line %d
-
-Warning: Undefined array key "c" in %s on line %d
 object(ArrayObject)#%d (1) {
   ["storage":"ArrayObject":private]=>
   array(2) {
diff --git a/ext/spl/tests/array_010.phpt b/ext/spl/tests/array_010.phpt
@@ -125,10 +125,6 @@ array(6) {
   [6]=>
   string(9) "changed 6"
 }
-
-Warning: Undefined array key 7 in %s on line %d
-
-Warning: Undefined array key "8th" in %s on line %d
 array(4) {
   [0]=>
   string(3) "1st"
diff --git a/ext/spl/tests/bug45622b.phpt b/ext/spl/tests/bug45622b.phpt
@@ -28,6 +28,4 @@ Doesn't trigger __get.
 Warning: Undefined array key "prop1" in %s on line %d
 Doesn't trigger __set.
 Doesn't trigger __unset.
-
-Warning: Undefined array key "prop3" in %s on line %d
 Shouldn't trigger __isset.
diff --git a/ext/spl/tests/bug66127.phpt b/ext/spl/tests/bug66127.phpt
@@ -21,6 +21,5 @@ unset($items[0][0]);
 crash();
 echo "Worked!\n";
 ?>
---EXPECTF--
-Warning: Undefined array key 0 in %s on line %d
+--EXPECT--
 Worked!

Original file line number	Diff line number	Diff line change
`@@ -170,8 +170,6 @@ object(ArrayObject)#2 (1) {`
`170`	`170`	`}`
`171`	`171`
`172`	`172`	`--> Unset existent, non-existent and dynamic:`
`173`		`-`
`174`		`-Warning: Undefined array key "nonexistent" in %s on line %d`
`175`	`173`	`Original wrapped object:`
`176`	`174`	`object(UsesMagic)#1 (3) {`
`177`	`175`	`["b"]=>`
Original file line number	Diff line number	Diff line change
`@@ -179,8 +179,6 @@ object(UsesMagic)#2 (2) {`
`179`	`179`	`}`
`180`	`180`
`181`	`181`	`--> Unset existent, non-existent and dynamic:`
`182`		`-`
`183`		`-Warning: Undefined array key "nonexistent" in %s on line %d`
`184`	`182`	`Original wrapped object:`
`185`	`183`	`object(C)#1 (3) {`
`186`	`184`	`["b"]=>`