[9.x] Adding dot notation support to Arr::only helper

[godismyjudge95]

This pull request adds support for dot notation which is supported elsewhere in the framework but currently is not supported via the Arr::only() helper.

Example:

$array = ['taxonomies' => ['colors' => ['red', 'blue'], 'positions' => ['top', 'bottom']]];

$newArray = Arr::only($array, 'taxonomies.colors');

$newArray would then be equal to:

['taxonomies' => ['positions' => ['top', 'bottom']]]

This also works with an array of dot notion keys:

$newArray = Arr::only($array, ['taxonomies.colors', 'taxonomies.positions'])

---

A few notes:

• I have added new test assertions that should cover all new cases of using Arr::only()
• There are a few random changes that had to be made to the test suite where the tests were depending on the order of the array output of Arr::only(). As the array order does not actually matter in any of these cases, I changed the tests to match the new order so they would pass.
• This should not cause any breaking changes as the output stays the same regardless of what is passed in for the $keys parameter (ie. a string, dot notation string, array of normal keys, or array of dot notation keys)

[Tofandel]

@godismyjudge95 I really'd like to have dot notation for Arr::only but your assumption that the order doesn't matter or that it's not a breaking change is wrong

If you had to change existing tests it definitely is a breaking change because it will also cause behavior change or failing test for people relying on Arr::only in their apps

Order change could have some unexpected issues as well if you were for example using the values in their respective order but I do agree, it's rare to have order matter when using keyed arrays, but not impossible, if you want to have it in 9.X you need to figure out a way to preserve the order (I see different options, either a recursive iterator on the original array that might be much less efficient, or an array_flip to get the index of where it was found and then an array_sort)

echo join(' ', Arr::only(['I' => 'He', 'like' => 'loves', 'hates' => 'dislikes', 'mars' => 'twix'], ['mars', 'I', 'like']));
// Currently: He loves twix
// With your version: twix He loves

echo ['id' => 1, 'secret' => 'shhh'] === Arr::only(['id' => 1, 'secret' => 'shhh', 'foo' => 'bar'], ['secret', 'id']);
// Now 1 vs after 0

The fact that you had to change a database DSN test illustrates this very well, with your change the DSN could very well be invalid now

Your other option would be to add a param like $preserveOrder defaulting to true, when it's true you keep the old logic, when it's false you use the new logic

[godismyjudge95]

@Tofandel ah ok. I didn't realize DSN was order sensitive. I will look into how difficult/complicated it is to keep the order. Was trying to keep the code as simple as possible but like you pointed out there are a few edge cases where array order matters. I have just never assumed a string keyed array would be in the order I expect so I thought this would be fine.

Thanks for the review.

[Tofandel]

No problem

It will be quite hard to keep the order, your other bet is to send it to the master branch so that it is included in 10.X as a breaking change

Here is my best demonstration

$url = join('', Arr::only(['protocol' => 'https://', 'domain' => 'example.com', 'port' => '8000', 'path' => '/foo'], ['domain', 'protocol']);
// https://example.com
// vs
// example.comhttps://

Obviously we could even use the fact that now it's ordered based on the keys we passed, and it makes it more predictable, performant and useful, and I prefer that, but yes sadly a breaking change

$url = join('', Arr::only(['domain' => 'example.com', 'protocol' => 'https://', 'port' => '8000', 'path' => '/foo'], ['protocol', 'domain']);
// example.comhttps://
// vs
// https://example.com

[godismyjudge95]

@Tofandel Good news! I figured out a way to keep the order, have dot notation, not break any existing tests/code, and along the way found/fixed a bug where the key had a dot in it.

The pull request now only has changes to the Arr::only method and the related tests in SupportArrTest

[Tofandel]

Nice, I'm reviewing it, and I noticed that yes you don't need to preserve the order of nested keys because that would be part of the new feature anyways that makes it easier to implement

[godismyjudge95]

array_fill_keys(array_keys) + array_replace would have killed the memory for a large array by keeping 3 copies of the array in memory at the same time, it goes from a O(2n+2y) implementation to a O(2y) as well, so we don't depend on the size of the original array anymore and only of the keys

Wow, yeah didn't think about time complexity and memory usage on this - probably should get in the habit of doing so 😄

You should also use static and not Arr like in the rest of the methods

I know... haha this was left over from me experimenting on the laravel playground to get it to work.

Thanks for reviewing this btw, it really helps me learn a lot.

[godismyjudge95]

Also apologies for all the commits (if you get notifications for them), not familiar with the Laravel formatting styles.

[Tofandel]

Ha no worries, I always have the same issue with the styling, will be squashed anyways. Would be good to have an autocorrecting github action like I have seen on some spatie repos

Looks all good to me

[taylorotwell]

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

[godismyjudge95]

@taylorotwell Almost all the other Arr helper methods support dot notation so it is very confusing when Arr::only does not support it. This request is more about being consistent in what the Arr helper methods support than adding a new feature.

[Tofandel]

I also agree with this, almost all array helpers support the dot notation, it is a bit confusing when Arr::only doesn't

This is a good PR, with a useful feature which does it in very little code 🙏

[hafezdivandari]

This also can be used on Arr:pull I opened a discussion for this: #43763