doctrine / lexer

Base library for a lexer that can be used in Top-Down, Recursive Descent Parsers.
https://www.doctrine-project.org/projects/lexer.html
MIT License
11.05k stars 60 forks source link

Replace object-like array with class #83

Closed greg0ire closed 1 year ago

greg0ire commented 1 year ago

Alternative to https://github.com/doctrine/lexer/pull/79 (which I still prefer)

The new class is templatable, which should enable us to specify that the ORM Lexer is an Lexer<self::T_*>, and in the future, define an enum called TokenType in the ORM, and switch to Lexer<TokenType>.

I tested on the ORM, and with this solution, the PHPUnit test suite no longer breaks.

The static analysis is broken because of the impossibility to know if we have an array or an object, and because I want to reuse the Token name.

I've considered adding another template param to AbstractLexer to fix this, but then it would have to be removed in 2.0, which would make implementing compatibility with 2.0 challenging from the SA point of view.

malarzm commented 1 year ago

The static analysis is broken because of the impossibility to know if we have an array or an object

Dang, I thought we'd be able to leverage https://psalm.dev/docs/annotating_code/type_syntax/conditional_types/ to know what we're dealing with, but I wrongly assumed we'd be able to check feature flag :/

SenseException commented 1 year ago

I would expect the ArrayAccess usage of Token here without the deprecation messages. Wouldn't that at least keep some of the old code?

greg0ire commented 1 year ago

Closing in favor of the more sensible #79