2.18.x: Lexer not longer implementing ArrayAccess

[alexander-schranz]

BC Break Report

Q	A
BC Break	yes
Version	2.18.x

Summary

The oro/doctrine-extensions is using the Doctrine\ORM\Query\Lexer, with adding support for doctrine/lexer version 3 in 2.18 of doctrine/orm the Doctrine\ORM\Query\Lexer not longer implements the ArrayAccess which leads to an unexpected error.

Update: its the Token class which is not longer implementing the ArrayAccess. Maybe we need to make sure that all doctrine extensions require the correct supported lexer version before we release 2.18.0. So no changes maybe require to fix this issue in orm package.

Previous behavior

ArrayAccess was implemented by Doctrine\ORM\Query\Lexer over the doctrine/lexer:2 package. doctrine/lexer:2 was installed which Token implemented the ArrayAccess.

Current behavior

ArrayAccess is not longer implemented which ends in:

/home/runner/work/sulu/sulu/vendor/oro/doctrine-extensions/src/Oro/ORM/Query/AST/Functions/String/GroupConcat.php:46
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:3662
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:3526
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:3491
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:2273
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:1217
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:881
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:850
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:257
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query/Parser.php:357
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query.php:276
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/Query.php:288
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/AbstractQuery.php:1212
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/AbstractQuery.php:1166
/home/runner/work/sulu/sulu/vendor/doctrine/orm/src/AbstractQuery.php:913
/home/runner/work/sulu/sulu/src/Sulu/Component/Rest/ListBuilder/Doctrine/DoctrineListBuilder.php:297
/home/runner/work/sulu/sulu/src/Sulu/Bundle/ContactBundle/Controller/ContactController.php:340
/home/runner/work/sulu/sulu/src/Sulu/Bundle/ContactBundle/Controller/ContactController.php:318
/home/runner/work/sulu/sulu/src/Sulu/Bundle/ContactBundle/Controller/ContactController.php:262
/home/runner/work/sulu/sulu/vendor/symfony/http-kernel/HttpKernel.php:181
/home/runner/work/sulu/sulu/vendor/symfony/http-kernel/HttpKernel.php:76
/home/runner/work/sulu/sulu/vendor/symfony/http-kernel/Kernel.php:197
/home/runner/work/sulu/sulu/vendor/symfony/http-kernel/HttpKernelBrowser.php:65
/home/runner/work/sulu/sulu/vendor/symfony/framework-bundle/KernelBrowser.php:175
/home/runner/work/sulu/sulu/vendor/symfony/browser-kit/AbstractBrowser.php:385
/home/runner/work/sulu/sulu/src/Sulu/Bundle/TestBundle/Kernel/SuluKernelBrowser.php:57
/home/runner/work/sulu/sulu/src/Sulu/Bundle/ContactBundle/Tests/Functional/Controller/AccountControllerTest.php:1532

How to reproduce

Using GroupConcat of oro/doctrine-extensions. As they never directly u se the doctrine/lexer namespace just the doctrine/orm namespace it is unexpected that in the minor jump the Orm/Lexer not longer shipped with ArrayAccess. So we maybe need to implement the ArrayAccess for the doctrine/orm Lexer and remove it then in doctrine/orm 3 to avoid this bc break.

[greg0ire]

So we maybe need to implement the ArrayAccess for the doctrine/orm Lexer and remove it then in doctrine/orm 3 to avoid this bc break.

Ah yes, that seems fair. In #11180 (comment), @derrabus was wondering why didn't allow doctrine/lexer 3 sooner. This must be the answer. That being said, I don't think how you could implement ArrayAccess in doctrine/orm 2. There is no class to implement it on, as we use the doctrine/lexer Token class directly. I think we should probably revert.

[greg0ire]

@alexander-schranz actually, I'm not sure we should revert. I think the crash is happening here, correct: https://github.com/oroinc/doctrine-extensions/blob/2.0.3/src/Oro/ORM/Query/AST/Functions/String/GroupConcat.php#L46 ?

The class in doctrine/orm is not the one exposing the lookahead property. The class doing that is Doctrine\Common\Lexer\AbstractLexer. This probably means oro/doctrine-extensions should indeed specify a version constraint for doctrine/lexer.

[alexander-schranz]

I did a PR on the oro extensions, oroinc/doctrine-extensions#96. But think I need to also increase the required version for orm and dbal as doctrine/lexer did not exist always on older orm versions.

[alexander-schranz]

Same problem found for: martin-georgiev/postgresql-for-doctrine#177 and here alrey PR exists ScientaNL/DoctrineJsonFunctions#95

[greg0ire]

To clarify a bit further why we might have not allowed doctrine/lexer 3 before, I think we probably thought best to give a chance to everyone to notice the deprecations, since they are more helpful. In the end, it's still good to allow Lexer 3 on ORM 2, because it allows everyone to do the upgrade to ORM more progressively: they can upgrade to Lexer 3, deploy, and then upgrade to ORM 3 separately. It's a smaller upgrade.

[alexander-schranz]

I think we can close this one as it is not an issue on the doctrine side as the other packages need to define there dependencies here deeper.