Re: Re: [RFC] token_get_all() TOKEN_AS_OBJECT mode

On Tue, Feb 25, 2020 at 7:16 PM Theodore Brown <theodorejb@outlook.com> wrote: > On Tue, Feb 25, 2020 at 9:59 AM Nikita Popov <nikita.ppv@gmail.com> wrote: > > > On Thu, Feb 13, 2020 at 10:47 AM Nikita Popov <nikita.ppv@gmail.com> > wrote: > > > > > This has been discussed a while ago already, now as a proper proposal: > > > https://wiki.php.net/rfc/token_as_object > > > > > > An open question is whether (at least to start with) PhpToken should be > > > just a data container, or whether we want to add some helper methods > to it. > > > If this generates too much bikeshed, I'll drop methods from the > proposal. > > > > I think this proposal is in a pretty decent shape now, and I'd like to > move > > it to voting soon. The only remaining open question is whether we want to > > add any additional predefined methods. As the class can now be extended, > > every library can add their own methods, but there might still be value > in > > providing some things by default, primarily for performance reason. For > > example, the proposed is() method can be a good bit more efficient when > > implemented directly in extension code. > > > > Any feedback on this point? > > Hi Nikita, > > Thanks for this RFC. The proposed PhpToken class will > definitely make it > easier to work with parsed tokens (I struggled with this when working on > the migration script for deprecated alternate array/string offset syntax). > > In regards to the open question of additional methods, the > getTokenName() > method would be very welcome. isIgnorable() would also be > helpful > sometimes, though I don't care much one way or another if it's added. > > But I'm really skeptical about the value of the is($kind) > method. Code > working with tokens should know the type of value being checked (ID int, > text content, or array of one or the other), and a method that accepts > any of three types can make it harder to understand what's going on. > > For example, the proposed isIgnorable() method doesn't > need to use > is(), it could just check whether the > id property is in the array > of token constants. > To provide some context, a method like is() is primarily useful in the implementation of other methods accepting a token-descriptor to search for. Token stream implementation commonly have many helper methods along the lines of public function findRight($pos, $findTokenType) { $tokens = $this->tokens; for ($count = \count($tokens); $pos < $count; $pos++) { if ($tokens[$pos]->is($findTokenType)) { return $pos; } } return -1; } which search for tokens in different directions, skip tokens, check that tokens exist while skipping whitespace etc. Having a fast primitive to perform the token type check would be quite useful for that, I think. Regards, Nikita