Remove hook_module_implements_alter and support in HookCollectorPass

I will continue to work on this

Let's invert these postponements since this one has a lot more failures.

Just confirming that I want this still assigned to me.

I will review.

I tracked down all of the other changes.
one was due to an over aggressive cleanup which I restored.

The only mystery is why Form/Functional/AlterTest.php changed https://git.drupalcode.org/project/drupal/-/merge_requests/14784/diffs#e...

I think it might be because it's defined with the module key instead of directly in the module: #[Hook('form_form_test_alter_form_alter', module: 'block')]
I suspect that we want to fix this, but it's not immediately clear where this is changing.

Edit: on further inspection there are more failures now because now
$implementationsByHook[$hook] is empty since

foreach (array_keys($implementationsByHookOrig[$hook], $module, TRUE) as $identifier) {
          $implementationsByHook[$hook][$identifier] = $module;
        }

doesn't populate since block is not installed, but I can't see how not having

      foreach ($this->moduleImplementsAlters as $alter) {
        $alter($moduleImplements, $hook);
      }

changes this.

block module and also sets the order of it but then that module isn't enabled in those tests and it's confused about that. Related to #3536470: TypeError if #[ReOrderHook] targets hook that has no implementations anywhere in enabled modules. but that is fixed, so I guess it's a different case. Added a check for that, not entirely sure that's the correct fix.

Yeah I found that too, the piece I couldn't figure out was why it didn't blow up before pulling the procedural call.

For alter they are all defined in the same module two are on behalf of other modules though (system and block)

This combined with the other failures makes me think it's an issue around defining hooks on behalf of other modules.

I wonder if we should deprecate that long term since a module exists check works and we have the other conditional issue we're working on.

Ok I think I confirmed it's a side effect of implementing on behalf of others.
I set the order parameter on that hook.

I wonder if we should just add a CR to this that if you're relying on the 'module' parameter that you may need to evaluate order.
That is a BC compatible change since #[Hook('form_form_test_alter_form_alter', module: 'block', order: Order::First)] would have done nothing before this issue since it was already first, adding it here moves it back to first.

BTW before 14 I did checkout main and compared calculateImplementations before and after with multiple variations.

I didn't see anything obvious that was changing. Now that we've identified how it's changing and it's a rare edge case I think we want to deprecate anyway, AND has a fairly simple workaround if it's even an issue. I think this might be ready beyond wanting to properly fix the order bit.

Extra notes, as I was writing this I reread the issue you mentioned and realized the issue. I fixed it here is the reasoning.
I think the real fix is if (!isset($implementationsByHook[$hook][$operation->identify()])) { needs to also check if the module is enabled, then we need to filter for empty order sets at the end, it's just a couple of lines so I added that but should be reviewed.

Leaving this assigned to me for now for addressing any feedback that comes up, but I think this is ready for review.

Gotta fix the missing type error failures too.

I found the side effect changing the order of the implemented on behalf of other modules:

The reOrderModulesForAlter method reset order to module order first before reapplying the order.
This only affects alters, I'm not sure we want to preserve or remove this now.

Adding the following back fixes the tests without needing to add a new order directive so this preserves current behavior so I'd vote for keeping it in.

// Order by module order first.
$modules = array_intersect(array_keys($this->moduleList), $modules);

This is finally ready for review!

I addressed some of your feedback and replied to the rest.

For posterity where it was introduced #692950-1: Use hook_module_implements_alter to allow modules to alter the weight of hooks in module_implements

Thanks!