Draft Add HookConvert

[nlighteneddesign]

Description

Explain the technical implementation of the work done.

To Test

• Add steps to test this feature

Drupal.org issue

Provide a link to the issue from https://www.drupal.org/project/rector/issues. If no issue exists, please create one and link to this PR.

[bbrala]

Thanks for working on this. A few things.

We don't need the whole config and setlist changes. We just need the rule, and we need some unit tests to cover the fule (hopefully those will work).

If this wont work, we need to add somefunctional testing, but lets try and avoid that. Unit tests are superior.

I'll push a few changes to help out (hopefully you allowed maintainer edits on the fork)

[bbrala]

I think we will need a new way to test this, since this also creates extra files. Not sure yet how, but i have some ideas.

[bbrala]

What you could do it create a few before and after files somewhere and share those. Perhaps a simple module would do with a few hooks that would be converted. I have a plan for functionally testing that. :)

[nlighteneddesign]

How would I run this locally? Since the last three commits it does not execute against core locally now. I think it's from 5fdfdbb1ee887a95b5c20fe926557a9c81e3d884

[bbrala]

You need to do the following steps:

composer config repositories.nlighteneddesign vcs https://github.com/nlighteneddesign/drupal-rector
composer require palantirnet/drupal-rector:dev-rectorOOPHooks

Then create a file rector.php with:

<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;

return static function (RectorConfig $rectorConfig): void {
  // Adjust the set lists to be more granular to your Drupal requirements.
  // @todo find out how to only load the relevant rector rules.
  //   Should we try and load \Drupal::VERSION and check?
  $rectorConfig->rule(\DrupalRector\Rector\Convert\HookConvertRector::class);

  if (class_exists('DrupalFinder\DrupalFinderComposerRuntime')) {
    $drupalFinder = new \DrupalFinder\DrupalFinderComposerRuntime();
  } else {
    $drupalFinder = new \DrupalFinder\DrupalFinder();
    $drupalFinder->locateRoot(__DIR__);
  }
  $drupalRoot = $drupalFinder->getDrupalRoot();
  $rectorConfig->autoloadPaths([
    $drupalRoot . '/core',
    $drupalRoot . '/modules',
    $drupalRoot . '/profiles',
    $drupalRoot . '/themes'
  ]);

  $rectorConfig->skip(['*/upgrade_status/tests/modules/*']);
  $rectorConfig->fileExtensions(['php', 'module', 'theme', 'install', 'profile', 'inc', 'engine']);
  $rectorConfig->importNames(true, false);
  $rectorConfig->importShortClasses(false);
};

Then just run rector

vendor/bin/rector process core/modules/user/ --dry-run

[bbrala]

Magic ^^

[bbrala]

You could also use $rectorConfig->importShortClasses(true); so it will use imports for class names, but this will also mean you will get extra changes, like \Drupal::Xxx becomes Drupal::Xxx with an import.

[nlighteneddesign]

Thanks! I'll try that.

[nlighteneddesign]

That worked, but it's expanding all class names even with $rectorConfig->importShortClasses(true);

public function help($route_name, \Drupal\Core\Routing\RouteMatchInterface $route_match)

Any other changes? The older version was importing them as expected if you review the user conversion issue: https://git.drupalcode.org/project/drupal/-/merge_requests/9890/diffs

[bbrala]

importNames might help there. Che k the docs of that method.

[bbrala]

A rebase would be great since that would fixup the tests.

[bbrala]

Rebased and fixed codestyle and some glaring phpstan issues.

[chx]

phpstan is wrong this is a php-parser v4 codebase not a v5 one. It doesn't work with v5 , I tried already.

[nlighteneddesign]

@bbrala we had to revert your last two commits cause it broke the core vs contrib detection as well as some conversions

[bbrala]

Sorry :) thanks.

[bbrala]

THink everything kinda works now, did quite some work to get tests running.

Only question i have; do we really need the final thing? Seems it is final anyways, so why bother? It trips up phpstan :x

[nlighteneddesign]

Thank you for your help! I am testing it against webform now, I will post links to compare when done.

Unfortunately we do want to keep that final bit, we sed the final off that class in my helper script so we can format arrays properly.

[bbrala]

Didnt get LegacyHook detection to work, although i think it should. Need to debug with real install instead on tests, but cannot do that right now. There is a comment in the class that is a hint how it should work.

The bigpipe seems all good tbh. So thats a good thing, and all without the custom printer ;)

[bbrala]

Merged, thanks all! Maybe create a new PR to work on how we will toggle the BC part of this code.