Remove deprecations related to the theme engine

That isn't a child issue, they about separate things. This is about theme engines (minus the theme engine preprocess bit that I removed in the other issue). So the .engine files and related topics.

That this is tracked as a child issue is just a bug with the clone issue feature, it's supposed to be on the same level, I'll fix that.

You can leave it with me for now. @longwave can take it if he's interested, I can't do it tonight.

If he doesn't I will take care of it.

Ok I think I got most of the remaining engine stuff including most scanning for it.

A few of things I'm not sure on:

One:
Not sure I cleaned up enough here: core/tests/Drupal/Tests/Core/Extension/ThemeExtensionListTest.php

Two:
How do we update .htaccess to remove .engine from exclude? And when?
My understanding is we keep: ThemeEngineExtensionList

Three:
I suspect we'll have test failures.

Four:
There is a bunch of stuff to review to ensure I pulled the right things.

Five:
How much do we need to update documentations since theme engines are technically extensions still, but are really tagged services of modules so they are kind of like a sub extension and in an even weirder state.
Ideally we'd deprecate ThemeEngineExtensionList and call it ThemeEngineList or something.

Six:
I think I might have taken this too far cleaning up ThemeManager: https://git.drupalcode.org/project/drupal/-/merge_requests/14894/diffs#3...
the extension variable doesn't seem to be useful anymore, I'm not sure pulling that is right though.

I think we originally said to possibly keep the extension service as a no-op thing because we couldn't deprecate it as it was still injected in places that need it. I haven't fully thought this through yet, lets see how it looks when it's green or close to.

We could see if there are some places that use it directly, but I think it's extremely rare. But I don' think renaming it makes sense. Either we leave it as barebones BC or we just drop it.

Tests like ThemeEngineExtensionListTest can imho just go, we might have introduced replacement tests in the original issue that make sure that theme engines are discovered, but not sure how easy that actually is because its tagged services and they're hard to access outside of a service. Possibly through the ThemeManager and at least indirectly by having test themes depend on test engines.

I would say theme engines are no longer extensions, no more than for example cache contexts or cache tag invalidators. They're just theme engines.

Looks like there's some install/update stuff validating theme engines that need to be updated?

That was the bit that was confusing me.

Can we remove the injections?

I think this #2973439: Make extension list services tagged services was a precursor to removing the list.

Yes I think we can stop injecting that service into ExtensionPathResolver for example as there's nothing left to resolve.

Very few real usages in contrib: https://search.tresbien.tech/search?q=extension.list.theme_engine%20-r%3.... no idea why that one module things it needs to reset this and the others looks generated lists. So hopefully committers are OK with just dropping that.

Thank you, I removed that outright, we can review if we want to do BC here, but I don't think we do.

I took a look at the failures and did a deeper look and added some todos.

The failures are coming because prefix is no longer set once we stop discovering .engine in ThemeEngineExtensionList

// Defaults to 'twig' (see self::defaults above).
      $engine = $theme->info['engine'];
      if (isset($engines[$engine])) {
        $theme->owner = $engines[$engine]->getExtensionPathname();
        // @todo this is no longer getting set.
        $theme->prefix = $engines[$engine]->getName();
      }

This no longer works, I am not sure if we need to preserve this though since I think we have it on the active theme.
This should be pretty easy to just remove and update the tests looking for it.

We also have this though: InstallerThemeEngineExtensionList which might be worth delaying for a follow up just like ThemeEngineExtensionList itself.

I think I'm going to go a little more drastic and possibly cut more than we need so we can see what fails. We can then add things back as needed.

Working on updating themeextension list, how do you autowire a string 'theme'?

I think you can provide a specific parameter in services.yml if you identify it through the key, see for example "$isSuperUserAccessEnabled", that would be one option.

On ThemeExtensionList, the only usage of ->prefix I see outside of the two explicit tests is in ThemeSettingsForm, that callback is deprecated, so I think could maybe remove both prefix and that engine form here as well? Not sure why it wasn't explicitly deprecated like owner, probably because there was no method?

Ok I kept themeEngineExtensionList
I removed the install time caching since it's no longer scanning for them.

Do we just break BC for 12 and delete the list too?

I answered my own question about the ThemeManager bit and it can be removed.

I pulled the prefix and more tests.

I also got autowiring working thanks to your suggestion.

Let's see what the tests think about this.

I pulled more tests, I left some questions in the IS, but I think we pull the extension list as a BC break.

The answer about the theme engine preprocess removal is yes, we remove it. They are no longer extensions so we don't scan them for preprocess functions.

Ok I got a few more test cases.

I am now stuck though.

The vast majority of the remaining tests are because SystemController uses owner to determine if the engine exists, but I can't seem to either autowire or use the create method to add a way to see if the service is available in the controller.
If anyone knows how we do that here I think that solves all of the remaining failures.

Tagging since this is the first removal of an extension type and there are questions.

I think this is ready for a deeper review!

The failure is random, but we can't rerun tests anymore without pushing.

Reviewed, only have some minor remarks.

I addressed all of the feedback except the formBuilder one which I just have one final question on.

Failure is random, we still can't rerun it though.
Going to set to needs review again even with the formBuilder question open.

All feedback has been addressed!

I think this is ready now. This is tagged framework manager review but IMHO that doesn't block RTBC.

Most of the early questions I think we figured out. The main thing I suppose is whether we're allowed to fully remove the extension.list.theme_engine service/class and subclasses, which we hadn't deprecated explicitly, but with the concept of theme engine extension type removed completely, there isn't much point to it.

It is very rarely used, see https://search.tresbien.tech/search?q=extension.list.theme_engine%20-r%3....

Since ThemeEngineExtensionList was explicitly marked @internal, and I can't think of any use case at all for interacting with the list of theme extensions on a site, because there are only two known ones in existence anyway, let's just go ahead here.

Our only other option really would be to add a last minute deprecation for these in 11.4, which we could still do after this is committed, but the likelihood of that helping anyone in reality is so small I don't see a reason we'd have to do the deprecation first and removal second.

Hope the above is right and someone doesn't re-open this in a week.

Committed/pushed to main, thanks!