php-ast icon indicating copy to clipboard operation
php-ast copied to clipboard

Allow extending the Node class

Open matt-allan opened this issue 3 years ago • 4 comments

This PR adds the ability to extend the Node class and parse code into your own classes. The idea is to offer a similar API to the tokenizer extension:

<?php

class MyNode extends \ast\Node
{
    public bool $myFlag = false;
}

$ast = MyNode::parseCode('<?php echo "hello, world!";', 90);

$ast->myFlag = true;

var_dump($ast);
Example output
object(MyNode)#1 (5) {
  ["kind"]=>
  int(132)
  ["flags"]=>
  int(0)
  ["lineno"]=>
  int(1)
  ["children"]=>
  array(1) {
    [0]=>
    object(MyNode)#2 (5) {
      ["kind"]=>
      int(283)
      ["flags"]=>
      int(0)
      ["lineno"]=>
      int(1)
      ["children"]=>
      array(1) {
        ["expr"]=>
        string(13) "hello, world!"
      }
      ["myFlag"]=>
      bool(false)
    }
  }
  ["myFlag"]=>
  bool(true)
}

This solves a similar problem as #217 but in a different way. Compared to dynamic properties this approach lets you add methods, constants, interfaces, etc. as well and everything can be statically analyzed properly.

This is my first major contribution to a PHP extension so please let me know if anything looks off! I based the changes on the tokenizer extension's source code.

matt-allan avatar Aug 03 '22 21:08 matt-allan

I tried using legacy arginfo to prevent the static return type from causing an error but that appears to be breaking a 'constructor throws' test. Not sure what the correct solution is here. I will try building on PHP 7.0 locally so I can see what the issue is. Seems to work fine on PHP 7.4 for me locally.

matt-allan avatar Aug 05 '22 14:08 matt-allan

@TysonAndre Thoughts on this functionality? The idea looks reasonable to me.

nikic avatar Aug 06 '22 16:08 nikic

Thoughts on this functionality? The idea looks reasonable to me.

The change itself seems reasonable once tests pass as long as the impact on the overall performance of parsing is small.


Aside: also see https://github.com/nikic/php-ast/issues/180 regarding build issues if 7.1 is an obstacle with this PR using the regular arg info EDIT: If you rebase, builds will stop being tested with 7.1 and below

TysonAndre avatar Aug 07 '22 12:08 TysonAndre

Thanks for the detailed review! I will follow up soon, I've been a bit busy starting a new job.

matt-allan avatar Aug 29 '22 14:08 matt-allan