[PP-1] Make doctrine/lexer:^3.0 compatible with \Drupal\Component\Annotation\Doctrine.

Postponed on 🐛 Annotation component has an undeclared dependency on doctrine/lexer 2 RTBC

still would be great to have this compatibility in 11.2

DocParser was written for Lexer 1 when token was an array, 2 added an object with ArrayAccess support and 3 dropped this support finalizing the move to object. "Luckily" the class is final so there is no easy way to add ArrayAccess back for the short time annotations are still supported before everything becomes an attribute. (I will crosslink this issue from the @final issue as a textbook example of why final is bad.) Also, why they dropped ArrayAccess I can't even imagine, it was four one line methods.

Recommendation: do not upgrade. Let this be until it's removed.

If that's not desirable I guess I can write a rector rule to convert every array access to an object access unless the variable is called $metadata and pray it works.

I agree upstream is hostile, every time we've raised anything like this it's been immediately won't fixed.

Two concerns with not upgrading though:

This will still be a dependency in Drupal 12, which we will be supporting until some time in 2030, so if there is a single security release in the next five years it will be annoying. But there is not really security surface area here, and if necessary we could fork.

Similarly, do we think this will support every PHP version released between now and at least 2028 or 2029? Given we hope nothing will actually be using annotation parsing at that point, PHP deprecations would probably be OK but a hard break would not.

2 added an object with ArrayAccess support and 3 dropped this support finalizing the move to object.

Given we control \Drupal\Component\Annotation\Doctrine\DocParser and it's already final, can we make it compatible with the object way of doing things, and allow ^2 || ^3 in composer.json, but stay pinned to ^2 (for now) in core-recommended? This adds forward compatibility for anyone who wants to upgrade for other reasons, but stays conservative otherwise - but also opens the door to finishing the upgrade in Drupal 12 if we want to. If the token arrays/objects are not leaked outside of the implementation this seems like it might be workable.

> can we make it compatible with the object way of doing things,

It's Turing complete, nothing stops you :P

Can is not the problem, want is. It's a bit painful to do it.

It passes tests, but I'm not sure how much we exercise the code any more, given we have few annotations left in core...

Looks it should be simple rector rule as there's only value, type and position are actually used by core as test-only MR showing all is compatible

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

rebased and cleaned up the drupal/core-recipe-unpack

Seems pretty straight forward, don't see a reason no

Needs another rebase.

rebased

The composer lock test fail on PHP 8.3 seems a legit one.

sorry, should be solved now, messed in conflict resolution

back to rtbc

Committed/pushed to 11.x, thanks!

This will need a backport MR for 11.2.x, although I also don't think it hurts if it's 11.3.x-only?

catch → closed merge request !12087

created MR for backport

Seems like a good 11.2 backport.

Thought about this some more and discussed with @longwave. If we were going to backport this to 10.x then it would mitigate against a Doctrine 2.x security release, but that seems unlikely, at least not in 10.5.x, and also there is not really much security surface with this code. So let's leave it in 11.3.x - moving back to fixed.

andypost → closed merge request !12303

andypost → closed merge request !12088

Automatically closed - issue fixed for 2 weeks with no activity.