Deprecate hook_hook_info groups, mark hook_hook_info() for deletion

lazy-loading legacy hook implementation code

Please have a look at views_hook_info().

None of that is legacy. Until something like #1972304: Add a HookEvent or #2043653: Allow modules to implement hooks by nominating methods in a module.implements.yml goes in, I don't see how or why we should move forward here.

#1443308-97: Add static cache to module_load_include() shows that views_hook_info() is a net negative for performance. With an opcode cache, having the hook implementations in the module file has a negligible impact, vs. the discovery which is non-negligible.

Also we can't nuke this until 9.x unfortunately, but we can discourage it before then.

I like the idea of killing it, although it would be nice for some of the code organization benefits that it provides to be retained (and in fact expanded).

But I guess that can still be done with require_once statements at the top of the .module file...

Should we issue a deprecation notice?

deprecated in Drupal 8.0.x, will be removed in Drupal 9.0.x

@dpi That sounds good but 8.3.x or 8.4.x would be the start were it was deprecated.

We're trying to figure out how to @trigger_error() for deprecated hooks here: #2870058: [policy, no patch] Document how to deprecate hooks

Policy accepted https://www.drupal.org/core/deprecation#how-hook
Still no replacement for the hook

I don't understand why this is being suggested.

If we replace hooks completely, fine. But we are not there yet I think.
Until then, something like hook_hook_info() is useful for code organization, similar to autoloading for classes (not autoloading itself, but the PSR-4 file organization that is made necessary by autoloading).

views_hook_info() is a net negative for performance. With an opcode cache, having the hook implementations in the module file has a negligible impact, vs. the discovery which is non-negligible.

To me, performance is not the main reason for not putting everything into the main *.module file.

I like the idea of killing it, although it would be nice for some of the code organization benefits that it provides to be retained (and in fact expanded).

Yes. Code organization +1.

But I guess that can still be done with require_once statements at the top of the .module file...

Yes, but:

• no longer forces a convention. Instead, you can now put your hook implementations into whichever file you want. Conventions are good. Forced conventions are good.
• requires explicit require_once statements, whereas with a hook_hook_info() you can simply create the file with the hook implementation. Such lists of require_once are a typical source of merge conflicts - easily resolved, but still annoying.

The issue summary contains some more arguments, which are quite old.
I don't know if these are still valid.

As long as we have hooks, it would be useful to be able to force a convention for file organization per hook.
I am not married to hook_hook_info() specifically, but we should find more convincing reasons to remove it, and/or suggest a good replacement.

Removed field_hook_info() from IS as it was moved to views_hook_info() in #2399931: Generic entity api field handler should live in views module not in field module

So only tokens and views using this hook but it needs to deprecate \Drupal\Core\Extension\ModuleHandlerInterface::getHookInfo() as well

Filed follow-up to remove broken ref #3346614: Remove stray reference to field_hook_info()

Closed as duplicates

- #1373884: [meta] Core does not use hook_hook_info()
- #1107528: Stop using magic functions and force modules to declare the hooks they want to implement
- #1029158: Let hook_hook_info() handle hooks in .install files (aka "rename MODULE.install to MODULE.install.inc")

If we replace hooks completely, fine. But we are not there yet I think.
Until then, something like hook_hook_info() is useful for code organization, similar to autoloading for classes (not autoloading itself, but the PSR-4 file organization that is made necessary by autoloading).

Just to answer #19 - a lot of modules and some of core too already puts the logic of their hook implementations into classes, so that the hook implementation in .module is just three lines or so. This pattern is available for every hook, not just the ones that have hook_hook_info() implemented for them.

Definitely no reason to keep this now that #3442009: OOP hooks using attributes and event dispatcher landed.

But doing so is tricky and needs two layers. We can't just remove hook_hook_info() implementations, because that would immediately break hooks that live in those files. First we need to deprecate hook implementations that live in those files. I don't think we need to do that one-one and introduce a deprecated flag on the hook info, we can do it for everything. But to avoid moving the token hook implementations twice, we probably want to convert those in core to services first.

I wonder if we want to roll this into #3481555: [Plan] Determine how to deprecate procedural hooks..

First deprecate procedural hooks that don't have the @LegacyHook attribute.

This could still leave @LegacyHook hook implementation in hook_hook_info() files.

But then when we're ready to drop Drupal 10 support, we can deprecate @LegacyHook too. And at the same time, we can deprecate hook_hook_info().

Actually #3442009: OOP hooks using attributes and event dispatcher removed this functionality. That turned out to be a mistake.

I can already see the future thanks to test issue where nicxvan is testing converting everything everywhere all at once (or, in short, the Oscar patch). The views hooks can be converted without a problem and almost all *.views.inc becomes empty and is deleted. However, these includes not only contain hooks but helpers as well.: datetime_type_field_views_data_helper, views_entity_field_label, views_field_default_views_data

So for now, #3484105: Automatically included .inc files are no longer included restores this functionality. These three helpers will need to be deprecated manually before hook_hook_info can be laid to rest.

Also we'll have to deprecate this but keep the system and views one around since contrib uses those so much.

The views hook classes support both types, but contrib will have to move those LegacyHook calls into the .module files at the least.

Per #31, we can't really deprecate this, not like we usually do.

> Also we'll have to deprecate this but keep the system and views one around since contrib uses those so much.

This doesn't just apply to system and views. It applies to all implementations. We can't tell modules to stop implementing this hook, because other modules that implement their hooks might still be in those files.

I think we should add a mention to hook_hook_info() docblock that this should no longer be implemented but existing implementations must be kept for BC.

And then we can just drop it in D12 (or whenever we remove the old hooks), no deprecation needed, it won't do anything anymore. It doesn't need to be converted, so no deprecation needed.

So IMHO, this task is actually not blocked on anything and it's just a documentation issue.

@berdir had a discussion in slack here about this: https://drupal.slack.com/archives/C079NQPQUEN/p1732497552707689

I copied my thoughts to the IS.

His approach is slightly different than mine, but I think they mesh well and will cause the least disruption.

The main issue with hook_hook_info is this:

If we deprecate it then modules will remove them.
Any modules that used the include files will no longer be autoloaded and broken, but they got no notice it was deprecated.

@berdir proposes that we update the documentation that it will be removed in 12.
When it's removed we'll have deprecated procedural hooks and modules will have already moved theirs hooks except hook hook info.
Modules that support 10, 11 and 12 can keep the hook hook info function since it provides the inc files (which presumably only have legacy hook implementations) This will break D12 sites that still have helpers in the .inc files.

The approach I propose in case the summary is updated.
We do what @berdir proposed, but also in 11.2 we deprecate the include files themselves.
This will force any modules that utilize for example mymodule.tokens.inc to move all of the functions out of the .inc, whether it's a hook or a helper.
Then when hook hook info is removed no modules will break because we've already deprecated the effect the hook hook info has.
The include files can be deprecated when we add them in module handler. This should also hit all custom modules as well.

I pushed up a POC, tons of tests should fail cause we have not removed the three helpers that core still has and this will trigger deprecations.

I pushed up a first attempt at documenting the expected lifecycle of hook_hook_info.

I also moved views.views.inc and datetime.views.inc so I could see which tests really fail.

Commit d23dd379163049272f855204ad92f5a0756766b0 will need to be reverted when the two issues postponing this one are in.

Turns out I missed content moderation views inc I'll add that to the deprecation issue.

I will likely combine the two issues postponing this one, since they really are about removing views.views.inc

The only failure is the one we need to add an expect deprecation for because it tests the functionality we're deprecating.

Tagging this for the deprecation method since this is a unique deprecation.

Ok the dependent issues got in, just waiting on tests.

We discussed this in slack - the 'soft' deprecation so that people don't jump the gun here is good.

I also wondered about a 'hard' (normal) deprecation in a future release so that we eventually tell people to remove it, but the risk of people still jumping the gun (e.g. breaking support with Drupal < 11.2 prematurely) will be there, and then when we finally get to a core release where that's not a problem, we might as well have just removed the supporting code anyway instead of formally deprecating the hook. That means some modules could still ship with crufty hook_hook_info() implementations, but when we eventually deprecate .module files altogether, it will get deleted with those.

Removing release manager tag per discussion in slack and @catch's comment here.

Posted a review on wording stuff.

This looks good otherwise and at this point, the actual changes in the MR are trivial. It will affect a large amount of modules though. https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22Imple... finds 1000 tokens.inc files, and https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22Imple... finds 2500 matches.

This also matches 7.x modules, likely a lot of false positives, but still, no way around the fact that a lot of modules will need to adjust for this one. The good thing is that the fix is trivial, anything can just be moved into the .module file and optionally converted to OOP hooks if desired, which resolves the "large hook implementations" concern. And as long as LegacyHook is used, it's fully backwards compatible.

CR looks pretty good to me. I'd explicitly link the Hook CR so people who are interested in that can find it.

I addressed your comments.

I didn't quite realize how many there are but thankfully it's an easy copy and paste, and there is the rector rule for converting.

I think this is ready then, as mentioned above, this impacts a massive amount of modules, but the solution is trivial and fully backwards compatible, optionally with new hooks and LegacyHook, so you don't need to require 11.2 for this.

Looks great. Obviously we already moved all the core implementations but I temporarily forgot that so it was nice to see such a small diff.

Took a long time but got there in the end.
Committed/pushed to 11.x, thanks!