DMU no longer finds and converts hooks

Same here with both D8-Beta14 and latest D8-dev. ( The dmu commits from 21-aug made a huge difference, though.)

I have the same error and stack trace here with Drupal 8.0.0-beta14 & latest drupalmoduleupgrader from git. Not sure what other information to provide. I ran drush dmu-upgrade on message_ui module.

I have the same error with the lastest git checkout of D8 and drupalmoduleupgrader

Confirmed. :( No idea what's going on but I will try and dig in more.

The "hook" plugin doesn't exist. The discovery of hooks is now done with \Drupal\drupalmoduleupgrader\Plugin\DMU\Indexer\Functions, aka the "functions" plugin.

Attached patch solves this exception. Mind that this will not magically make the drupalmoduleupgrader work again, there are other exceptions that are popping up after this one is fixed.

Thanks for the patch. I applied it and re-ran drush dmu-upgrade.

drush dmu-upgrade message_ui
Indexing...done.
Drush command terminated abnormally due to an unrecoverable error.                                                                                      [error]
Error: Call to undefined method Drupal\drupalmoduleupgrader\Plugin\DMU\Indexer\Functions::query() in
/Applications/MAMP/htdocs/dev.domain.com/modules/drupalmoduleupgrader/src/Plugin/DMU/Converter/HookFormAlter.php, line 24

I can't find the query() method in the latest version of drupalmoduleupgrader from git. Is it supposed to use the getQuery() method?

@mccrodp: I'm looking into it. Just had a brief chat with @phenaproxima on IRC and it's OK for him to do broaden the scope of this issue to do a couple more fixes.

I'm getting a similar fatal error, but in the Classes class.

The count() method was removed in commit 892036edf, but apparently not all its usages were fixed. I grepped the codebase and it only seems to be used in the PSR4 class at the moment.

Fixed the count() fatal error.

Next up:

exception 'Drupal\Component\Plugin\Exception\PluginNotFoundException' with message 'The "test" plugin does not exist.' in core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php:57

Trying it on a project that actually has tests, am getting:

Fatal error: Call to a member function sortKey() on null in modules/drupalmoduleupgrader/vendor/grom358/pharborist/src/NodeCollection.php on line 24

OK the sortKey() on null was easy, back on the one from #10.

Fixed the fatal error from #10 as well as the following ones:

exception 'Drupal\Component\Plugin\Exception\PluginNotFoundException' with message 'The "unit_test" plugin does not exist.' in
core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php:57

Fatal error: Call to undefined method Drupal\drupalmoduleupgrader\Plugin\DMU\Indexer\Functions::query() in modules/drupalmoduleupgrader/src/Plugin/DMU/Converter/HookFormAlter.php on line 24

I can now run the drush dmu-upgrade command successfully. I am using the Language Icons module for testing.

Probably with other modules there will be some more fatals remaining.

I also want to add a disclaimer: I have no idea what I'm doing :) There might be better ways to solve these problems, I am not familiar at all with how Pharborist works. It does look like a very nice library!

The patch applied cleanly and is running in the terminal for me with no errors or warnings. Will take a closer look at the output soon, but it's converted the info to yaml and various other parts, so seems good to me. Many thanks again, looks like a lot of changes were needed in this patch.

First of all In the past (months ago), DMU worked fine for updating some smaller modules. Thanks a lot for this module!
Now, I am trying up update the Workflow module. The following error appears after applying #13:

Indexing...done.
PHP Fatal error:  Call to undefined method Pharborist\Functions\FunctionDeclarationNode::get() in \modules\drupalmoduleupgrader\src\Plugin\DMU\Converter\HookFormAlter.php on line 33

Fatal error: Call to undefined method Pharborist\Functions\FunctionDeclarationNode::get() in \modules\drupalmoduleupgrader\src\Plugin\DMU\Converter\HookFormAlter.php on line 33
Drush command terminated abnormally due to an unrecoverable error.       [error]

Error: Call to undefined method Pharborist\Functions\FunctionDeclarationNode::get() in
\modules\drupalmoduleupgrader\src\Plugin\DMU\Converter\HookFormAlter.php, line 33

#15 can be fixed by changing line 33 in \src\Plugin\DMU\Converter\HookFormAlter.php like this:

    foreach ($alter_hooks as $alter_hook) {
      /** @var \Pharborist\Functions\FunctionDeclarationNode $function */
-      $function = $indexer->get($alter_hook->id)->get(0);
+      $function = $indexer->get($alter_hook->id);

The following error appears (3 times) after fixing #15/#16 :

Indexing...done.
PHP Fatal error:  Call to undefined method Pharborist\Functions\FunctionDeclarationNode::get() in \modules\drupalmoduleupgrader\src\Plugin\DMU\Converter\HookMenuAlter.php on line 50

#17 can be fixed by simple removing line 50:

  public function convert(TargetInterface $target) {
    $target
      ->getIndexer('function')
      ->get($this->pluginDefinition['hook'])
-      ->get(0)
      ->setDocComment($this->buildFixMe(NULL, [], self::DOC_COMMENT));

After fixing #17/#18, the following error appears:
PHP Parse error: syntax error, unexpected 'class' (T_CLASS), expecting identifier (T_STRING) or variable (T_VARIABLE) or '{' or '$' in \core\lib\Drupal\Core\Controller\ControllerResolver.php on line 151

I guess this is because my drush (OnWindows with Acquia DevDesktop) runs on PHP 5.4 instead of 5.5? Don't know how to fix/set that.

[EDIT] Restarting the drush-console somehow did the trick. Now runs on 5.6. :-)

Got @pfrenssen's fix in as a start; I'm seeing the same errors as @johnv on Diff module; will try his suggestions next.

@johnv's additional fixes get me to the point that I can convert Diff module and even enable it! YEAH! :D

However, things are still pretty broken compared to how they were working in January. No permissions.yml is created, no routing.yml is created, etc.

I grepped and found a ton of other calls to get(0) (including in Pharborist, though, hm...); will try deleting some of them in the same manner and see if the situation improves.

Erred on the conservative side and only blitzed ones that were related to the indexer.

Ah, working in parallel! :-)

With #23 in place, i get the following:

Indexing...done.
Render context is empty, because render() was called outside of a renderRoot() or renderPlain() call. Use renderPlain()/renderRoot() or #lazy_builder/#pre_render instead.

I do get changed files and a new info.yml file.

Yep, I got that too, not sure what that is yet.

With the patch in #24, analyze mode still works, so that's nice that we didn't break that.

Still not seeing *routing.yml or *permissions.yml tho. :\

So starting with permissions, since that's easiest... unfortunately I can't seem to get CLI debugging working, so here's how far I got with moving around echo 'bananas'; :P

The code that needs to fire to convert from hook_permission() => module.permission.yml is in src/Plugin/DMU/Converter/HookPermission.php:

  public function convert(TargetInterface $target) {
    try {
      $permissions = $this->executeHook($target, $this->pluginDefinition['hook']);
      $this->writeInfo($target, 'permissions', $permissions);
    }
    catch (\LogicException $e) {
      return;
    }
  }

This method is not getting called at all though. (echo 'bananas'; inside convert() never appears.)

So what calls ->convert()? That happens in drupalmoduleupgrader.drush.inc in the function drush_drupalmoduleupgrader_dmu_upgrade($module) { function.

  $converters = \Drupal::service('plugin.manager.drupalmoduleupgrader.converter');
  foreach (_dmu_plugin_list('converter') as $id) {
    /** @var \Drupal\drupalmoduleupgrader\ConverterInterface $converter */
    $converter = $converters->createInstance($id);

    if ($converter->isExecutable($target)) {
      drush_log(\Drupal::translation()->translate('Executing plugin: @plugin_id', ['@plugin_id' => $id]), 'notice');
      try {
        $converter->convert($target); # the magic sauce...
      }
      catch (Exception $e) {
        drush_log($e->getMessage(), 'error');
        // Being a notice, the stack trace will only appear in verbose mode.
        drush_log($e->getTraceAsString(), 'notice');
      }
    }
  }

The output of var_dump($converters); is super confusing. https://gist.github.com/webchick/ec5806ceef5565422f2f There are only a couple of references to anything DMU or Converter in there.

var_dump(_dmu_plugin_list('converter')); is more promising. https://gist.github.com/webchick/c58fc6b141163d04df98 It list dozens of converters, and you can see hook_permission is one of them:

array(157) {
...
  [146]=>
  string(15) "hook_permission"
...
}

What happens then is it loops through each of those and attempts to instantiate them with:

$converter = $converters->createInstance($id);

Here's one such dump for Blocks, as an example. https://gist.github.com/webchick/c174d9a8db68e5ea1492 It seems to have one of everything under the sun in it, but eventually you see an array like:

    ["hook"]=>
    array(4) {
      [0]=>
      string(10) "block_info"
      [1]=>
      string(15) "block_configure"
      [2]=>
      string(10) "block_save"
      [3]=>
      string(10) "block_view"
    }

This all comes by way of the annotation on above the converter class, like:

/**
 * @Converter(
 *  id = "blocks",
 *  description = @Translation("Converts Drupal 7 blocks to plugins."),
 *  hook = { "block_info", "block_configure", "block_save", "block_view" },
 *  fixme = @Translation("hook_!hook is gone in Drupal 8.

It has been left here by the Drupal Module Upgrader so that you can move its
logic into the appropriate block plugins, which should be in the
src/Plugin/Block directory. Once all logic is moved into the plugins, delete
this hook."),
 *  documentation = {
 *    "https://www.drupal.org/node/1880620"
 *  }
 * )
 */
class Blocks extends ConverterBase {

Any who. Once a converter is instantiated, the next check is against isExecutable() in ConverterBase:

  public function isExecutable(TargetInterface $target) {
    // If the plugin applies to particular hook(s), only return TRUE if the
    // target module implements any of the hooks. Otherwise, return TRUE
    // unconditionally.
    if (isset($this->pluginDefinition['hook'])) {
      return (boolean) array_filter((array) $this->pluginDefinition['hook'], [ $target->getIndexer('function'), 'has' ]);
    }
    else {
      return TRUE;
    }
  }

Since this plugin does in fact apply to hooks, we do the first logic branch. It's checking to see if the module contains the hook. And it's returning FALSE, despite the module does in fact implement the hook.

So presumably, just blindly removing ->get(0) to kill the fatal error is not the right approach, and it likely needs to be swapped with something else, but I've no idea what just now.

More specific title, based on current findings.

How can I run the tests? I'm confused because there is a phpunit.xml.dist in the module so it seems to be the intention to run this with the phpunit that is included in core. This doesn't work though since drupalmoduleupgrader has its own vendor folder so its dependencies and namespaces are not registered.

$ cd core/
$ ./vendor/bin/phpunit
Fatal error: Class 'cebe\markdown\Markdown' not found in /home/pieter/v/drupal/drupal/modules/drupalmoduleupgrader/src/Issue.php on line 64

I found out why it is not working. The hooks are defined in annotations. For example for hook_menu() the relevant part of the annotation looks like this:

/**
 * @Converter(
 *  id = "links",
 *  description = @Translation("Converts Drupal 7's hook_menu() links to plugin definitions."),
 *  hook = "menu",
 */

This annotation is evaluated to see if it is "executable", i.e. if it applies to the piece of PHP code that is being analyzed. This happens in ConverterBase::isExecutable():

  public function isExecutable(TargetInterface $target) {
    // If the plugin applies to particular hook(s), only return TRUE if the
    // target module implements any of the hooks. Otherwise, return TRUE
    // unconditionally.
    if (isset($this->pluginDefinition['hook'])) {
      return (boolean) array_filter((array) $this->pluginDefinition['hook'], [ $target->getIndexer('function'), 'has' ]);
    }
    else {
      return TRUE;
    }
  }

Unfortunately this never matches, because the function "menu()" does not exist in the codebase.

I discussed this with @phenaproxima on IRC, and he suggested to simply prefix the hook declaration in the annotation with hook_.

Converted the annotations and searched for more instances where the old 'hook' indexer was used and replaced these with the 'function' indexer.

This seems quite ready, but note that this doesn't solve all errors. The errors that occur now are not related to converting hooks.

Have committed the patch. The problem seems to be solved but it's hard to be 100% sure since there are other errors still occurring. Will close the issue for now, will reopen if the bug reappears after addressing the other errors.

YOU KICK SO MUCH ASS!! :D :D THANK YOU!! Will test shortly.