Resolve #3485896 "Attempt simplification"

Closes #3485896

core/lib/Drupal/Core/Extension/ModuleHandler.php

@@ -80,11 +80,23 @@ class ModuleHandler implements ModuleHandlerInterface {
    *   An array keyed by hook, classname, method and the value is the module.
    * @param array $groupIncludes
    *   An array of .inc files to get helpers from.
+   * @param array<string, list<string>> $orderedExtraTypes
+   *   A multidimensional array of hooks that have been ordered and the
+   *   extra_types they have been ordered against. This is stored separately
+   *   from $hookImplementationsMap to prevent ordering again since this set
+   *   has already been fully ordered in HookCollectorPass.
    *
    * @see \Drupal\Core\DrupalKernel
    * @see \Drupal\Core\CoreServiceProvider
    */
-  public function __construct($root, array $module_list, protected EventDispatcherInterface $eventDispatcher, protected array $hookImplementationsMap, protected array $groupIncludes = []) {
+  public function __construct(
+    $root,
+    array $module_list,
+    protected EventDispatcherInterface $eventDispatcher,
+    protected array $hookImplementationsMap,
+    protected array $groupIncludes = [],
+    protected array $orderedExtraTypes = [],
+  ) {
     $this->root = $root;
     $this->moduleList = [];
     foreach ($module_list as $name => $module) {
@@ -192,7 +204,8 @@ protected function add($type, $name, $path) {
     $filename = file_exists($php_file_path) ? "$name.$type" : NULL;
     $this->moduleList[$name] = new Extension($this->root, $type, $pathname, $filename);
     $this->resetImplementations();
-    $hook_collector = HookCollectorPass::collectAllHookImplementations([$name => ['pathname' => $pathname]]);
+    $paths = [$name => ['pathname' => $pathname]];
+    $hook_collector = HookCollectorPass::collectAllHookImplementations($paths);
     // A module freshly added will not be registered on the container yet.
     // ProceduralCall service does not yet know about it.
     // Note in HookCollectorPass:
@@ -200,7 +213,7 @@ protected function add($type, $name, $path) {
     // Load all includes so the legacy section of invoke can handle hooks in includes.
     $hook_collector->loadAllIncludes();
     // Register procedural implementations.
-    foreach ($hook_collector->getImplementations() as $hook => $moduleImplements) {
+    foreach ($hook_collector->getImplementations($paths) as $hook => $moduleImplements) {
       foreach ($moduleImplements as $module => $classImplements) {
         foreach ($classImplements[ProceduralCall::class] ?? [] as $method) {
           $this->invokeMap[$hook][$module][] = $method;
@@ -427,27 +440,38 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) {
       $this->alterEventListeners[$cid] = [];
       $hook = $type . '_alter';
       $hook_listeners = $this->getHookListeners($hook);
+      $extra_modules = FALSE;
+      $extra_listeners = [];
       if (isset($extra_types)) {
-        // For multiple hooks, we need $modules to contain every module that
-        // implements at least one of them in the correct order.
-        foreach ($extra_types as $extra_type) {
-          foreach ($this->getHookListeners($extra_type . '_alter') as $module => $listeners) {
-            if (isset($hook_listeners[$module])) {
-              $hook_listeners[$module] = array_merge($hook_listeners[$module], $listeners);
-            }
-            else {
-              $hook_listeners[$module] = $listeners;
-              $extra_modules = TRUE;
-            }
+        $extra_hooks = array_map(static fn ($x) => $x . '_alter', $extra_types);
+        // First get the listeners implementing extra hooks.
+        foreach ($extra_hooks as $extra_hook) {
+          $hook_listeners = $this->findListenersForAlter($extra_hook, $hook_listeners, $extra_modules);
+        }
+        // Second, gather implementations ordered together. These are only used
+        // for ordering because the set might contain hooks not included in
+        // this alter() call. \Drupal\Core\Hook\HookPriority::change()
+        // registers the implementations of combined hooks.
+        foreach (array_merge($extra_hooks, [$type . '_alter']) as $extra_hook) {
+          if (isset($this->orderedExtraTypes[$extra_hook])) {
+            $orderedHooks = $this->orderedExtraTypes[$extra_hook];
+            $extra_listeners = $this->findListenersForAlter(implode(':', $orderedHooks));
+            // Remove already ordered hooks.
+            $extra_hooks = array_diff($extra_hooks, $orderedHooks);
           }
         }
       }
-      // If any modules implement one of the extra hooks that do not implement
-      // the primary hook, we need to add them to the $modules array in their
-      // appropriate order.
-      $modules = array_keys($hook_listeners);
-      if (isset($extra_modules)) {
-        $modules = $this->reOrderModulesForAlter($modules, $hook);
+      // If multiple alters were called, but they were already ordered by
+      // ordering attributes then keep that order.
+      if (isset($extra_hooks) && empty($extra_hooks)) {
+        $modules = array_keys(array_intersect_key($extra_listeners, $hook_listeners));
+      }
+      else {
+        // Otherwise, use a legacy ordering mechanism if needed.
+        $modules = array_keys($hook_listeners);
+        if ($extra_modules) {
+          $modules = $this->legacyReOrderModulesForAlter($modules, $hook);
+        }
       }
       foreach ($modules as $module) {
         foreach ($hook_listeners[$module] ?? [] as $listener) {
@@ -472,7 +496,7 @@ public function alter($type, &$data, &$context1 = NULL, &$context2 = NULL) {
    *   The list, potentially reordered and changed by
    *   hook_module_implements_alter().
    */
-  protected function reOrderModulesForAlter(array $modules, string $hook): array {
+  protected function legacyReOrderModulesForAlter(array $modules, string $hook): array {
     // Order by module order first.
     $modules = array_intersect(array_keys($this->moduleList), $modules);
     // Alter expects the module list to be in the keys.
@@ -572,7 +596,36 @@ protected function getHookListeners(string $hook): array {
         }
       }
     }
+
     return $this->invokeMap[$hook] ?? [];
   }
 
+  /**
+   * Helper to get hook listeners when in alter.
+   *
+   * @param string $hook
+   *   The extra hook or combination hook to check for.
+   * @param array<string, list<callable>> $hook_listeners
+   *   Hook listeners for the current hook_alter.
+   * @param bool|null $extra_modules
+   *   Whether there are extra modules to order.
+   *
+   * @return array<string, list<callable>>
+   *   The hook listeners.
+   */
+  public function findListenersForAlter(string $hook, array $hook_listeners = [], ?bool &$extra_modules = NULL): array {
+    foreach ($this->getHookListeners($hook) as $module => $listeners) {
+      if (isset($hook_listeners[$module])) {
+        $hook_listeners[$module] = array_merge($hook_listeners[$module], $listeners);
+      }
+      else {
+        $hook_listeners[$module] = $listeners;
+        // It is used by reference.
+        // @phpcs:ignore DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable
+        $extra_modules = TRUE;
+      }
+    }
+    return $hook_listeners;
+  }
+
 }