Issue #3490355 by nicxvan, godotislate, catch: Add procedural hook short circuit per module or file

core/lib/Drupal/Core/Hook/Attribute/StopProceduralHookScan.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Core\Hook\Attribute;
+
+/**
+ * Defines a StopProceduralHookScan attribute object.
+ *
+ * This allows contrib and core to mark when a file has no more
+ * procedural hooks.
+ */
+#[\Attribute(\Attribute::TARGET_FUNCTION)]
+class StopProceduralHookScan {
+
+}

core/lib/Drupal/Core/Hook/HookCollectorPass.php

@@ -10,6 +10,7 @@
 use Drupal\Core\Extension\ProceduralCall;
 use Drupal\Core\Hook\Attribute\Hook;
 use Drupal\Core\Hook\Attribute\LegacyHook;
+use Drupal\Core\Hook\Attribute\StopProceduralHookScan;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
@@ -74,7 +75,7 @@ class HookCollectorPass implements CompilerPassInterface {
    * {@inheritdoc}
    */
   public function process(ContainerBuilder $container): void {
-    $collector = static::collectAllHookImplementations($container->getParameter('container.modules'));
+    $collector = static::collectAllHookImplementations($container->getParameter('container.modules'), $container);
     $map = [];
     $container->register(ProceduralCall::class, ProceduralCall::class)
       ->addArgument($collector->includes);
@@ -123,6 +124,8 @@ public function process(ContainerBuilder $container): void {
    * @param array $module_filenames
    *   An associative array. Keys are the module names, values are relevant
    *   info yml file path.
+   * @param Symfony\Component\DependencyInjection\ContainerBuilder|null $container
+   *   The container.
    *
    * @return static
    *   A HookCollectorPass instance holding all hook implementations and
@@ -130,15 +133,18 @@ public function process(ContainerBuilder $container): void {
    *
    * @internal
    *   This method is only used by ModuleHandler.
+   *
+   * * @todo Pass only $container when ModuleHandler->add is removed https://www.drupal.org/project/drupal/issues/3481778
    */
-  public static function collectAllHookImplementations(array $module_filenames): static {
+  public static function collectAllHookImplementations(array $module_filenames, ?ContainerBuilder $container = NULL): static {
     $modules = array_map(fn ($x) => preg_quote($x, '/'), array_keys($module_filenames));
     // Longer modules first.
     usort($modules, fn($a, $b) => strlen($b) - strlen($a));
     $module_preg = '/^(?<function>(?<module>' . implode('|', $modules) . ')_(?!preprocess_)(?!update_\d)(?<hook>[a-zA-Z0-9_\x80-\xff]+$))/';
     $collector = new static();
     foreach ($module_filenames as $module => $info) {
-      $collector->collectModuleHookImplementations(dirname($info['pathname']), $module, $module_preg);
+      $skip_procedural = isset($container) ? $container->hasParameter("$module.hooks_converted") : FALSE;
+      $collector->collectModuleHookImplementations(dirname($info['pathname']), $module, $module_preg, $skip_procedural);
     }
     return $collector;
   }
@@ -153,70 +159,83 @@ public static function collectAllHookImplementations(array $module_filenames): s
    * @param $module_preg
    *   A regular expression matching every module, longer module names are
    *   matched first.
+   * @param $skip_procedural
+   *   Skip the procedural check for the current module.
    *
    * @return void
    */
-  protected function collectModuleHookImplementations($dir, $module, $module_preg): void {
+  protected function collectModuleHookImplementations($dir, $module, $module_preg, bool $skip_procedural): void {
     $hook_file_cache = FileCacheFactory::get('hook_implementations');
     $procedural_hook_file_cache = FileCacheFactory::get('procedural_hook_implementations:' . $module_preg);
 
-    $iterator = new \RecursiveDirectoryIterator($dir, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::UNIX_PATHS | \FilesystemIterator::FOLLOW_SYMLINKS);
-    $iterator = new \RecursiveCallbackFilterIterator($iterator, static::filterIterator(...));
-    $iterator = new \RecursiveIteratorIterator($iterator);
-    /** @var \RecursiveDirectoryIterator | \RecursiveIteratorIterator $iterator*/
-    foreach ($iterator as $fileinfo) {
-      assert($fileinfo instanceof \SplFileInfo);
-      $extension = $fileinfo->getExtension();
-      $filename = $fileinfo->getPathname();
+    // Check only hook classes.
+    if ($skip_procedural) {
+      $dir = $dir . '/src/Hook';
+    }
+
+    if (is_dir($dir)) {
+      $iterator = new \RecursiveDirectoryIterator($dir, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::UNIX_PATHS | \FilesystemIterator::FOLLOW_SYMLINKS);
+      $iterator = new \RecursiveCallbackFilterIterator($iterator, static::filterIterator(...));
+      $iterator = new \RecursiveIteratorIterator($iterator);
+      /** @var \RecursiveDirectoryIterator | \RecursiveIteratorIterator $iterator*/
+      foreach ($iterator as $fileinfo) {
+        assert($fileinfo instanceof \SplFileInfo);
+        $extension = $fileinfo->getExtension();
+        $filename = $fileinfo->getPathname();
 
-      if (($extension === 'module' || $extension === 'profile') && !$iterator->getDepth()) {
-        // There is an expectation for all modules and profiles to be loaded.
-        // .module and .profile files are not supposed to be in subdirectories.
-        include_once $filename;
-      }
-      if ($extension === 'php') {
-        $cached = $hook_file_cache->get($filename);
-        if ($cached) {
-          $class = $cached['class'];
-          $attributes = $cached['attributes'];
+        if (($extension === 'module' || $extension === 'profile') && !$iterator->getDepth() && !$skip_procedural) {
+          // There is an expectation for all modules and profiles to be loaded.
+          // .module and .profile files are not supposed to be in subdirectories.
+
+          include_once $filename;
         }
-        else {
-          $namespace = preg_replace('#^src/#', "Drupal/$module/", $iterator->getSubPath());
-          $class = $namespace . '/' . $fileinfo->getBasename('.php');
-          $class = str_replace('/', '\\', $class);
-          if (class_exists($class)) {
-            $attributes = static::getHookAttributesInClass($class);
-            $hook_file_cache->set($filename, ['class' => $class, 'attributes' => $attributes]);
+        if ($extension === 'php') {
+          $cached = $hook_file_cache->get($filename);
+          if ($cached) {
+            $class = $cached['class'];
+            $attributes = $cached['attributes'];
           }
           else {
-            $attributes = [];
+            $namespace = preg_replace('#^src/#', "Drupal/$module/", $iterator->getSubPath());
+            $class = $namespace . '/' . $fileinfo->getBasename('.php');
+            $class = str_replace('/', '\\', $class);
+            if (class_exists($class)) {
+              $attributes = static::getHookAttributesInClass($class);
+              $hook_file_cache->set($filename, ['class' => $class, 'attributes' => $attributes]);
+            }
+            else {
+              $attributes = [];
+            }
+          }
+          foreach ($attributes as $attribute) {
+            $this->addFromAttribute($attribute, $class, $module);
           }
         }
-        foreach ($attributes as $attribute) {
-          $this->addFromAttribute($attribute, $class, $module);
-        }
-      }
-      else {
-        $implementations = $procedural_hook_file_cache->get($filename);
-        if ($implementations === NULL) {
-          $finder = MockFileFinder::create($filename);
-          $parser = new StaticReflectionParser('', $finder);
-          $implementations = [];
-          foreach ($parser->getMethodAttributes() as $function => $attributes) {
-            if (!StaticReflectionParser::hasAttribute($attributes, LegacyHook::class) && preg_match($module_preg, $function, $matches)) {
-              $implementations[] = ['function' => $function, 'module' => $matches['module'], 'hook' => $matches['hook']];
+        elseif (!$skip_procedural) {
+          $implementations = $procedural_hook_file_cache->get($filename);
+          if ($implementations === NULL) {
+            $finder = MockFileFinder::create($filename);
+            $parser = new StaticReflectionParser('', $finder);
+            $implementations = [];
+            foreach ($parser->getMethodAttributes() as $function => $attributes) {
+              if (StaticReflectionParser::hasAttribute($attributes, StopProceduralHookScan::class)) {
+                break;
+              }
+              if (!StaticReflectionParser::hasAttribute($attributes, LegacyHook::class) && preg_match($module_preg, $function, $matches)) {
+                $implementations[] = ['function' => $function, 'module' => $matches['module'], 'hook' => $matches['hook']];
+              }
             }
+            $procedural_hook_file_cache->set($filename, $implementations);
+          }
+          foreach ($implementations as $implementation) {
+            $this->addProceduralImplementation($fileinfo, $implementation['hook'], $implementation['module'], $implementation['function']);
           }
-          $procedural_hook_file_cache->set($filename, $implementations);
-        }
-        foreach ($implementations as $implementation) {
-          $this->addProceduralImplementation($fileinfo, $implementation['hook'], $implementation['module'], $implementation['function']);
         }
-      }
-      if ($extension === 'inc') {
-        $parts = explode('.', $fileinfo->getFilename());
-        if (count($parts) === 3 && $parts[0] === $module) {
-          $this->groupIncludes[$parts[1]][] = $filename;
+        if ($extension === 'inc') {
+          $parts = explode('.', $fileinfo->getFilename());
+          if (count($parts) === 3 && $parts[0] === $module) {
+            $this->groupIncludes[$parts[1]][] = $filename;
+          }
         }
       }
     }

core/modules/system/tests/modules/hook_collector_skip_procedural/hook_collector_skip_procedural.info.yml

+name: 'Test skipping procedural for whole module'
+type: module
+description: 'Test skipping procedural for whole module.'
+package: Testing
+version: VERSION

core/modules/system/tests/modules/hook_collector_skip_procedural/hook_collector_skip_procedural.module

+<?php
+
+/**
+ * @file
+ * Implement hooks.
+ */
+
+declare(strict_types=1);
+
+/**
+ * This implements a hook but should not be picked up.
+ *
+ * We have set procedural_hooks: skip.
+ */
+function hook_collector_skip_procedural_cache_flush(): void {
+  // Set a global value we can check in test code.
+  $GLOBALS['skip_procedural_all'] = 'skip_procedural_all';
+}

core/modules/system/tests/modules/hook_collector_skip_procedural/hook_collector_skip_procedural.services.yml

+parameters:
+  hook_collector_skip_procedural.hooks_converted: true

core/modules/system/tests/modules/hook_collector_skip_procedural_attribute/hook_collector_skip_procedural_attribute.info.yml

+name: 'Test skipping procedural for part of the module'
+type: module
+description: 'Test skipping procedural for part of the module.'
+package: Testing
+version: VERSION
+procedural_hooks: scan

core/modules/system/tests/modules/hook_collector_skip_procedural_attribute/hook_collector_skip_procedural_attribute.module

+<?php
+
+/**
+ * @file
+ * Implement hooks.
+ */
+
+declare(strict_types=1);
+
+use Drupal\Core\Hook\Attribute\StopProceduralHookScan;
+
+/**
+ * This implements a hook and should be picked up.
+ *
+ * We have set procedural_hooks: scan.
+ */
+function hook_collector_skip_procedural_attribute_cache_flush(): void {
+  // Set a global value we can check in test code.
+  $GLOBALS['procedural_attribute_skip_find'] = 'procedural_attribute_skip_find';
+}
+
+/**
+ * This implements a hook but should not be picked up.
+ *
+ * This attribute should stop all procedural hooks after.
+ * We implement on behalf of other modules so we can pick them up.
+ */
+#[StopProceduralHookScan]
+function hook_collector_on_behalf_procedural_cache_flush(): void {
+  // Set a global value we can check in test code.
+  $GLOBALS['procedural_attribute_skip_has_attribute'] = 'procedural_attribute_skip_has_attribute';
+}
+
+/**
+ * This implements a hook but should not be picked up.
+ *
+ * The attribute above should prevent this from being found.
+ */
+function hook_collector_on_behalf_cache_flush(): void {
+  // Set a global value we can check in test code.
+  $GLOBALS['procedural_attribute_skip_after_attribute'] = 'procedural_attribute_skip_after_attribute';
+}

core/tests/Drupal/KernelTests/Core/Hook/HookCollectorPassTest.php

@@ -101,4 +101,26 @@ public function testHooksImplementedOnBehalfFileCache(): void {
     $this->assertTrue(isset($GLOBALS['on_behalf_procedural']));
   }
 
+  /**
+   * Test procedural hooks for a module are skipped when skip is set..
+   */
+  public function testProceduralHooksSkippedWhenConfigured(): void {
+    $module_installer = $this->container->get('module_installer');
+    $this->assertTrue($module_installer->install(['hook_collector_skip_procedural']));
+    $this->assertTrue($module_installer->install(['hook_collector_on_behalf_procedural']));
+    $this->assertTrue($module_installer->install(['hook_collector_skip_procedural_attribute']));
+    $this->assertTrue($module_installer->install(['hook_collector_on_behalf']));
+    $this->assertFalse(isset($GLOBALS['skip_procedural_all']));
+    $this->assertFalse(isset($GLOBALS['procedural_attribute_skip_has_attribute']));
+    $this->assertFalse(isset($GLOBALS['procedural_attribute_skip_after_attribute']));
+    $this->assertFalse(isset($GLOBALS['procedural_attribute_skip_find']));
+    drupal_flush_all_caches();
+    $this->assertFalse(isset($GLOBALS['skip_procedural_all']));
+    $this->assertFalse(isset($GLOBALS['procedural_attribute_skip_has_attribute']));
+    $this->assertFalse(isset($GLOBALS['procedural_attribute_skip_after_attribute']));
+    // This is the only one that should be found.
+    $this->assertTrue(isset($GLOBALS['procedural_attribute_skip_find']));
+
+  }
+
 }