Deprecate drupal_find_theme_templates to Theme Registry service

Thanks for this clean-up. These is my favourite clean-ups:)

+++ b/core/includes/theme.inc
@@ -117,51 +117,8 @@ function drupal_theme_rebuild() {
  *   The functions found, suitable for returning from hook_theme;
  */
 function drupal_find_theme_functions($cache, $prefixes) {

Likely will need those deprecated notices in the docblocks.

re #8 that is strange.

+++ b/core/lib/Drupal/Core/Theme/Registry.php
@@ -766,4 +766,168 @@ public function getPrefixGroupedUserFunctions() {
+  public function find_theme_templates($cache, $extension, $path) {

Since this is a OOP thing now it should be findThemeTemplates() or just findTemplates() to save a bit of redundency but most important is the camel casing for methods.

Thanks for the cleanup @stevector. This is the related type of move #2591515: Move twig_without() to the TwigExtension where all the other filters are and deprecate and this patch is trying to touch the same bit of code #2339447: Improve theme registry build performance by 85% which may cause a re-roll.

Not sure if this will will make it into a patch release or 8.1.x, so I'll just leave it until 8.0.0 is released.

I like the idea of this issue, getting rid of procedural functions, but I think we can do one better and make this one of the goals of #2869859: [PP-1] Refactor theme hooks/registry into plugin managers.

This way, themes (base themes) would finally have the power to properly hook into this process.

The above issue #2869859: [PP-1] Refactor theme hooks/registry into plugin managers really makes this issue a moot point.

This issue really pertains to #2957440: Refactor ThemeRegistry and Theme\Registry for BC purposes.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

needs re-roll for 10.1 and fix deprecation to that version

MR had just a few failures.

Warning: foreach() argument must be of type array|object, string given in Drupal\Core\Theme\Registry->findThemeFunctions() (line 932 of /var/www/html/repos/drupal/core/lib/Drupal/Core/Theme/Registry.php)
#0 /var/www/html/repos/drupal/core/includes/bootstrap.inc(158): _drupal_error_handler_real(2, 'foreach() argum...', '/var/www/html/r...', 932)
#1 /var/www/html/repos/drupal/core/lib/Drupal/Core/Theme/Registry.php(932): _drupal_error_handler(2, 'foreach() argum...', '/var/www/html/r...', 932)
#2 /var/www/html/repos/drupal/core/themes/engines/twig/twig.engine(17): Drupal\Core\Theme\Registry->findThemeFunctions(Array, '.html.twig', 'core/themes/cla...')
#3 /var/www/html/repos/drupal/core/lib/Drupal/Core/Theme/Registry.php(455): twig_theme(Array, 'theme_engine', 'claro', 'core/themes/cla...')
#4 /var/www/html/repos/drupal/core/lib/Drupal/Core/Theme/Registry.php(370): Drupal\Core\Theme\Registry->processExtension(Array, 'twig', 'theme_engine', 'claro', 'core/themes/cla...')
#5 /var/www/html/repos/drupal/core/lib/Drupal/Core/Theme/Registry.php(247): Drupal\Core\Theme\Registry->build()
#6 /var/www/html/repos/drupal/core/lib/Drupal/Core/Utility/ThemeRegistry.php(88): Drupal\Core\Theme\Registry->get()
#7 /var/www/html/repos/drupal/core/lib/Drupal/Core/Utility/ThemeRegistry.php(69): Drupal\Core\Utility\ThemeRegistry->initializeRegistry()
#8 /var/www/html/repos/drupal/core/lib/Drupal/Core/Theme/Registry.php(267): Drupal\Core\Utility\ThemeRegistry->__construct('theme_registry:...', Object(Drupal\Core\Cache\ChainedFastBackend), Object(Drupal\Core\ProxyClass\Lock\DatabaseLockBackend), Array, true)
#9 /var/www/html/repos/drupal/core/lib/Drupal/Core/Theme/ThemeManager.php(141): Drupal\Core\Theme\Registry->getRuntime()
#10 /var/www/html/repos/drupal/core/lib/Drupal/Core/Render/Renderer.php(436): Drupal\Core\Theme\ThemeManager->render('file_upload_hel...', Array)
#11 /var/www/html/repos/drupal/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, true)
#12 /var/www/html/repos/drupal/core/lib/Drupal/Core/Render/Renderer.php(160): Drupal\Core\Render\Renderer->render(Array, true)
#13 /var/www/html/repos/drupal/core/lib/Drupal/Core/Render/Renderer.php(583): Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
#14 /var/www/html/repos/drupal/core/lib/Drupal/Core/Render/Renderer.php(161): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#15 /var/www/html/repos/drupal/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php(269): Drupal\Core\Render\Renderer->renderPlain(Array)
#16 /var/www/html/repos/drupal/core/modules/image/src/Plugin/Field/FieldWidget/ImageWidget.php(144): Drupal\file\Plugin\Field\FieldWidget\FileWidget->formElement(Object(Drupal\file\Plugin\Field\FieldType\FileFieldItemList), 0, Array, Array, Object(Drupal\Core\Form\FormState))
#17 /var/www/html/repos/drupal/core/lib/Drupal/Core/Field/WidgetBase.php(349): Drupal\image\Plugin\Field\FieldWidget\ImageWidget->formElement(Object(Drupal\file\Plugin\Field\FieldType\FileFieldItemList), 0, Array, Array, Object(Drupal\Core\Form\FormState))
#18 /var/www/html/repos/drupal/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php(167): Drupal\Core\Field\WidgetBase->formSingleElement(Object(Drupal\file\Plugin\Field\FieldType\FileFieldItemList), 0, Array, Array, Object(Drupal\Core\Form\FormState))
#19 /var/www/html/repos/drupal/core/modules/image/src/Plugin/Field/FieldWidget/ImageWidget.php(117): Drupal\file\Plugin\Field\FieldWidget\FileWidget->formMultipleElements(Object(Drupal\file\Plugin\Field\FieldType\FileFieldItemList), Array, Object(Drupal\Core\Form\FormState))
#20 /var/www/html/repos/drupal/core/lib/Drupal/Core/Field/WidgetBase.php(111): Drupal\image\Plugin\Field\FieldWidget\ImageWidget->formMultipleElements(Object(Drupal\file\Plugin\Field\FieldType\FileFieldItemList), Array, Object(Drupal\Core\Form\FormState))
#21 /var/www/html/repos/drupal/core/lib/Drupal/Core/Entity/Entity/EntityFormDisplay.php(183): Drupal\Core\Field\WidgetBase->form(Object(Drupal\file\Plugin\Field\FieldType\FileFieldItemList), Array, Object(Drupal\Core\Form\FormState))
#22 /var/www/html/repos/drupal/core/lib/Drupal/Core/Entity/ContentEntityForm.php(121): Drupal\Core\Entity\Entity\EntityFormDisplay->buildForm(Object(Drupal\user\Entity\User), Array, Object(Drupal\Core\Form\FormState))
#23 /var/www/html/repos/drupal/core/modules/user/src/AccountForm.php(311): Drupal\Core\Entity\ContentEntityForm->form(Array, Object(Drupal\Core\Form\FormState))
#24 /var/www/html/repos/drupal/core/lib/Drupal/Core/Entity/EntityForm.php(106): Drupal\user\AccountForm->form(Array, Object(Drupal\Core\Form\FormState))
#25 [internal function]: Drupal\Core\Entity\EntityForm->buildForm(Array, Object(Drupal\Core\Form\FormState))
#26 /var/www/html/repos/drupal/core/lib/Drupal/Core/Form/FormBuilder.php(534): call_user_func_array(Array, Array)
#27 /var/www/html/repos/drupal/core/lib/Drupal/Core/Form/FormBuilder.php(281): Drupal\Core\Form\FormBuilder->retrieveForm('user_form', Object(Drupal\Core\Form\FormState))
#28 /var/www/html/repos/drupal/core/lib/Drupal/Core/Controller/FormController.php(73): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\user\ProfileForm), Object(Drupal\Core\Form\FormState))
#29 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#30 /var/www/html/repos/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#31 /var/www/html/repos/drupal/core/lib/Drupal/Core/Render/Renderer.php(583): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#32 /var/www/html/repos/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#33 /var/www/html/repos/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#34 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(163): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#35 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(74): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#36 /var/www/html/repos/drupal/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#37 /var/www/html/repos/drupal/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#38 /var/www/html/repos/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#39 /var/www/html/repos/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#40 /var/www/html/repos/drupal/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#41 /var/www/html/repos/drupal/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#42 /var/www/html/repos/drupal/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#43 /var/www/html/repos/drupal/core/lib/Drupal/Core/DrupalKernel.php(698): Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#44 /var/www/html/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#45 {main}
.

Getting this error , which is causing the test failures.

Updated title as only one left

+++ b/core/includes/theme.inc
@@ -114,53 +114,14 @@ function drupal_theme_rebuild() {
 function drupal_find_theme_functions($cache, $prefixes) {

is deprecated and removed

Hi I just rebased

It needs work to remove all mentions of "theme function", left a set of review-comments

From @andypost review:

• findThemeFunctions is not removed
• core/lib/Drupal/Core/Theme/Registry.php is not refactored

Adding MR changes as patch for backup before rebase.

Made changes to MR and address review comments.

Not sure how to handle adding new constructor argument.

Created new issue to handle adding new constructor argument. #3490397: Inject the file_system service in the theme registry service.

Should we handle adding new constructor argument in this issue or in other issue ?

Left some small comments on the MR

@smustgrave

Phpcs failing without those nit changes hence keeping them.

This looks pretty good. I have a couple of comments.

1. There seems to be a lot of our of scope code style changes were those causing actual failures? just saw your last comment.
2. You do not need to test pure bc wrapper functions, please remove that test.

My question is did we consider adding a helper?
Then we could inject that new helper into registry and the twig engine. It seems like a lot to use the whole registry just to get the templates in the engine.

I really think this piece should be moved to a themetemplatefinder.

Also you'd add the constructor here, not a child issue.

@nicxvan : Thanks for the review.

> My question is did we consider adding a helper ?

AFAIK, there wasn't any discussion in this regards.

> It seems like a lot to use the whole registry just to get the templates in the engine.

Agreed.

> I really think this piece should be moved to a themetemplatefinder.

Seems reasonable. Where do you suggest its location ?

How does core/lib/Drupal/Core/Theme/Themetemplatefinder.php sound ?

And do we need interface for this ?

I'd go with:

core/lib/Drupal/Core/Theme/TemplateFinder.php

I don't think it needs an interface, I may be wrong about the interface though.