[meta] Clean up hook classes in core

In 📌 Split File hooks into separate classes Active I split out hooks based on common dependencies, and kept those with no dependencies in the FileHooks class.

Certain hook groups should be in a class together.

I would force using single class hooks wherever it is possible. There very few use cases when hooks are highly related to each other.

Cross posted from 📌 OOP hooks using event dispatcher Needs review . The "implements" seems redundant now.

/**
 * Implements hook_help().
 */
#[Hook('help')]
Do you think that "implements" note is redundant for this kind of hooks?

It gets worse in single hook classes.

/**
 * Implements hook_help().
 */
#[Hook('help')]
final class Help {

  /**
   * Implements hook_help().
   */
  public function __invoke(): void {

  }

}

Thank you for commenting.

Re 6 I think there are a bunch, especially if you have a shared helper method.

For 7, I agree, but what would you suggest for comments then? A description of what it is doing?

@nicxvan we probably need to understand why "Implements hook_*" comment was initially added to Drupal coding standars. I guess it is needed to distinguish ordinary PHP functions from functions that implement Drupal hooks. Obviously with the Hook attribute is no longer needed. Also api.drupal.org might use this notation when listing hook implementatations.

For me it seems that in most cases docblock in hooks is not needed at all.

This issue is very similar to #3376518: Allow omitting @var, @param and @return tags or their descriptions if the variable name and type is enough → . Maybe we should take same approach once it is resolved.

It's so the api project can find them and document implementations, there is an issue to replace that.

I do think that it is useful to split hooks into common groups and IMHO, it's pretty often that multiple hooks are related, for example modules might implement multiple entity CRUD or access hooks. Some hooks like tokens by design come in a pair.

In the original issue, I proposed to split by hook group, which translates fairly well into the first part of a given hook.

From the meta issue to convert core:

I think the main topic where I'd love to see some improvements is class grouping. I can see that it is capable for splitting it up into classes, but currently it's 1:1 by file. I don't know how complicated that is, especially since the code isn't in order, but I think it would be great if we could from the start adopt some consistent patterns that would make it much easier to see at a glance what kind of hooks a module implements and where they are, like plugins. This is very unlikely to happen later if we don't do it from the start I think. At first, I thought we could have a few specific rules, like FormHooks, and EntityHooks. But what if we just generalize that into prefixing it based on the first word of the hook name? This won't work so great for modules that consist of multiple parts, like hooks for content_moderation and content_translation would both go into ContentHooks. Not perfect, but also not completely wrong?

User module is currently mostly UserHooks, with 18 functions and 370 lines of code. With my suggestion, we'd have:

HelpHooks
ThemeHooks
JsHooks
EntityHooks
UserHooks
TemplateHooks
MailHooks
ElementHooks
ToolbarHooks
FormHooks
FilterHooks

(+ the already created UserTokensHooks and 2 views classes)

I think one of the biggest advantages of the plugin system is that the grouping/namespacing allows you to very quickly get an overview of what kind of plugins a module provides. Using such a grouping of Hooks would IMHO have a similar benefit. Doing single hook classes means we have a lot more classes, to the point that scanning them isn't quite as efficient anymore. We can still say that standalone hooks that we don't expect to have multiple like cron could be a class with a single __invoke() method.

I agree, it's also useful to have less code loading for hooks when invoked too.

It would be good to open a new coding standards issue for removing the Implements doc - I agree that looks redundant with the hook attribute.