Pass the hook as last argument to method

I was also thinking about this at some point, some hooks already kind of have this feature, such as form alters that pass in $form_id.

But I'm not sure this this will really result in better code and should be encouraged. Less lines of code is only one thing to consider, more conditional code also increases complexity and is more likely to have bugs.

I think it would be worth to look at some real-world use cases of this, the example here is obviously made up (you would not want to implement translation and regular entity update/insert hooks at the same time, since you would be called multiple times on the same save for example), the only 1% difference part also seems a bit extreme ;)

(...) the example here is obviously made up (you would not want to implement translation and regular entity update/insert hooks at the same time,

The only hook made up is entity_translation_insert. My hope was to be able to write something like:

  #[Hook('entity_insert')]
  #[Hook('entity_update')]
  #[Hook('entity_delete')]
  #[Hook('entity_translation_delete')]
  #[Hook('entity_revision_delete')]
  public function do(EntityInterface $entity, str): void {
    $op = match($hook) {
       ...
    };
   \Drupal::service(...)->do($entity, $op);
  }

Yeah, that example implies that there's some sort of grouped mapping. e.g. insert/update would be one method/parameter on your service and the 3 delete hooks another. I don't think that's better than two methods.

We're also discussing to remove the coding standard for the Implements hook_foo(), further reducing boilerplate code.

I very much expect the tests will hit some edge cases. For example, there are hooks with optional parameters, such as hook_entity_field_access(). this isn't going to work with that.

It's also essentially dynamically changing the signature of all hooks.

Further, invoke is not the most common way of invoking hooks, you'd have to handle invokeAll and invokeAllWith too.

I think the minute edge case isn't worth the risk and complication here.

Further the method would be more complex to implement and harder to grok.

I really think multiple methods with a helper would be the way to go.

-1 for the proposal as is, for the reasons already given. Overloading existing hooks with an extra optional argument just feels like it is going to run into edge cases.

I think this is clearer with an adapter for each hook implementation that calls a single helper method.

If there are enough use cases for it I could perhaps get behind an alternative method signature, something like

#[Hooks(['entity_insert', 'entity_update', 'entity_delete'])]
public function do(string $hook, ...$args) {
...
}

ie. the hook gets passed in first - but I'm not convinced this is needed enough to do this. It feels like a step back to Drupal 6 and earlier when we had hooks that took $op arguments.

I suppose if you really wanted it to be magic we could use reflection and pass in the hook name as in #7 if the first parameter is string $hook but that would probably be too surprising and not really discoverable.

Always passing the hook as last parameter is problemetic in multiple ways:
- Variadic hooks that accept any number of arguments.
- Hooks with many optional arguments.
- Hooks where we add/invent new parameters in the future.

An alternative is to mark one parameter (the first one) with a special attribute.
Or mark the method itself with an attribute.

class MyHooks {
  #[Hook(...)]
  #[Hook(...)]
  public function foo(#[HookName] string $hook, $other_arg) {}
}

class MyHooks {
  #[Hook(...)]
  #[Hook(...)]
  #[HookNameAsFirstArgument]
  public function foo(string $hook, $other_arg) {}
}

I think to remember a case where this would have been convenient, but in the end I did not really need.

I still think this is the wrong way to do it, it's cleaner to just have a helper and pass the caller manually to the helper.

The situations @donquixote mentioned are just some of the issues.