Refactor array_merge() usage in loops as possible for performance

Thanks for proposing this and getting the patch started!

Interesting. A new language feature. Slick! ;)

However, the '...' array "spread" operator only exists in PHP 7.4. We'll have to wait for a version of core where that's the minimum. So far, 9.1.x still allows PHP 7.3. Might have to bump this to 10.0.x, TBD.

Meanwhile, some questions / concerns about how this should work once we can use it:

1. +++ b/core/lib/Drupal/Core/Authentication/AuthenticationCollector.php
   @@ -72,10 +72,7 @@ public function getSortedProviders() {
   +      $this->sortedProviders = array_merge([], ...$this->providerOrders);
   

   If I'm reading the spec correctly, I believe this could just be:

     $this->sortedProviders = [...$this->providerOrders];
   

   No?

2. +++ b/core/lib/Drupal/Core/Config/ConfigManager.php
   @@ -249,9 +249,9 @@ public function findConfigEntityDependents($type, array $names, ConfigDependency
   -    return $dependencies;
   +    return array_merge([], ...$dependencies);
   

   Why bother? Why not still return $dependencies and just change what we're doing in the loop?

3. +++ b/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php
   @@ -58,8 +58,9 @@ public function checkConfigSchema(TypedConfigManagerInterface $typed_config, $co
   +    $errors = array_merge([], ...$errors);
   

   Seems like a no-op. Don't you end up with $errors the same before an after this line? How/why are they different?

4. +++ b/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php
   @@ -122,11 +123,12 @@ protected function checkValue($key, $value) {
   +      return array_merge($errors, ...$nested_errors);
   

   I don't think we nee this, but if we did, I believe it would be:

   return [...$errors, ...$nested_errors];

   But again, I'd rather we just did:

   $errors[] = %this->checkValue(...);

   and then keep return $errors;

5. +++ b/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php
   @@ -340,12 +340,14 @@ public function fieldAccess($operation, FieldDefinitionInterface $field_definiti
   -    $grants = [':default' => $default];
   ...
   +      $grants[] = [$module => $this->moduleHandler()->invoke($module, 'entity_field_access', [$operation, $field_definition, $account, $items])];
   

   These two lines should be enough for this whole hunk.

6. +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.php
   @@ -492,8 +492,9 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) {
   +        $extra_modules = array_merge([], ...$extra_modules);
   

   no-op?

There's lots more of the above in the rest of the patch, but I'll stop here and see what's up. ;)

Thanks!
-Derek

Actually, seems that a lot of this could be refactored now without ...$foo at all. E.g.:

  errors = [];
  foreach ($something as $whatever) { 
    $errors[] = $this->checkValue($whatever...);
  }
  return $errors;

We definitely don't need to array_merge() each time inside the loop, but we don't need ...$ to replace it, either.

hi
patch #11 applied successfully
drupal 9.3.x-dev & php 7.3
This seems removed all possible "array_merge" from inside the loop.
thankyou.

The code is certainly prettier in many cases. Nice work.

Committed and pushed cbcdb8b2d42 to 10.0.x and 3cf6d577f81 to 9.4.x. Thanks!

We should get a follow-up for Drupal 10. Since the minim version of PHP there is 8... we change
return array_merge([], ...$dependencies);
TO
return array_merge(...$dependencies);

@Akhildev.cs please don't post screenshots of code being applied. DrupalCI tells us this already so it looks like a comment purely make to receive issue credit.