Skip to content

Conversation

@jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 21, 2025

Pull Request Type

  • I have checked there is no other PR open for the same change.

Detailed Description

  1. PR PHP 8.5 | Fix various "Using null as an array offset" deprecation notices #956 added some defensive coding to the offset*() methods in the CaseInsensitiveDictionary class, but was a little overzealous by also adding the defensive coding in the offsetSet() method which already would throw an exception and didn't need the extra defensive coding. This has been cleaned up now.

  2. When running the tests on PHP 8.5, two more test-only deprecation notices show up, though they don't fail the tests due to the deprecation being hit in a class constant definition, i.e. when reading the class:

Deprecated: Using null as an array offset is deprecated, use an empty string instead in path/to/Requests/tests/Utility/CaseInsensitiveDictionary/ArrayAccessTest.php on line 24

Deprecated: Using null as an array offset is deprecated, use an empty string instead in path/to/Requests/tests/Utility/CaseInsensitiveDictionary/GetAllTest.php on line 19

In both cases, the downlow of it is that PHP would already convert the null key in the class constant to an empty string at definition of the array, so whether the array is declared with a null key or a '' (empty string) key doesn't make any actual difference for the tests as by the time the data would hit the test code, the key would already be converted to an empty string.

With that in mind, I've taken the decision to make this empty string key explicit.

This doesn't diminish the value of the test and the guard code for the offset*() methods receiving a null $key is still being tested via the ArrayAccessTest::testOffsetSetWithoutKey() test and the ArrayAccessTest::testAccessValidEntries() with the "Null key will be converted to empty string" data set which is still in place as the data for that test is created via the DATASET_REVERSED constant (instead of the DATASET constant). This was previously designed this way explicitly to allow for testing the null offset case.

1. PR 956 added some defensive coding to the `offset*()` methods in the `CaseInsensitiveDictionary` class, but was a little overzealous by also adding the defensive coding in the `offsetSet()` method which already would throw an exception and didn't need the extra defensive coding.
This has been cleaned up now.

2. When running the tests on PHP 8.5, two more test-only deprecation notices show up, though they don't fail the tests due to the deprecation being hit in a class constant definition, i.e. when reading the class:
```
Deprecated: Using null as an array offset is deprecated, use an empty string instead in path/to/Requests/tests/Utility/CaseInsensitiveDictionary/ArrayAccessTest.php on line 24

Deprecated: Using null as an array offset is deprecated, use an empty string instead in path/to/Requests/tests/Utility/CaseInsensitiveDictionary/GetAllTest.php on line 19
```

In both cases, the downlow of it is that PHP would _already_ convert the `null` key in the class constant to an empty string _at definition of the array_, so whether the array is declared with a `null` key or a `''` (empty string) key doesn't make any actual difference for the tests as by the time the data would hit the test code, the key would already be converted to an empty string.

With that in mind, I've taken the decision to make this empty string key explicit.

This doesn't diminish the value of the test and the guard code for the `offset*()` methods receiving a `null` `$key` is still being tested via the `ArrayAccessTest::testOffsetSetWithoutKey()` test and the `ArrayAccessTest::testAccessValidEntries()` with the "Null key will be converted to empty string" data set which is still in place as the data for that test is created via the `DATASET_REVERSED` constant (instead of the `DATASET` constant). This was previously designed this way explicitly to allow for testing the `null` offset case.
@jrfnl jrfnl added this to the 2.0.16 milestone Nov 21, 2025
@schlessera schlessera merged commit 6927e08 into develop Nov 21, 2025
31 of 34 checks passed
@schlessera schlessera deleted the feature/php-8.5-fix-caseinsensitive-dictionary-tests branch November 21, 2025 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants