[policy, no patch] Allow both annotations and attributes in Drupal 11

There are three main concerns for me, some of them conflict:

1. https://github.com/doctrine/annotations is already deprecated, and could go EOL, we don't know when yet. Assuming Drupal 11 is supported until 2028/29, there is a fairly high risk of us being on the hook to support/fork it, we might have to do that for Drupal 10 anyway too.

2. If we release Drupal 11 in six months, that's almost no time to complete conversions, even if we slipped to the December window it's still not that long.

3. This is compounded by contrib plugin types, since implementations of those types can't convert until the plugin manager is converted, so there is a cascade of dependencies.

I have not really thought through the implementation, but I'm wondering if we could move the annotation + annotation/attribute discovery to a contributed module for Drupal 11. This module would then have the Doctrine annotations dependency, and it would have to be added as a dependency of any non-converted module.

This would potentially allow a couple of things:
1. Contrib plugin managers that haven't converted or want to allow more time for conversions with a deprecation can add the module as a dependency.
2. The annotations module could potentially alter core discovery to re-enable annotation + attribute discovery for core plugin types, and then could be added as a dependency of any un-converted module to keep it working.

We should be able to tell via project usage how popular this module is, and this will help inform decisions when Doctrine announces an EOL.

Re #3.1 we have already forked parts of doctrine/annotations into core so we could just fork the other parts that we need and add them to 10.3, marking them @internal but letting us drop the dependency in D11.

Regarding whether we want to support annotations for core plugin types, I think that depends on how fast the rest of the conversion goes; but we already know we have quite a bit of work converting all the plugin managers, so we would need to complete all of that for 10.3, and then we are expecting contrib/custom code to convert all their plugins in a very short window to be ready for 11.0. But then again if we are extending the support cycle of 10.x until 2026 and Drupal 12.0, maybe this is still OK?

Is there a performance penalty for supporting both discovery types?

Its not just the very short support window, but that contrib would ideally be able to work with Drupal 10 and 11 to support an easier transition.

@Chi for each plugin manager that supports both types there's a penalty because you have to do essentially double the discovery - once of attributes and once for annotations.

However, if we don't do the double scanning for core plugin managers, then there's no penalty as such until a contrib plugin manager is supporting both (using classes provided by core).

Another idea, also work though, would be to move the annotations support to a module in core, so that it's possible for 11.x sites to switch it off when they're confident they've got no annotations left but to support annotations for everything by default. But also we could do that with a $settings check somewhere.

Even with rector support, converting annotations to attributes is going to be a hard ask for contrib, let alone custom stuff where the original vendors are nowhere to be found anymore.

Look at the drupal core usage metrics. The 8 to 9 and 9 to 10 upgrade paths, while impressively pulled off from a development perspective, have already left the body of real sites out there horrendous outdated and insecure. The majority of Drupal 8+ sites aren't even using secure core versions as it stands. That fact alone should be enough of a wake up call to postpone this IMHO.

> 1. https://github.com/doctrine/annotations is already deprecated, and could go EOL, we don't know when yet. Assuming Drupal 11 is supported until 2028/29, there is a fairly high risk of us being on the hook to support/fork it, we might have to do that for Drupal 10 anyway too.

Realistically, what is the risk of doing that? We're parsing PHP files of modules, what could be a security risk/attack vector? If you can alter those files that then you can already execute PHP code.

I'm not sure I understand the benefit of the module idea. That would require contrib/custom projects to depend on that module, and how would that module inject itself into a contrib plugin manager extending from the default plugin manager?

The main concern IMHO is performance, supporting both currently means we look for annotations first, second for attributes. Even if we switch it*, it would mean an extra file_get_contents() call for any class in those folders that does not have an Attribute. But as mentioned in #8, we could still remove support for annotation discovery for (specific?) core plugin types like Blocks (they have a lot of invalidations due to many derivatives, less of a concern with entity types for example) and deprecate that for D11.

The problem with switching is 🐛 AttributeClassDiscovery fails while trying to include non valid plugin class Fixed , we we are already getting first reports of discovery breaking with php fatal errors, in that case a class that neither has attribute nor annotation. Which I think we should discourage, to avoid wasting CPU/IO on classes that have nothing to parse.

I'm not sure I understand the benefit of the module idea. That would require contrib/custom projects to depend on that module, and how would that module inject itself into a contrib plugin manager extending from the default plugin manager?

It would be so that sites can switch it off. But the dependency issue makes it painful (even if we could work out implementation difficulties). I think we could achieve the same with a $settings flag or container parameter. We could even have a hook_requirements() that checks a k/v value which we set when no annotations are found any more and tells you it's time to change the settings flag.

It would be so that sites can switch it off.

Does it mean that a single contrib module that relies on annotation discovery will cause the whole site to depend on doctrine/annotations?

> It would be so that sites can switch it off.

What happens if you accidentally switch that module off and then realize that one module had not converted an entity type over to attributes? There's a pretty good chance that your side is now broken in a way that can only be recovered by fixing that.

A settings.php $settings flag I can imagine, also because I see that as an optimization for sites that know what they are doing and it is always going to be as easy to switch back as it was to turn it off.

> Does it mean that a single contrib module that relies on annotation discovery will cause the whole site to depend on doctrine/annotations?

That depends on how we do it. Your site definitely would still depend on annotation parsing. We already had to copy paste \Drupal\Component\Annotation\Doctrine\SimpleAnnotationReader and DocParser, we could do the same with any class that requires it and still remove the dependency. Those two classes already make up the most of the component that we use anyway.

We keep talking about optimization here but until we have profiled the difference between the discovery methods we are still in the dark, it might be the case that really it doesn't make a noticeable difference.

Does it mean that a single contrib module that relies on annotation discovery will cause the whole site to depend on doctrine/annotations?

Yes.

A settings.php $settings flag I can imagine, also because I see that as an optimization for sites that know what they are doing and it is always going to be as easy to switch back as it was to turn it off.

Yeah it seems easier to implement as a $settings flag, and also more useful.

Did a bit of profiling, but the current amount of plugins with just core means there isn't a lot to see.

To see anything at all, I had to disable the file cache, then AttributeDiscoveryWithAnnotations::parseClass is about 1.65ms, with 1.1ms in SimpleAnnotationReader::getClassAnnotation() on a request after creating a new block_content entity (simplest thing I could think of to trigger discovery again.

I'd suggest it makes sense to do some profiling once a big part of core is converted, and then disable plugin cache completely or something like that, so we can easily profile a lot of plugin discovery.

Once Drupal drops dependency of doctrine/annotations it should be possible to disable opcache.save_comments option. Given how intensively comments are used in Drupal codebase it should greatly reduce the memory footprint.