On 4/26/2016 10:38 AM, Alexander Lisachenko wrote:
> Hello, Nikita!
> 
> It's a great news! Having this stuff in the 7.1 core is awesome!
> 
> But I have several petitions for this API:
> 1) Could it avoid using of raw functions for parsing API, like
> ast\parse_file or ast\parse_code
> 2) Could you please put all the parsing stuff into the `Parser` class and
> put it into the `Php\` namespace, as well with node classes?
> 

The current coding standard still prescribes to do so. However, these
requests keep popping up and I think that this should be discussed at
some point. However, putting everything into a static class just to
mimic some OO that you might like makes absolutely no sense. We have
multi-paradigm support for a reason: best tool for the job?

That being said, the current implementation violates more rules of the
coding standard (camelCase vs. snake_case).

Currently we have the following:

  namespace ast {

    parse_file()

    parse_code()

    get_kind_name()

    kind_uses_flags()

    class Node {
      public $kind;
      public $flags;
      public $lineno;
      public $children;
    }

  }

  namespace ast\Node {

    class Decl extends ast\Node {
      public $endLineno;
      public $name;
      public $docComment;
    }

  }

From a pure coding standards view and current conventions in core at
least the following has to change:

  ast_parse_file()

  ast_parse_code()

  ast_get_kind_name() // ast_kind_name?

  ast_kind_uses_flags() // ast_kind_has_flags?

  class AstNode {
    public $kind;
    public $flags;
    public $lineno;
    public $children;
  }

  class AstDecl extends AstNode {
    public $end_lineno;
    public $name;
    public $doc_comment;
  }

Of course usage of the \Php\Ast or \PHP\AST or \Php\AST or \PHP\Ast
namespaces is possible but you already see from the four variations that
this opens a huge can of worms that would require discussion.

Personally I would design the API as follows, given that it is not a
goal to have an anemic model in order to support direct changes of the
public properties without any kind of validation. If that is an intended
functionality things might need to be implemented a bit differently (and
the anemic DTO approach could even make sense). Also note that I am
modelling it in an OO way because it fits the actual data structure (tree).

  enum AstKind {

    /*API = [ ORDINAL, NAME ]*/

    FUNCTION = [ 66, 'AST_FUNC_DECL' ];

    CLASS = [ 67, 'AST_CLASS' ];

    // ...

    /** @see \AstKind::getName() */
    public function __toString(): string;

    /** bitfield */
    public function getFlags(): int;

    public function getName(): string;

    public function getOrdinal(): int;

    public function hasFlags(): bool;

  }

  final class AstNode implements Countable, Traversable {

    private AstKind $kind;

    private int $line_number;

    private array<AstNode> $children = [];

    public function getKind(): AstKind;

    public function getLineNumber(): int;

  }

  final class AstDeclarationNode extends AstNode {

    private string $doc_comment;

    private int $end_line_number;

    private string $name;

    /** @see \AstDeclarationNode::getName() */
    public function __toString(): string;

    public function getDocComment(): string;

    public function getEndLineNumber(): int;

    public function getName(): string;

  }

  final class Ast implements Countable, Traversable {

    private AstNode $root;

    private function __construct();

    public static function fromFile(string $path): Ast;

    public static function fromCode(
      string $code,
      string $filename = 'string code'
    ): Ast;

    /** @see \Ast::getDump() */
    public function __toString(): string;

    public function getDump(bool $line_numbers = false): string;

  }

I do not think that it is necessary to use the exact same constant names
as are used in the C source (e.g. AST_FUNC_DECL vs. AST_CLASS). The
userland API could be more consistent here. You might disagree thought
but in the end only the ordinal must be an absolute perfect match.

-- 
Richard "Fleshgrinder" Fussenegger

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to