hoaproject/Regex

Immutable nodes + DSL-specific nodes

unkind opened this issue · 5 comments

Hi,

After trying to reassemble regex with some modifications in AST, I suggest to add in TODO list 2 things:

  1. Make DSL-specific nodes, e.g. NamedCapturingGroupNode.
  2. Make nodes immutable.

Why?

  1. In order to change AST, I'd need just create new node which already has all node-specific fields in the constructor, quite easy.
  2. API of nodes is cleaner: fewer methods.
  3. You don't have to think about "modify existing one or create new one?". Modifying existing is dangerous as you can forget to update something. Creating new one is hard when Node is just generic class.

I compare my DX with PHP-Parser's Traverser's.

Mutable nodes also are weird from semantical point of view. Changed regex is different regex, it's a value object. Mutable VO makes no sense.

However, I'd say this library is very unique for PHP, I didn't find working analogues (even considering fact that this lib doesn't support full PCRE spec, e.g. (*VERB)). The library is cool anyway.

Hywan commented

Hello :-),

Thanks for the suggestion. The library relies on hoa/compiler and the latter does not support specific node, but only a generic tree nodes. It is possible to map the tree to a set of specific nodes though, so there is no technical limitation here. And in the same spirit, those specific nodes can be immutable.

We accept any PR. This library is not urgent and we don't focus that much right now on it.

Yes, the grammar does not support the entire PCRE specification. Feel free to open a bug when a construction/syntax is missing and we will be glad to implement it!

The library relies on hoa/compiler and the latter does not support specific node, but only a generic tree nodes.

What's the point in interface Element in Visit interface? :)

interface Visit
{
    /**
     * Visit an element.
     *
     * @param   \Hoa\Visitor\Element  $element <...> ← This parameter always receives `TreeNode`?
     * @param   mixed                 &$handle    Handle (reference).
     * @param   mixed                 $eldnah     Handle (not reference).
     * @return  mixed
     */
    public function visit(Element $element, &$handle = null, $eldnah = null);
}
Hywan commented

Because hoa/visitor defines this method with this signature :-).

Hywan commented

And hoa/visitor is generic over Element. It is used by other libraries than hoa/compiler.

OK, I got you, created #37 for (*VERB).

P.S. Maybe you have 2 different visitors (interfaces) then. Different bounded contexts, different visitors. If model is too generic, it becomes painful for all users of this model. Models can be very similar (for example, GeoPoint and 2dPoint; Point abstraction for both won't help anyone), but semantically different.