Skip to content

[Support] Fix Arr::array to return [] by default #57013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ahmedsaed27 wants to merge 6 commits into from
Closed

[Support] Fix Arr::array to return [] by default #57013

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
51f3af6
[Support] Fix Arr::array to return [] by default
ahmedsaed27 Sep 11, 2025
405a06a
[Support] Add throwOnNotFound flag to Arr::array
ahmedsaed27 Sep 11, 2025
3925878
[Style] Format Arr and SupportArrTest with Pint
ahmedsaed27 Sep 11, 2025
65b677c
[Style] Format SupportArrTest with Pint
ahmedsaed27 Sep 11, 2025
8ddfe27
[Style] Fix anonymous class spacing to satisfy StyleCI
ahmedsaed27 Sep 11, 2025
2ca9186
[Support] Remove double check `$value === $default` in Arr::array
ahmedsaed27 Sep 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/Illuminate/Collections/Arr.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,26 @@ public static function add($array, $key, $value)

/**
* Get an array item from an array using "dot" notation.
*
* @param \ArrayAccess|array $array
* @param string|int|null $key
* @param array|null $default
* @param bool $throwOnNotFound
* @return array
*
* @throws \InvalidArgumentException
* @throws \Illuminate\Support\ItemNotFoundException
*/
public static function array(ArrayAccess|array $array, string|int|null $key, ?array $default = []): array
public static function array(ArrayAccess|array $array, string|int|null $key, array $default = [], bool $throwOnNotFound = false): array
{
$value = Arr::get($array, $key, $default);

if ($throwOnNotFound && $value === $default && ! Arr::has($array, $key)) {
Copy link
Contributor

@shaedrich shaedrich Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't !Arr::has($array, $key) work without $value === $default or am I missing something here?

Copy link
Author

@ahmedsaed27 ahmedsaed27 Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

! Arr::has($array, $key) on its own can’t distinguish between key is missing and key exists but its value equals the default.

For example:

$data = ['roles' => []];
// Key exists, value is [], same as default
Arr::array($data, 'roles', [], true);

If we only relied on ! Arr::has(), this would incorrectly throw. That’s why the check combines both:

$value === $default → catch when get() gave us the default

! Arr::has($array, $key) → confirm it’s truly missing, not just equal.

This way existing keys are respected, and only missing ones trigger ItemNotFoundException.

Copy link
Contributor

@shaedrich shaedrich Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? 🤨

framework/src/Illuminate/Collections/Arr.php

Lines 214 to 236 in 7b5e185

/**
* Determine if the given key exists in the provided array.
*
* @param \ArrayAccess|array $array
* @param string|int|float $key
* @return bool
*/
public static function exists($array, $key)
{
if ($array instanceof Enumerable) {
return $array->has($key);
}
if ($array instanceof ArrayAccess) {
return $array->offsetExists($key);
}
if (is_float($key)) {
$key = (string) $key;
}
return array_key_exists($key, $array);
}

Copy link
Author

@ahmedsaed27 ahmedsaed27 Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you’re right that Arr::exists() (and therefore Arr::has()) will correctly detect the presence of a key, even if its value is null or [].

The subtlety is that Arr::get() falls back to the default when the key is missing, so without the $value === $default guard, we can’t tell whether the returned value actually came from the array or from the default.

Example:

$data = ['roles' => []];

// Both `Arr::get($data, 'roles', [])` and `Arr::get([], 'roles', [])` return []

Here exists() will return true in the first case and false in the second — so combining ! Arr::has($array, $key) with $value === $default lets us safely tell the difference between key truly missing and key exists with default-like value.

Copy link
Author

@ahmedsaed27 ahmedsaed27 Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? 🤨

framework/src/Illuminate/Collections/Arr.php

Lines 214 to 236 in 7b5e185

/**
* Determine if the given key exists in the provided array.
*
* @param \ArrayAccess|array $array
* @param string|int|float $key
* @return bool
*/
public static function exists($array, $key)
{
if ($array instanceof Enumerable) {
return $array->has($key);
}
if ($array instanceof ArrayAccess) {
return $array->offsetExists($key);
}
if (is_float($key)) {
$key = (string) $key;
}
return array_key_exists($key, $array);
}

value === $default was mainly there as an extra safeguard — kind of a double-check to ensure we only throw when the key is truly missing.
In practice Arr::has() already guarantees that distinction, so it’s not strictly needed, but that was the reasoning behind having it in.

Copy link
Contributor

@shaedrich shaedrich Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you’re right that Arr::exists() (and therefore Arr::has()) will correctly detect the presence of a key, even if its value is null or [].

The subtlety is that Arr::get() falls back to the default when the key is missing, so without the $value === $default guard, we can’t tell whether the returned value actually came from the array or from the default.

Example:

$data = ['roles' => []];

// Both `Arr::get($data, 'roles', [])` and `Arr::get([], 'roles', [])` return []

Here exists() will return true in the first case and false in the second — so combining ! Arr::has($array, $key) with $value === $default lets us safely tell the difference between key truly missing and key exists with default-like value.

Is this written by AI?

Copy link
Author

@ahmedsaed27 ahmedsaed27 Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no I was just trying to explain my perspective with an example. I thought it might help to catch any potential edge cases and thats why I initially left it in as a kind of double check.

Copy link
Contributor

@shaedrich shaedrich Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks for the explanation 👍🏻

throw new ItemNotFoundException(
sprintf('Array key [%s] not found.', $key)
);
}

if (! is_array($value)) {
throw new InvalidArgumentException(
sprintf('Array value for key [%s] must be an array, %s found.', $key, gettype($value))
Expand Down
53 changes: 53 additions & 0 deletions tests/Support/SupportArrTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,59 @@ public function testItReturnsEmptyArrayForMissingKeyByDefault()
$this->assertSame([], Arr::array($data, 'missing_key'));
}

public function test_it_throws_item_not_found_exception_when_array_is_empty_and_throw_on_not_found_is_true()
{
$this->expectException(ItemNotFoundException::class);
$this->expectExceptionMessage('Array key [roles] not found.');

$data = [];

Arr::array($data, 'roles', [], true);
}

public function test_it_throws_item_not_found_exception_when_key_is_missing_and_throw_on_not_found_is_true()
{
$this->expectException(ItemNotFoundException::class);
$this->expectExceptionMessage('Array key [roles] not found.');

$data = ['name' => 'Taylor'];

Arr::array($data, 'roles', [], true);
}

public function test_it_supports_nested_keys_with_dot_notation()
{
$data = ['user' => ['roles' => ['admin']]];

$result = Arr::array($data, 'user.roles');

$this->assertSame(['admin'], $result);
}

public function test_it_throws_for_nested_non_array_value()
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessageMatches(
'#^Array value for key \[user.name\] must be an array, string found.#'
);

$data = ['user' => ['name' => 'Taylor']];

Arr::array($data, 'user.name');
}

public function test_it_should_not_throw_when_key_exists_even_if_equal_to_default()
{
$data = ['roles' => []];

// In the current (buggy) version without Arr::has(),
// this will wrongly throw ItemNotFoundException
// because $value === $default ([] === []).
$result = Arr::array($data, 'roles', [], true);

$this->assertSame([], $result); // expected to return the actual empty array
}

public function testHas()
{
$array = ['products.desk' => ['price' => 100]];
Expand Down