Ensure all modules are autoloaded with PSR-4 regardless if enabled or not

I hadn't thought of this very good point.

Taking the contrib module "monolog" as an example, all of the /modules/monolog/src and /core/vendor/monolog files will get loaded by Drupal even if the monolog module is not enabled or being used.

Didn't we just recently go to a lot of work to ensure this DIDN'T happen, as it did crazy things to plugins?

@Crell: Plugin discovery used to use the actual namespaces registered to the classloader.

This is no longer the case however, plugin/annotation discovery gets a list of namespaces from the container/DrupalKernel, which in turn gets the module list from config.extension - at least that's what core does, nothing stops you putting something different than container.namespaces in a services.yml

So plugin discovery itself should be OK.

However...

@Fabianx:

D7 => D8 WTF - Using e.g. sites/all/modules/memcache/memcache.inc did just work

This didn't just work.

At least you had to do:

$conf['cache_backends'][] = 'sites/all/modules/memcache/memcache.inc';
$conf['cache_default_class'] = 'MemCacheDrupal';

Once a module starts using autoload: psr-4: { 'Drupal\\my_module\\': 'src/'}

Shouldn't they not do this?

This is a step in transitioning us to a static composer world - as the rest of the PHP world already uses.

There's no chance that 8.x will get there since we can't drop the module interface from core or the requirements that comes with.

 public function getFormId($form_arg, FormStateInterface &$form_state) {
    // If the $form_arg is the name of a class, instantiate it. Don't allow
    // arbitrary strings to be passed to the class resolver.
    if (is_string($form_arg) && class_exists($form_arg)) {
      $form_arg = $this->classResolver->getInstanceFromDefinition($form_arg);
    }

This opens up modules being able to specify classes from other modules for forms (quite a common use case), but then forget to add a dependency because it works as long as that module is in the file system. This seems like more of a gotcha than not adding autoload: psr-4 to contrib modules with a composer.json

Given #6 I think this is a task now.

Time to resurrect this issue? #2796953: [regression] Plugins extending from classes of uninstalled modules lead to fatal error during discovery would have been caught much sooner if phpunit only autoloaded classes from enabled modules (see #2797269: Improve MigrationPluginListTest).

A little late for 8.1.x...

Time has passed, new perspectives and such.

Following on a suggestion and discussion @ #3522410-7: Using instances of classes defined by modules in service container parameters causes fatal errors + #3522410-11: Using instances of classes defined by modules in service container parameters causes fatal errors

What if we did the inverse to what is currently proposed here, back to the original intent of the ticket before direction change in #2536610-6: Ensure all modules are autoloaded with PSR-4 regardless if enabled or not

Always have all modules available in autoloader.

Then, the concept of "installed" means whether a modules hooks/services/discovery/plugins etc etc is loaded.

Maybe there was more context in referenced IRC discussion, 10 years ago.

Always have all modules available in autoloader.

+1 from me as a contrib developer.

Yes as #6 calls out this will require contrib to do some work with the (hacky) use of class_exists() that has been used to determine if a module was enabled, however we now have a robust system with Drupal Rector that would allow automated updates by converting class_exists('\Drupal\some_module\some_class') to \Drupal::service('module_handler')->moduleExists('some_module') && class_exists('\Drupal\some_module\some_class')

For DX: I have encountered this in a couple scenarios related to implementing interfaces:

Scenario 1:
Decorating a core service that may not always be enabled using autowire:
The decorator has to implement the service interface, however with the core module namespace not resolvable and the design of Symfony PHP will throw a fatal error that the interface was not found. If the interface was resolvable Symfony onInvalid: ContainerInterface::IGNORE_ON_INVALID_REFERENCE should allow for the service to remain autowired yet not registered in the container.

Scenario 2 (I'm working through this with contrib modules right now)
Plugin implementing interfaces from two modules, where one is optional additional features:
Module A exists in contrib and provides a plugin system.
Module B wishes to extend the features of module A, to do this it will provide an additional interface that third party plugins will implement that provides these features.
Module C implements a Module A plugin and also wishes to implement the enhancements of Module B.

In this scenario Module B must be enabled for Module C's plugins to load Module B's interface forcing the site to use Module B even if they do not desire the extra features. As above this would be due to PHP throwing a fatal error for an unknown interface. If Module B's namespace were resolvable it need only be a composer dependency not an install dependency.

Of course there are ways around both of these, such as with use of additional meta packages, custom ServiceProvider alters, not using autowiring, etc, however these push us away from aligning with upstream support and the general PHP ecosystem.

Scenario #2 from #30 should be solved by providing two plugin classes, the second one implementing the interface from module C. If you wanted this to be automatically used by existing config specifying the first plugin, then the class could be swapped out by a plugin info alter.

Having a composer dependency on a module that may or may not be installed can introduce problems if module C isn't compatible with newer Drupal core versions - the site won't be able to update, and it also won't be able to composer remove because that would lead to the fatal error again.

Having a composer dependency on a module that may or may not be installed can introduce problems if module C isn't compatible with newer Drupal core versions -

For clarity, I believe you mean Module B.

That really is no different than having a dependency on module that can be installed that isn't compatible (you can't PMU it if its a Drupal Dependency of Module C) or a dependency on any other composer library that would conflict with Drupal Core.

If you choose to implement a dependency or even install a package on your Drupal site you need to have vetted the developer to be sure they will continue to maintain the software (or be prepared to dump them should it be required).

should be solved by ...

For brevity I did not list all the scenarios that had been considered and failed an initial architectural review.

If you choose to implement a dependency or even install a package on your Drupal site you need to have vetted the developer to be sure they will continue to maintain the software (or be prepared to dump them should it be required).

No it's not the same, because you're specifically talking about the case of a contrib developer making a deliberate choice to add a hard composer dependency on another contrib module, instead of making that dependency properly optional. If the dependency is genuinely optional, then version compatibility issues are easier to mitigate later. Saying 'it's the site owner's fault if they install a module' is not really applicable in the context of an issue against core specifically to make bad scenarios as described above more likely.

No it's not the same, because you're specifically talking about the case of a contrib developer making a deliberate choice to add a hard composer dependency on another contrib module, instead of making that dependency properly optional.

Welcome to the PHP Language. Interfaces will/must be loaded by the parser to be implemented. This is no different than being dependent upon psr/log for the interfaces it provides.

Loading the namespace into the autolaoder makes the module optional, otherwise (one of) the (possible) solution is that Module C has to make Module B (the module that provides the interface) a hard dependency. Autoloading makes it so as long as the interface is properly formatted PHP it loads (Module B would then have config options to disable its features, but now it sits taking up space and processing time in all Drupal related module functions such as the hook dispatchers, updates checker, etc)

it's the site owner's fault if they install a module

To be clear, the primary fault was directed to the developer who failed to vet their dependencies. The site owner example was thrown in as secondary responsibility.

Welcome to the PHP Language. Interfaces will/must be loaded by the parser to be implemented. This is no different than being dependent upon psr/log for the interfaces it provides.

The plugin system includes explicit support for discarding plugins that implement missing interfaces during discovery to support use cases like the one above - e.g. modules can ship plugins that will be discovered only when the module the plugin requires is installed. You're pretending as if this doesn't exist and no-one has thought about this before.

Symfony spent a good couple of years factoring out various interfaces it provides into a separate 'contracts' component so that people could use the interface without depending on the entire component, this was explicitly to avoid projects having to add dependencies on entire components to use one interface which they otherwise weren't using.

We had to workaround a PHP bug with missing traits to support attribute discovery in #3502913: Add a fallback classloader that can handle missing traits for attribute discovery, and contributed to it being fixed in PHP 8.5 https://github.com/php/php-src/issues/17959

Patronising comments about how PHP works won't cover a lack of research into the API under discussion.

Loading the namespace into the autolaoder makes the module optional

It doesn't make it optional in the code base you're arguing for the exact opposite here.

There's also an existing use case now that #30 would break.

Module A implements e.g. a field formatter plugin that should only be discovered when module B is installed, because it relies on features in in module B. Let's say it depends on a formatter base clase or trait that module B provides.

With HEAD, the field formatter that module A provides will only be discovered by the plugin system when module B is installed.

With the change here, it would be discovered whenever module B is a composer dependency. However, the base class or trait may rely on other code in module B other than the trait/base class - so actually trying to use it when module B isn't installed could result in fatal errors or it otherwise not working at all.

It also means module A would behave differently in three ways rather than two - when module B is installed, when it's required by composer but not installed, and when it's not required by composer at all.

There is no proposal here for how to provide backwards compatibility for that case or otherwise how it would be addressed.

it doesn't make it optional in the code base you're arguing for the exact opposite here.

Sorry if there is confusion around this point. I have been trying to say that yes the composer package is not optional in the code base, the Drupal Module is optional from the standpoint of Drupal core operations. (Module disabled, Drupal doesn't care that it is on disk, composer however can still access the libraries/interfaces).

There is no proposal here for how to provide backwards compatibility for that case or otherwise how it would be addressed.

Thank you for providing a technical concern that is rooted in a current logic instead of developer design choice. I don’t have an answer for that scenario at this moment and would need to take time to consider the impact.

I do wonder if this issue been acted upon sooner, or at least been in the roadmap for all new designs, might we have avoided those concerns as part of the attributes conversion. We can't change the past, and just have to push on, just want to annotate how we could possible work on additional coordination during design phases.

We had to workaround a PHP bug with missing traits to support attribute discovery in

Indeed. I believe I was monitoring a number of issues related to annotations conversions including that issue.

interfaces it provides into a separate 'contracts' component

FWIW that’s currently the leading contender for my project at the moment, though option #30-2 would IMO having been through the design evaluation also be significantly valid in this particular case.

Realistically Module B belongs inside Module A’s code base, however when maintainers don't act we look at the options.

You're pretending as if this doesn't exist and no-one has thought about this before.

There is significantly more in my designs evaluations than can be laid out here in short blurbs, especially since the majority is not directly related to this issue. I have noted previously some ideas you have suggested would work technically they failed the DX review which leads to the scenario proposed.

@cmlara can you share your full design concerns somewhere so we can evaluate?

Whether it affects the likelihood of this issue being resolved I cannot say for now but I think it would be informative to read what seems like a deep analysis as I consider issues in general.

Postponing for a few reasons:
1. The question in 38
2. How will we handle all the conflicts because this is kind of a bedrock Drupal assumption
3. What edge cases are missing for discovery based on answers in 35

I'm going to close this as works as designed for 35, 36 and 38.

If you can share more information we can explore further.

I would love to do this but there are a number of very large challenges which from my perspective are bigger than the plugin system stuff.

• composer require drupal/extension can bring in sub modules for example, webform ships with loads
• Install profiles / distributions change where you have to look for modules
• Test module affect the class loader and discovery as well - based on the $settings['extension_discovery_scan_tests'] = TRUE; setting
• Multi site also affects the class loader and discovery

Getting around all of these problems is not at simple. In order to do this I think we need to consider deprecating and then removing support for install profiles and mulit-site shipping their own modules as this would make this problem space considerably simpler.

I'm going change this to active. I think this is worthwhile to pursue but we can descope sites/all and sites/* and install profile modules - ie. still support them with the kernel adding the class loader. The only complexity will be tests but that feels surmoutable.

Prior art worth seeing: https://github.com/fenetikm/autoload-drupal

I've done quite a bit of thinking about this and how we might tackle it. Here's where I'm at:

1. We can tackle the plugin dependency issue by ensuring all the classes in the inherited change are for installed modules.
2. We need to deprecate modules in profiles and sites/all and sites/SITE_PATH and remove it before because composer can not know all this and build portable autoload files. I thought maybe we could do this later and just sort out autoloading for core extensions and extensions managed by composer BUT we have a problem. The way ExtensionDiscovery deals with precedence - ie. if you have a node module in sites/SITE_NAME and that takes precedence over the core module. I know people have used that in the past to do interesting and wrong things. But this same capability allows you to move a site to a newer version of a module than than one in modules/ so that seems like a use-case that someone might depend on. One potential way to overcome this is to change \Drupal\Core\DrupalKernel::updateModules() to prepend there autoloader but only with paths that composer cannot handle.
3. Test modules and sub module discovery complicates all of this.