regexp-tree: traverse/transform: path.remove() or any other way to change the collection does not work

Hi,

Changing the parent collection (e.g. expressions) in a traverse() or transform() has an effect on the traversion. E.g. the next node will be skipped if I use this (in coffeescript):

    RegExpTree.traverse(ast, {

      Char: (path) ->
        {node, parent, property, index} = path
        if parent.type == "Alternative"
          if node.value == " " or node.value == "\n"
            path.remove()
      }
    )

I assume, the index is used in the iteration and that changes when removing the item from the collection.

Perhaps I am doing it wrong? but I assume the remove() of NodePath is for exactly this purpose. What is the preferred way to delete a node inside traverse()?

As a workaround I tried to set node.type = null or to an undefined type name, but this gives me an error. Would be nice if traverse() and generate() could skip such a node. Also optimize could remove such nodes.

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 36 (36 by maintainers)

Most upvoted comments

nope, these lines don’t trigger the problem:

    // Remove 'b' itself.
    path.remove();
    // Remove 'c' as well.
    cPath.remove();

because after deleting both nodes from the array processing is finished, so the wrong index in ast-traverse doesn’t matter any more. The problem only appears when you delete a node and return to ast-traverse and want to process a next node. So, you should change it to:

    if(value == "b")
        path.remove();
    else if(value == "c")
        path.remove();

question is how to check expectations. You already have a second traverse which checks if “b” and “c” are removed. This may be a matter of taste, but IMHO, all this code hides the intention of the test. Many things are already tested in another test. The simplest check would use generate() and expect a result. Or if you do not want to use generate() you could check values of AST nodes instead, e.g. you could look for “a” and “d” at the expected positions.

hg42 on Apr 6, 2017