marcioAlmada / yay

Yay is a high level PHP preprocessor
https://github.com/marcioAlmada/yay
MIT License
572 stars 35 forks source link

Bug in nested loops or matching a list with trailing delimiter? #30

Closed SerafimArts closed 8 years ago

SerafimArts commented 8 years ago

A little experiment for simple annotations implementation:

input:

@Any(a = 66)
@Some(a = 23, b = 42)
class TestClass {}

macro:

macro ·global ·unsafe {
    @·ns()·class(
        ·ls (
            ·chain (T_STRING·field, ·token('='), ·label()·value),
            ·token(',')
        )
        ·fields
    )
    class T_STRING·class_name
} >> {
    new class(new \ReflectionClass(T_STRING·class_name::class)) extends ·class {
        public function __construct(\ReflectionClass $context) {
            $fields = [];
            ·fields ··· {
                $this->T_STRING·field = $fields[··stringify(T_STRING·field)] = ·value;
            }
            parent::__construct($fields, $context);
        }
    }
    class T_STRING·class_name
}

output:

@Any(a = 66) // <- this is bug
new class(new \ReflectionClass(TestClass::class)) extends Some {
  public function __construct(\ReflectionClass $context) {
    $fields = [];
    $this->a = $fields['a'] = 23;
    $this->b = $fields['b'] = 42;
    parent::__construct($fields, $context);
  }
}
class TestClass {}

Try to fix bug on line 1:

macro ·global ·unsafe {
    ·ls ( // <- Add one more loop
        ·chain (
            ·token('@'), ·ns()·class,
            ·token('('),
                ·ls (
                    ·chain (T_STRING·field, ·token('='), ·label()·value),
                    ·token(',')
                ) ·annotation_arguments,
            ·token(')')
        ),
        ·token(T_WHITESPACE)
    ) ·annotations

    class T_STRING·class_name
} >> {
    ·annotations ··· {
        new class(new \ReflectionClass(T_STRING·class_name::class)) extends ·class
        {
            public function __construct(\ReflectionClass $context)
            {
                $fields = [];
                ·annotation_arguments ··· {
                    $this->T_STRING·field = $fields[··stringify(T_STRING·field)] = ·value;
                }
                parent::__construct($fields, $context);
            }
        }
    }
    class T_STRING·class_name
}

New ouput:

@Any(a = 66)
new class(new \ReflectionClass(TestClass::class)) extends Some
        {
            public function __construct(\ReflectionClass $context·cfcd208495d565ef66e7dff9f98764da)
            {
                $fields·cfcd208495d565ef66e7dff9f98764da = [];
                $this->a = $fields·cfcd208495d565ef66e7dff9f98764da['a'] = 23;
                $this->b = $fields·cfcd208495d565ef66e7dff9f98764da['b'] = 42;

                parent::__construct($fields·cfcd208495d565ef66e7dff9f98764da, $context·cfcd208495d565ef66e7dff9f98764da);
            }
        }

    class TestClass {}

wtf? o__0 Its probably https://github.com/marcioAlmada/yay/issues/20 issue?

marcioAlmada commented 8 years ago

Hi! Thanks for trying this so early. A few notes:

1 - Your first case is actually behaving right. You designed a macro that matches a single annotation before a class, so only the green part below is being matched:

- @Any(a = 66)
+ @Some(a = 23, b = 42)
+ class TestClass

2 - There seems to be a huge confusion on how ·ls() works. By default, ·ls means a list of matches separated by a delimiter, and it won't allow a trailing delimiter.:

// so the following parser:
chain(
   token('{'),
   ls(token(T_STRING)->as('letters'), token(',')) ,
   token('}')
)

// will match
{A, B, C} 

// but won't  match
{A, B, C,}
        ^ fails on the last ',' and backtracks to '{'

This means that the parser API needs more care. We certainly need a version of ·ls() that is capable to match a list with a trailing delimiter. Your use case falls on that same culprit as you have:

@Some(foo = bar) T_WHITESPACE
@Any(b = 1) T_WHITESPACE // it's a trailing delimiter! 
class Foo {}

I've just added a ·lst() parser that matches a list and possibly the trailing delimiter, while keeping the old behavior of ·ls.

3 - There are future optimiaztions being planned that could result in whitespace being elided. So in general, ·token(T_WHITESPACE) is not intended to be stable until we have a spec guaranteeing it. A suggestion would be to use ·token(';') as a list delimiter instead, resulting in a more PHP like syntax:

@Any(a = 66);
@Some(a = 23, b = 42);
class MyClass {...}

4 - On your latest attempt, indeed it's a bug. You marked the macro as ·unsafe but the macro hygiene is still renaming the variables. The issue was fixed with commit f6ef3a2.

After fixing the macro hygiene bug, I did this test macro as an example: annotations.

marcioAlmada commented 8 years ago

Thanks for reporting this!

SerafimArts commented 8 years ago

@marcioAlmada Thank you for such a detailed review!

Lack of complete documentation required study examples (thanks to them) , so I do not quite understand correctly the "entire device" as a whole =) ...not yet.

P.S. More lacks record syntax at the end or beginning of the file, or... inside methods, as example:

@Verify($a > 10);
function a($a) { ... }
// >>
function a($a) {
  assert($a > 10, 'Verify [$a > 10] failed for function ' . __FUNCTION__);
}

I dont know how implement this with yay =(

marcioAlmada commented 8 years ago

I don't know exactly what you're trying to achieve now, it seems t be some form of design by contract using annotations. Something like this would work for the specific case of @Verify:

macro {

    /* DBC macro to check for a condition before a method/function */

    @Verify\Before(···expression);

    ·optional(·repeat(·either(·token(T_PUBLIC), ·token(T_PROTECTED), ·token(T_PRIVATE), ·token(T_STATIC))))·modifiers

    function ·optional(·label())·name (···args) ·optional(·chain(·token(':'), ·ns()·type))·ret_type {
       ···body
    }

} >> {
    ·modifiers function ·name (···args) ·ret_type {
        // @Verify\Before
        assert(··unsafe(···expression), 'Verify "' . ··stringify(···expression) . '" failed for function ' . __FUNCTION__);
        ···body
    }
}

Making a more general purpose DBC framework would be a bit trickier but it's possible. For a more complex example of YAY being used you can refer to yay-enums.