Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:98624
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain kelunik.com from 81.169.146.216 cause and error)
MIME-Version: 1.0
In-Reply-To: <153a6a0f-1c22-6560-8bca-584e92915840@fleshgrinder.com>
References: <CAF+90c9vdsLuh1GAB4VPbYzJp=XiH6a5w79t5Kt=JLV6=xkdDA@mail.gmail.com>
 <16.06.40046.20A35D85@pb1.pair.com> <153a6a0f-1c22-6560-8bca-584e92915840@fleshgrinder.com>
Date: Fri, 24 Mar 2017 20:15:16 +0100
Message-ID: <CANUQDCj=nJKRwe=yiiXOCnh=Mh_3Fo0HZ+APN_FQsDTdHbPBgg@mail.gmail.com>
To: PHP Internals <internals@lists.php.net>
Cc: Andrea Faulds <ajf@ajf.me>, Nikita Popov <nikita.ppv@gmail.com>
Content-Type: multipart/alternative; boundary=94eb2c0c5c725ec960054b7ed077
Subject: Re: [PHP-DEV] Re: TOKEN_AS_OBJECT for token_get_all()
From: me@kelunik.com (Niklas Keller)

--94eb2c0c5c725ec960054b7ed077
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

2017-03-24 19:35 GMT+01:00 Fleshgrinder <php@fleshgrinder.com>:

> On 3/24/2017 4:23 PM, Andrea Faulds wrote:
> > Hi Nikita,
> >
> > Nikita Popov wrote:
> >
> >> I'd like to add a new TOKEN_AS_OBJECT flag to token_get_all(), which
> >> returns an array of PhpToken objects, rather than the mix of plain
> >> strings
> >> and arrays we currently have. The PhpToken class is defined as:
> >>
> >> class PhpToken {
> >>     public $type;
> >>     public $text;
> >>     public $line;
> >> }
> >
> > Rather than adding a flag to token_get_all() to return objects, you
> > could potentially instead make an equivalent static method on PhpToken
> > (PhpToken::getAll() perhaps). That would avoid mixing =E2=80=9Cobject-o=
riented=E2=80=9D
> > and =E2=80=9Cprocedural=E2=80=9D styles, though I don't know if it matt=
ers. It seems
> > cleaner to me.
> >
> > Thanks!
> >
>
> I am also against adding another flag because it violates the single
> responsibility principle. However, adding a `getAll` method to the
> `PhpToken` data class also seems very wrong, because it violates the
> single responsibility once again.
>
> The real and proper solution is to have an actual PHP Lexer that is
> capable of creating a token stream.
>
>     class Lexer {
>         public function __construct(string $source);
>         public static function fromFile(string $path): self;
>         public function tokenize(): TokenStream;
>     }
>

If PhpToken::getAll() violates the SRP, then Lexer::fromFile() is just the
same sort of violation.

Regards, Niklas


>     final class TokenStream implements IteratorAggregate {
>         public function getIterator(): Generator<Token>
>         public function toArray(): Token[]
>     }
>
>     final class Token {
>         // Ideally this would be an enum, but...
>         public const OPEN_TAG;
>         public const CLOSE_TAG;
>         // ...
>
>         // I hope these are not mutable!
>         public int $category;  // type
>         public string $lexeme; // text
>         public int $line;
>         public int $column;    // we don't have that :(
>
>         /** @see token_name */
>         public function getCategoryName(): string;
>
>         // We could add `is*` methods for the various categories here.
>         public function isOpenTag(): bool;
>         public function isCloseTag(): bool;
>         // ...
>     }
>
> With this in place, we're ready for the future. The `TokenStream` can
> easily use a generator over the internal array. Or we offer the
> `toArray` method only at the beginning. Users will have to call it, but
> that overhead is tiny, considering that we can extend upon it in the
> future without introducing any kind of breaking change.
>
> Obviously this could go into the namespace `PHP\Parser`, or we prefix
> everything with `PHP` (I'd be for the former).
>
> On a side note, going for `Php` instead of `PHP` is inconsistent with
> the naming that is currently dominating the PHP core:
>
> https://secure.php.net/manual/en/indexes.functions.php
>
> There are already some things that violate it, e.g. `Phar` instead of
> `PHAR`, but most things keep acronyms intact (`DOM`, `XML`, `PDO`, ...).
> Introducing even more inconsistency to PHP seems like a very bad idea to
> me. I know that this is considered bikeshedding by many people, but
> consistency is very important and we are already lacking it on too many
> levels as it is.
>
> --
> Richard "Fleshgrinder" Fussenegger
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--94eb2c0c5c725ec960054b7ed077--