doctrine/reflection is abandoned

The problem with roave/better-reflection for us is that it does not extend from PHP's builtin \ReflectionClass so it's not a drop in replacement. This also goes for PHPDocumentor's reflection library. This means that we will need extend / rebuild Simple annotation to work with them. The reason we use doctrine/reflection is that it let's us get doc comments without have to load code. This results in a massive memory saving when doing test discovery and also to a lesser extent for plugin discovery.

And \Doctrine\Common\Reflection\StaticReflectionParser::$classAnnotationOptimize offers us a massive benefit because we only ever want the class annotations - so this rips out most of the code before passing it into \Doctrine\Common\Annotations\TokenParser::__construct() and then token_get_all().

Whatever we replace doctrine/reflection with will need performance testing to prove we've not regressed - especially wrt to memory usage.

May I humbly suggest just moving the entire vendor/doctrine/reflection/lib/Doctrine/Common/Reflection into core (*except one class, see below) ? I can attest it was written with Drupal in mind, it's not a lot of code, the only external dependency is the TokenParser from Annotations which I understand Drupal would still use? RuntimePublicReflectionProperty relies on Proxy as well but that class can be ommitted, it is simply not used -- I presume Doctrine might be using it elsewhere but Reflection itself does not and neither does Drupal.

Looks great, not so much of API surface

+++ b/core/lib/Drupal/Component/ClassFinder/ClassFinderInterface.php
@@ -0,0 +1,21 @@
+   * @param string $class
...
+   * @return string|null
...
+  public function findFile($class);

+++ b/core/lib/Drupal/Component/Reflection/ReflectionProviderInterface.php
@@ -0,0 +1,36 @@
+   * @return ReflectionClass
...
+  public function getReflectionClass();
...
+   * @param string $name
...
+   * @return ReflectionMethod
...
+  public function getReflectionMethod($name);
...
+   * @param string $name
...
+   * @return ReflectionProperty
...
+  public function getReflectionProperty($name);

it could use type hints

This is critical in that we're going to need to resolve it before it goes completely unsupported, but more urgent due to #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves).

I don't think we want a new component.

If we really only ever need the class annotations I wonder if we should bake-in the optimisation and remove the property and method handling?

That is very very tempting. All of this might disappear anyway once PHP 8 is our minimum and we can move to the builtin attributes - see https://stitcher.io/blog/attributes-in-php-8

@longwave pretty sure it is deliberate - it is using more that annotations.

For the 2 outstanding threads.

The licensing situation is interesting because , we need to look this up carefully, it's been long ago, but I believe at least most but potentially all the code is written by yours truly who is happy to relicense it :)

We need to do something to provide a BC layer - the Doctrine reflection stuff is used in contrib - see http://grep.xnddx.ru/search?text=Doctrine%5CCommon%5CReflection&filename=

Or we could leave this in composer.json and encourage the contrib projects to change.

Leaving it in composer.json unused sounds sensible to me, once we open 10.x and remove it there it'll be an extra prompt for contrib to stop using it, if 8.1 hasn't broken it first. Can always revisit later if there's a pressing reason to.

Is there any way to let users know that the Doctrine reflection stuff is deprecated other then a CR?

The only way would be to class_alias some classes and issue deprecation notices I think.

Thanks for the link. That's 12 people. We can always ask, if we want.

The code from doctrine/reflection has been copied to drupal/components.
The code from doctrine/reflection is no longer been used in core.
All copied code has the original license in the file.
For me it is RTBC.

If somebody can add a deprecation message to the external code with a class_alias call, then please do so.

I think we need a change record here that details any likely changes something that is using doctrine reflections in the same way core is. We also need a release note for this.

Updated the Is and created a CR.

Back to RTBC.

Committed a0a22f8 and pushed to 9.3.x. Thanks!

FYI as of Drupal core 9.3.8 I am still seeing that drupal/core requires doctrine/reflection (^1.1). Do we need to reopen this?

I think so, this only got closed due to inactivity

This was not closed due to inactivity; it was committed to HEAD well before 9.3.0 and closed due to being fixed. :)

The reason 9.3.8 (and all future Drupal 9 releases) still have a dependency on doctrine/reflection is that removing a dependency introduces a BC break. Therefore, the issue only deprecated core's dependency on the package alongside providing a replacement for it. As the release note explains, the legacy dependency is removed in Drupal 10.0.x instead.

Thanks!