[11.1] update gather rules to manage hook preprocess

One possible idea, it's quite horrible.

Instead of the exception, can we reverse engineer what the expected function name would be from the OOP hook implementation, check function_exists(), and if it's not there, trigger_error() something?

I'm not sure why this is failling:

core/tests/Drupal/KernelTests/Core/Asset/ResolvedLibraryDefinitionsFilesMatchTest.php fails cause it's looking for modules as themes and they don't exist.

This test doesn't run locally so I can't easily debug.

For some reason this change is triggering module handler to not have the modules when LibraryDiscoveryParser::buildByExtension is checking so it checks for modules as themes.

This doesn't make sense, this shouldn't change anything related to this.

I even tried an alternate version where we pass the module in through collect by attribute to procedural check and throw the error only there if the function doesn't exist.

I reverted my changes and it still fails. Is this an 11.1.x only failure?

That test is busted on 11.1.x: https://git.drupalcode.org/project/drupal/-/pipelines/499192

I added a test for this specifically, but even with the expect exception for the one we expect, I'm getting a symfony exception.

I could use some assistance getting past this:

1) Drupal\KernelTests\Core\Hook\HookCollectorPassTest::testHookPreprocessNoFunction
Symfony\Component\HttpFoundation\Exception\SessionNotFoundException: There is currently no session available.

I converted to trigger_error as suggested in 2. There doesn't seem to be a way to assert user warnings anymore. I tried converting to an exception in the test as suggested by the phpunit issue, but I run into the same symfony exception.

This is truly ready for review now!

Please credit @fathershawn for his help on slack getting me past the test issues!

The failure is unrelate I think and exists on 11.1.x HEAD.

Created 🐛 [11.1] ResolvedLibraryDefinitionsFilesMatchTest is failing Active for the failing test on 11.1.x HEAD

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I think the bot is wrong here cause it's a different branch.

This looks great.

Currently, if you add an OOP preprocess hook, you get an exception on 11.1 because it doesn't support preprocess has OOP hooks, and it's trying to stop you prematurely converting things that won't work.

With the MR, the next 11.1 patch release will allow the OOP hook to exist and ignore it, as long as there's a procedural version too - e.g. still using LegacyHook, but warn you if the legacy hook is missing.

The result is that modules will be able to require >= 10.4.0 | >= 11.1.8 (or whatever exact version it ends up in) if they want to convert. Otherwise their only option would be >= 10.4.0 | > 11.2.0

Committed 59f1bee and pushed to 11.1.x. Thanks!

I updated the CR in https://www.drupal.org/node/3496491 →

quietone → closed merge request !12136

Automatically closed - issue fixed for 2 weeks with no activity.