From 4d609749451abceef51c61c831c2a0494800beca Mon Sep 17 00:00:00 2001 From: rhertogh Date: Wed, 26 Jul 2023 21:46:33 +0200 Subject: [PATCH 1/7] Removed optimization for PHP predating v7.4 --- framework/db/BaseActiveRecord.php | 4 ++-- framework/helpers/BaseArrayHelper.php | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/framework/db/BaseActiveRecord.php b/framework/db/BaseActiveRecord.php index 88fb2f11d7c..f6157d0e0d3 100644 --- a/framework/db/BaseActiveRecord.php +++ b/framework/db/BaseActiveRecord.php @@ -282,7 +282,7 @@ public function canSetProperty($name, $checkVars = true, $checkBehaviors = true) */ public function __get($name) { - if (isset($this->_attributes[$name]) || array_key_exists($name, $this->_attributes)) { + if (array_key_exists($name, $this->_attributes)) { return $this->_attributes[$name]; } @@ -290,7 +290,7 @@ public function __get($name) return null; } - if (isset($this->_related[$name]) || array_key_exists($name, $this->_related)) { + if (array_key_exists($name, $this->_related)) { return $this->_related[$name]; } $value = parent::__get($name); diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index 0e615a579db..462e4f74737 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -327,7 +327,7 @@ public static function setValue(&$array, $path, $value) */ public static function remove(&$array, $key, $default = null) { - if (is_array($array) && (isset($array[$key]) || array_key_exists($key, $array))) { + if (is_array($array) && array_key_exists($key, $array)) { $value = $array[$key]; unset($array[$key]); @@ -616,9 +616,7 @@ public static function map($array, $from, $to, $group = null) public static function keyExists($key, $array, $caseSensitive = true) { if ($caseSensitive) { - // Function `isset` checks key faster but skips `null`, `array_key_exists` handles this case - // https://www.php.net/manual/en/function.array-key-exists.php#107786 - if (is_array($array) && (isset($array[$key]) || array_key_exists($key, $array))) { + if (is_array($array) && array_key_exists($key, $array)) { return true; } // Cannot use `array_has_key` on Objects for PHP 7.4+, therefore we need to check using [[ArrayAccess::offsetExists()]] From 000f16417d3797e00aeb37d67fb43572ebf2faa2 Mon Sep 17 00:00:00 2001 From: rhertogh Date: Wed, 26 Jul 2023 22:35:10 +0200 Subject: [PATCH 2/7] Fixed `BaseArrayHelper::keyExists()` and `BaseArrayHelper::remove()` to deal with float keys (even when their value is `null`) --- framework/helpers/BaseArrayHelper.php | 12 ++++++++-- tests/framework/helpers/ArrayHelperTest.php | 25 +++++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index 462e4f74737..7f166571c20 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -327,6 +327,10 @@ public static function setValue(&$array, $path, $value) */ public static function remove(&$array, $key, $default = null) { + if (is_float($key)) { + $key = (int)$key; + } + if (is_array($array) && array_key_exists($key, $array)) { $value = $array[$key]; unset($array[$key]); @@ -608,15 +612,19 @@ public static function map($array, $from, $to, $group = null) * Checks if the given array contains the specified key. * This method enhances the `array_key_exists()` function by supporting case-insensitive * key comparison. - * @param string $key the key to check + * @param string|int $key the key to check * @param array|ArrayAccess $array the array with keys to check * @param bool $caseSensitive whether the key comparison should be case-sensitive * @return bool whether the array contains the specified key */ public static function keyExists($key, $array, $caseSensitive = true) { + if (is_float($key)) { + $key = (int)$key; + } + if ($caseSensitive) { - if (is_array($array) && array_key_exists($key, $array)) { + if (is_array($array) && (isset($array[$key]) || array_key_exists($key, $array))) { return true; } // Cannot use `array_has_key` on Objects for PHP 7.4+, therefore we need to check using [[ArrayAccess::offsetExists()]] diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 063eeafcaf6..8a654abd2c5 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -125,10 +125,14 @@ public function testToArray() public function testRemove() { - $array = ['name' => 'b', 'age' => 3]; - $name = ArrayHelper::remove($array, 'name'); + $array = ['name' => 'b', 'age' => 3, 1.1 => null]; + $name = ArrayHelper::remove($array, 'name'); $this->assertEquals($name, 'b'); + $this->assertEquals($array, ['age' => 3, 1.1 => null]); + + $floatVal = ArrayHelper::remove($array, 1.1); + $this->assertNull($floatVal); $this->assertEquals($array, ['age' => 3]); $default = ArrayHelper::remove($array, 'nonExisting', 'defaultValue'); @@ -506,14 +510,17 @@ public function testMergeEmpty() /** * @see https://github.com/yiisoft/yii2/pull/11549 */ - public function test() + public function testGetValueWithFloatKeys() { $array = []; $array[1.0] = 'some value'; + $array[2.0] = null; $result = ArrayHelper::getValue($array, 1.0); - $this->assertEquals('some value', $result); + + $result = ArrayHelper::getValue($array, 2.0); + $this->assertNull($result); } public function testIndex() @@ -711,6 +718,9 @@ public function testKeyExists() $array = [ 'a' => 1, 'B' => 2, + 1 => 3, + 2.2 => 4, // Note: Foats are cast to ints, which means that the fractional part will be truncated. + 3.3 => null, ]; $this->assertTrue(ArrayHelper::keyExists('a', $array)); $this->assertFalse(ArrayHelper::keyExists('b', $array)); @@ -721,6 +731,13 @@ public function testKeyExists() $this->assertTrue(ArrayHelper::keyExists('b', $array, false)); $this->assertTrue(ArrayHelper::keyExists('B', $array, false)); $this->assertFalse(ArrayHelper::keyExists('c', $array, false)); + + $this->assertTrue(ArrayHelper::keyExists(1, $array)); + $this->assertTrue(ArrayHelper::keyExists(2, $array)); + $this->assertTrue(ArrayHelper::keyExists('2', $array)); + $this->assertTrue(ArrayHelper::keyExists(2.2, $array)); + $this->assertTrue(ArrayHelper::keyExists(3, $array)); + $this->assertTrue(ArrayHelper::keyExists(3.3, $array)); } public function testKeyExistsArrayAccess() From 9efcd5718d3704c80623a3c36fa6f34dc4fe8b1c Mon Sep 17 00:00:00 2001 From: rhertogh Date: Wed, 26 Jul 2023 22:39:18 +0200 Subject: [PATCH 3/7] Fixed typo in "Floats" --- tests/framework/helpers/ArrayHelperTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 8a654abd2c5..204788b6903 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -719,7 +719,7 @@ public function testKeyExists() 'a' => 1, 'B' => 2, 1 => 3, - 2.2 => 4, // Note: Foats are cast to ints, which means that the fractional part will be truncated. + 2.2 => 4, // Note: Floats are cast to ints, which means that the fractional part will be truncated. 3.3 => null, ]; $this->assertTrue(ArrayHelper::keyExists('a', $array)); From fe0260f51891a01ae7b86456935900330566d309 Mon Sep 17 00:00:00 2001 From: rhertogh Date: Wed, 26 Jul 2023 22:59:00 +0200 Subject: [PATCH 4/7] Skipp ArrayHelper float key tests for PHP >= v8.1 --- tests/framework/helpers/ArrayHelperTest.php | 49 ++++++++++++++++++--- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/tests/framework/helpers/ArrayHelperTest.php b/tests/framework/helpers/ArrayHelperTest.php index 204788b6903..0d706bc552f 100644 --- a/tests/framework/helpers/ArrayHelperTest.php +++ b/tests/framework/helpers/ArrayHelperTest.php @@ -125,6 +125,25 @@ public function testToArray() public function testRemove() { + $array = ['name' => 'b', 'age' => 3]; + $name = ArrayHelper::remove($array, 'name'); + + $this->assertEquals($name, 'b'); + $this->assertEquals($array, ['age' => 3]); + + $default = ArrayHelper::remove($array, 'nonExisting', 'defaultValue'); + $this->assertEquals('defaultValue', $default); + } + + /** + * @return void + */ + public function testRemoveWithFloat() + { + if (version_compare(PHP_VERSION, '8.1.0', '>=')) { + $this->markTestSkipped('Using floats as array key is deprecated.'); + } + $array = ['name' => 'b', 'age' => 3, 1.1 => null]; $name = ArrayHelper::remove($array, 'name'); @@ -512,14 +531,18 @@ public function testMergeEmpty() */ public function testGetValueWithFloatKeys() { + if (version_compare(PHP_VERSION, '8.1.0', '>=')) { + $this->markTestSkipped('Using floats as array key is deprecated.'); + } + $array = []; - $array[1.0] = 'some value'; - $array[2.0] = null; + $array[1.1] = 'some value'; + $array[2.1] = null; - $result = ArrayHelper::getValue($array, 1.0); + $result = ArrayHelper::getValue($array, 1.2); $this->assertEquals('some value', $result); - $result = ArrayHelper::getValue($array, 2.0); + $result = ArrayHelper::getValue($array, 2.2); $this->assertNull($result); } @@ -718,10 +741,8 @@ public function testKeyExists() $array = [ 'a' => 1, 'B' => 2, - 1 => 3, - 2.2 => 4, // Note: Floats are cast to ints, which means that the fractional part will be truncated. - 3.3 => null, ]; + $this->assertTrue(ArrayHelper::keyExists('a', $array)); $this->assertFalse(ArrayHelper::keyExists('b', $array)); $this->assertTrue(ArrayHelper::keyExists('B', $array)); @@ -731,8 +752,22 @@ public function testKeyExists() $this->assertTrue(ArrayHelper::keyExists('b', $array, false)); $this->assertTrue(ArrayHelper::keyExists('B', $array, false)); $this->assertFalse(ArrayHelper::keyExists('c', $array, false)); + } + + public function testKeyExistsWithFloat() + { + if (version_compare(PHP_VERSION, '8.1.0', '>=')) { + $this->markTestSkipped('Using floats as array key is deprecated.'); + } + + $array = [ + 1 => 3, + 2.2 => 4, // Note: Floats are cast to ints, which means that the fractional part will be truncated. + 3.3 => null, + ]; $this->assertTrue(ArrayHelper::keyExists(1, $array)); + $this->assertTrue(ArrayHelper::keyExists(1.1, $array)); $this->assertTrue(ArrayHelper::keyExists(2, $array)); $this->assertTrue(ArrayHelper::keyExists('2', $array)); $this->assertTrue(ArrayHelper::keyExists(2.2, $array)); From a8a56785c8ca438126ad5f077c0a4de1cde2a240 Mon Sep 17 00:00:00 2001 From: rhertogh Date: Wed, 26 Jul 2023 23:16:39 +0200 Subject: [PATCH 5/7] updated changelog for #19914 (Fixed `ArrayHelper::keyExists()` and `::remove()` functions when the key is a float and the value is `null`) --- framework/CHANGELOG.md | 1 + framework/helpers/BaseArrayHelper.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index dc620a07486..a6f07ced9bf 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -14,6 +14,7 @@ Yii Framework 2 Change Log - Bug #19868: Added whitespace sanitation for tests, due to updates in ICU 72 (schmunk42) - Enh #19884: Added support Enums in Query Builder (sk1t0n) - Bug #19906: Fixed multiline strings in the `\yii\console\widgets\Table` widget (rhertogh) +- Bug #19914: Fixed `ArrayHelper::keyExists()` and `::remove()` functions when the key is a float and the value is `null` (rhertogh) 2.0.48.1 May 24, 2023 diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index 7f166571c20..90c3a3b99a5 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -327,6 +327,7 @@ public static function setValue(&$array, $path, $value) */ public static function remove(&$array, $key, $default = null) { + // This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) if (is_float($key)) { $key = (int)$key; } @@ -619,6 +620,7 @@ public static function map($array, $from, $to, $group = null) */ public static function keyExists($key, $array, $caseSensitive = true) { + // This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) if (is_float($key)) { $key = (int)$key; } From e4e6865d2dda6708438870425bac623c924d7184 Mon Sep 17 00:00:00 2001 From: rhertogh Date: Thu, 27 Jul 2023 10:59:02 +0200 Subject: [PATCH 6/7] Removed optimization for PHP predating v7.4 --- framework/helpers/BaseArrayHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index 90c3a3b99a5..e8886dde816 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -626,7 +626,7 @@ public static function keyExists($key, $array, $caseSensitive = true) } if ($caseSensitive) { - if (is_array($array) && (isset($array[$key]) || array_key_exists($key, $array))) { + if (is_array($array) && array_key_exists($key, $array)) { return true; } // Cannot use `array_has_key` on Objects for PHP 7.4+, therefore we need to check using [[ArrayAccess::offsetExists()]] From f1cb41400cb228499f042ad70999b3b461548a0e Mon Sep 17 00:00:00 2001 From: rhertogh Date: Fri, 4 Aug 2023 16:46:20 +0200 Subject: [PATCH 7/7] Marked todo comments in BaseArrayHelper as such --- framework/helpers/BaseArrayHelper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/helpers/BaseArrayHelper.php b/framework/helpers/BaseArrayHelper.php index e8886dde816..56411163e1e 100644 --- a/framework/helpers/BaseArrayHelper.php +++ b/framework/helpers/BaseArrayHelper.php @@ -327,7 +327,7 @@ public static function setValue(&$array, $path, $value) */ public static function remove(&$array, $key, $default = null) { - // This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) + // ToDo: This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) if (is_float($key)) { $key = (int)$key; } @@ -620,7 +620,7 @@ public static function map($array, $from, $to, $group = null) */ public static function keyExists($key, $array, $caseSensitive = true) { - // This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) + // ToDo: This check can be removed when the minimum PHP version is >= 8.1 (Yii2.2) if (is_float($key)) { $key = (int)$key; }