Introduce ExtensionTypeInterface with public constants for extension types

There's more extensions possible

$ find . -name '*.engine' -print
./core/themes/engines/twig/twig.engine
./core/modules/system/tests/themes/engines/nyan_cat/nyan_cat.engine
./core/modules/system/tests/fixtures/HtaccessTest/access_test.engine

Using PHP 8.1 feature https://www.php.net/manual/en/language.enumerations.backed.php this enums can give file finder pattern to extension.list claases

A sort of related thought I've had for a while: why are theme engines a separate extension type? Can't they just be modules that provide a theme engine service? Worth opening a separate issue to discuss this?

Re #7 - nice idea! but I bet we can't have more then one engine enabled same time

I have been looking at replacing usages of 'module', ... with ExtensionType::Module, using backed enumerations. They are not a drop-in replacement, like a constant would be, they are causing errors when used in string context (like array keys).

One potential solution is to use ExtensionType::Module->value in those cases. I am not sure how ergonomic that is. Perhaps we should create an ExtensionTypeInterface with public constants instead?

One potential solution is to use ExtensionType::Module->value in those cases. I am not sure how ergonomic that is. Perhaps we should create an ExtensionTypeInterface with public constants instead?

Yeah feels nicer than ->value

The MR implements a new interface that contains public constants, as suggested in #10 and agreed on #11.

Looks good, there's a couple of place in DrupalKernel and ConfigImporter we should update too in my book

I replaced strings by constants in the commit above for DrupalKernel and ConfigImporter.

I noticed that the core folder has +5k uses of 'module' alone. Is the expectation to replace them all with the newly introduced constants? Or are we good with the current patch?

grep --recursive "'module'" ./web/core |wc --lines

5810

Wearing my Update Manager maintainer hat, I just tried to port that core module to use this. Mostly pretty simple. However, Update Manager has a project_type used internally that handles 'module' and 'theme' (for which this constant is nice), but it also has to deal with these options:

     'module-disabled' => t('Uninstalled modules'),
     'theme-disabled' => t('Uninstalled themes'),

Those obviously don't make sense for this new enum/constant. So we're left with code that has to do a mix of the const + hard coded strings, anyway, e.g.:

  $project_types = [
    'core' => t('Drupal core'),
    ExtensionTypeInterface::MODULE => t('Modules'),
    ExtensionTypeInterface::THEME => t('Themes'),
    'module-disabled' => t('Uninstalled modules'),
    'theme-disabled' => t('Uninstalled themes'),
  ];

Not the end of the world, but not totally beautiful, either. 😅

Maybe we don't want to bother, and we should revert the commit I just pushed. But it's a start at cutting into the remaining 5810 instances of 'module', in a core module that really cares about extension types...

I think we should aim to convert things that deal with extension types, so config importer and the kernel make sense, as does update module.

But no, I don't think we need to convert all 5k of them

More accurate title now that we've moved on from a PHP enum for this.

Can it be documented the scope of this? As mentioned there are tons of instances of 'module', 'theme', and 'profile'

Should instances like

$config_importer->getExtensionChangelist('module', 'install');

be updated to use the constants?

There are enough instances think it would be good idea of limit to specific sections of the repo at first and maybe open follow ups for other sections?

Yeah, that's a legitimate request. 😅

Good news from greplandia. Although we've got ~5500 remaining hits for 'module', 4794 of those are from fixtures (mostly for migrate tests), and we're definitely not going to change those. So there's really only ~730 real hits left to consider the scope of, in ~290 unique files. If you filter out tests (or at least filenames that have 'test' in them), we're down to only ~120 files to consider.

Very quick attempt to analyze the results. Some big buckets:

1. Extension list scanning
2. Adding dependencies
3. Tests
4. Doc comments
5. Other

Does that seem like a somewhat reasonable scope for the follow-ups?

There's really not *that* many places that are using 'module' or 'theme' for extension list scanning. Maybe better to port those here as part of this initial issue. Pushed a few more commits for that. If I have more time today I'll keep going.

I basically did this:

for i in `cat 3350946-21.module-string-find-no-fixture-files-only.txt`
 perl -pi.bak -e "s/'module'/ExtensionTypeInterface::MODULE/g" $i

It hit a bunch of docs we probably don't want to change. It didn't insert the necessary use statements to the changed files, etc. This isn't going to work at all. 😅 But I'm posting here as a do-not-test patch (instead of pushing anything to the MR) if folks wanna see what happens if we basically do the proposed change to all of core.

IMHO, this isn't an obvious win in some subsystems, which is making me wonder if this is worth doing at all. Is 'module' vs. 'theme' in our code really a "problem" we need to solve? 😉

Based on Slack between @larowlan, @mherchel, @e0ipso and myself, we agreed this shouldn't block SDC. I opened 📌 [PP-2] Port SDC to use ExtensionTypeInterface Postponed as a child issue of this, so we can unblock SDC while we sort out the scope here. See #3340712-123: Add Single Directory Components as a new experimental module → for more.

I suspect we're going to need a different Interface for things like config dependency types, etc, so there will probably be other child issues here.

RE #10 Enum vs Interface::Constants (sorry for being late to the parts):
- If we create interface constants, then extension type will always be a string. No leveraging of type safety and static analysis.
- If we create enums, we CAN not only replace string typed values with the new constants (by using 'ExtensionType::Module->value'), but ALSO leverage type safety and static analysis for NEW APIs:

  public function getInstances(ExtensionType $extensionType) {...}
  ...
  $instances = $service->getInstance(ExtensionType::Module);

Just to make sure the consequences of the decision are clear to everyone.

(And: Yes, i'm a PHP8 typesafety fanboy.)