Deprecated hooks not flagged

[benk-cogapp]

I'm using phpstan-drupal with phpstan-deprecation-modules, and it doesn't appear to be flagging up usage of deprecated hooks. Specifically, I have an implementation of hook_search_api_results_alter in a custom module that is being scanned, and the implementation doesn't get flagged even though the hook is marked as deprecated. This would be a super useful feature for code checking for D9.

[mglaman]

The problem is that hooks are not directly invoked code. We would need to somehow bootstrap the hook registry and check if the hook is invoked.

This is somewhere that the upgrade_status provides more benefit since hooks cannot be easily statically checked.

[benk-cogapp]

Sure, I can see that's a tricky problem to solve statically! I'll check out upgrade_status in the interim.

[mglaman]

Notes to explain the problem:

   $hooks = ['search_api_results'];
    foreach ($this->tags as $tag) {
      $hooks[] = "search_api_results_$tag";

      $event = new ProcessingResultsEvent($this->results);
      $this->getEventDispatcher()->dispatch("$event_base_name.$tag", $event);
      $this->results = $event->getResults();
    }
    $description = 'This hook is deprecated in search_api 8.x-1.14 and will be removed in 9.x-1.0. Please use the "search_api.processing_results" event instead. See https://www.drupal.org/node/3059866';
    $this->getModuleHandler()->alterDeprecated($description, $hooks, $this->results);

The problem is that hooks do not get deprecated in a normal fashion as other methods or classes. You pass a string to the alterDeprecated method and that triggers the deprecation. There isn't an easy way to check if the hook is implemented and bubble up the flag.

[mglaman]

I tried an approach in #135 that didn't work because we could only error when the deprecatedAlter was invoked not by scanning each function.

[mglaman]

Here is one idea that I did not consider previously:

• Have a rule that scans defined functions
• Drop the module name from the beginning of function name (using the .module filename as module name)
• Check if that function prefixed with hook_ can be found
• Check if the discovered hook_* is deprecated

See https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/search_api.api.php

The one problem is that we now must include the .api.php files if they exist. Which may have side effects.

[goba]

Per https://www.drupal.org/project/upgrade_status/issues/3142908#comment-15201817 there are only 4 deprecated hooks in Drupal 9 core, so even a similar hardcoded check like phpstan-drupal does for constants could work.

• hook_field_widget_form_alter
• hook_field_widget_multivalue_form_alter
• hook_field_widget_multivalue_WIDGET_TYPE_form_alter
• hook_field_widget_WIDGET_TYPE_form_alter

@fanton points out that this can be verified at https://api.drupal.org/api/drupal/deprecated/9?deprecated_textfilter=hook_

We could also write one-off code for these in Upgrade Status but it would be more universal to cover it in phpstan-drupal.

[mglaman]

I like this idea. I want the rule to be configurable via a parameter. So that way we have DeprecatedHookRule with string $hook_name as an argument.

So something like:

services:
	-
		class: DeprecatedHookRule
		tags:
			- phpstan.rules.rule
		arguments:
			- 'field_widget_form_alter'

That would also also allow handling search_api_results_alter by another definition of:

services:
	-
		class: DeprecatedHookRule
		tags:
			- phpstan.rules.rule
		arguments:
			- 'search_api_results_alter'

OR! I think PHPStan merges parameters, so maybe the rule could be:

parameters:
    drupal:
        deprecatedHooks:
             - field_widget_form_alter
             - search_api_results_alter
services:
	-
		class: DeprecatedHookRule
		tags:
			- phpstan.rules.rule
		arguments:
			- '%drupal.deprecatedHooks%'

[goba]

Sounds great :)

[Kingdutch]

I implemented this in the context for Open Social last month but I did not have time to move it to a PR for the phpstan-drupal module: https://github.com/goalgorilla/open_social/compare/main...internal/phpstan-deprecated-hook-rule

The work that's needed:

• Copy it from Open Social to PHPStan Drupal
• Create a fixture module and complete the test case for the feature so we have reliable confirmation that it works and keeps working

See also the Slack discussion: https://drupal.slack.com/archives/C033S2JUMLJ/p1694172339837999