Skip to content

fix(Helpers): handle zero key and non-array intermediate values in ArrayHelper::dotKeyExists() - #10757

Open
banghamid wants to merge 2 commits into
codeigniter4:developfrom
banghamid:fix/array-helper-dot-key-exists
Open

banghamid wants to merge 2 commits into
codeigniter4:developfrom
banghamid:fix/array-helper-dot-key-exists

Conversation

@banghamid

@banghamid banghamid commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Description
ArrayHelper::dotKeyExists() previously had two bugs when traversing dot-notation paths:

  1. Early loop termination on segment '0': while ($currentIndex = array_shift($indexes)) evaluated '0' as falsy and immediately exited the loop, returning true without checking whether '0' or any subsequent segments existed in the array (e.g., ArrayHelper::dotKeyExists('0', []) and ArrayHelper::dotKeyExists('contacts.friends.0.nonexistent', $array) both returned true).
  2. Unhandled TypeError on non-array intermediate values: When an intermediate segment resolved to a scalar or null (e.g., contacts.friends.1.age.sub where age is an integer), calling array_key_exists() on the next segment threw a TypeError.

This PR:

  • Uses while (($currentIndex = array_shift($indexes)) !== null) so '0' segments are properly traversed.
  • Verifies is_array() on $currentArray and wildcard $item elements before calling array_key_exists().
  • Adds regression assertions in ArrayHelperDotKeyExistsTest.
  • Adds a changelog entry in \docs/changelogs/v4.7.7.md.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@carson-codeigniter4 carson-codeigniter4 Bot added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 10, 2026
@gr8man

gr8man commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

Thank you @banghamid for this PR! The fix is clean, accurate, and addresses two very real issues: premature loop termination due to '0' being falsy in PHP, and fatal TypeErrors when encountering scalar intermediate segments.

The logic and test coverage look very solid. I have just a couple of minor suggestions before this is ready to merge:

1. Test coverage for ! is_array($item)

In ArrayHelper.php:158, you added:

if (! is_array($item) || ! array_key_exists($currentIndex, $item)) {

Currently, all items traversed in the wildcard test datasets are arrays, so the ! is_array($item) branch is not covered. It would be great to add a regression assertion for non-array items within a wildcard search, for example in testDotKeyExistsWithWildCard:

$this->assertFalse(ArrayHelper::dotKeyExists('*.name', [['name' => 'Fred'], 'not-an-array']));

2. Positive assertions for '0' and null values (Optional)

The new assertions for '0' are currently all negative (assertFalse). Adding a couple of positive assertions would ensure full regression coverage:

$this->assertTrue(ArrayHelper::dotKeyExists('0', ['0' => 'value']));
$this->assertTrue(ArrayHelper::dotKeyExists('0', ['0' => null]));

3. Changelog

Per CI4 contributing guidelines, please add a brief note under ## Bugs Fixed in docs/changelogs/v4.7.7.md

Other than these minor points, the fix looks great!

@banghamid

Copy link
Copy Markdown
Contributor Author

Thanks for taking a look @gr8man!

Good catch on the wildcard non-array item branch not being covered yet. I pushed an update to address all three points:

  • Added the wildcard test case with a non-array item ('*.name' against [['name' => 'Fred'], 'not-an-array']) in testDotKeyExistsWithWildCard().
  • Added the positive assertions for '0' keys (including a null value) in testDotKeyExists().
  • Added a changelog entry under ## Bugs Fixed in docs/changelogs/v4.7.7.md.

Let me know if anything else is needed!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants