Expand hook documentation to explain caller responsibilities

Created on 23 March 2011, about 14 years ago
Updated 29 January 2025, 2 months ago

I'm having a minor debate with @jhodgon over in #1101678: hook_search_status() should return 0 if search category disabled β†’ , and looking at out API documentation, I think Drupal documentation treats our APIs as if they are internal-only. That is, hooks are documented in terms of implementation, not invocation.

I think we should change that in Drupal 8.

The rationale:

-- For any hook (or method, too), you have two sides to its use

1. Modules that implement hook_foo().
2. Modules that call hook_foo().

Since most hook invocations are done through module_invoke_all() or other abstraction functions, there is no clear place to document how invoking the hook is to be handled. That is to say, we know what a single module should return for the hook, but do we know what _all_ modules will return for the hook.

Before we dismiss this idea as burdensome or insist that invoking modules should simply test their code, let's consider this a good place for some introspection and inspection of our APIs.

An API isn't really an API if it's only internally-facing. If, say, search.module is the only code that ever calls hook_search_status(), then we don't have an API function, we have a simple function registry callback.

If Drupal is going to have true APIs, then hooks need to be treated as encapsulated features of the code, such that they can be called consistently from any invoking function.

As I see it, the best way to enable that discipline (aside from encapsulating all module_invoke_all() calls in a module-specific wrapper, which is a Bad Idea), is to document our API functions so that both the implementer and the invoker can read the docs and understand the expectations of the system.

Sample change:

/**
 * Report the status of indexing.
 *
 * @return
 *  An associative array with the key-value pairs:
 *  - 'remaining': The number of items left to index.
 *  - 'total': The total number of items to index.
 *
 * @ingroup search
 */
function hook_search_status() {
...
/**
 * Report the status of indexing.
 *
 * The core search module invokes this hook on active modules to return
 * the completion status of the search index stored in {search_dataset}.
 * Implementing modules do not need to check whether they are active when
 * calculating their return values, it is the responsibility of the caller to do so.
 *
 * @return
 *  An associative array with the key-value pairs:
 *  - 'remaining': The number of items left to index.
 *  - 'total': The total number of items to index.
 *
 * @ingroup search
 */
function hook_search_status() {
...

We would also want to document instances where the hook passes by reference (or has other oddities) such that it cannot be used with module_invoke_all().

Ideally, these new elements would have a standard docblock marker (like @note or @remark or @post). That can be worked out later, if we want to go this direction.

Thoughts?

🌱 Plan
Status

Closed: works as designed

Component

Coding Standards

Created by

πŸ‡ΊπŸ‡ΈUnited States agentrickard Georgia (US)

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡³πŸ‡ΏNew Zealand quietone

    I read this issue and the one referred to in the issue summary to fully understand it. Like the others on this issue I personally do see the need for a standard to enhance the hook documentation. My personal view aside, there hasn't been support for this in the 9 years it has been open. Therefor, I am closing this.

    If anyone want to pursue this, re-open the issue and add a comment.

Production build 0.71.5 2024