NodeTraverser - support REMOVE_NODE in traverseNode

[ondrejmirtes]

There's this code in NodeTraverser: https://github.com/nikic/PHP-Parser/blob/1b479e7592812411c20c34d9ed33db3957bde66e/lib/PhpParser/NodeTraverser.php#L122-L143

There's a missing if for REMOVE_NODE case. It's valid to return REMOVE_NODE from leaveNode because it's supported in traverseArray. I think it's pretty hard to figure out in a custom node visitor whether the leaveNode method is called from traverseNode or traverseArray so it's hard to avoid the LogicException leaveNode() returned invalid value...

Is there any implementation reason why it's not supported in traverseNode? Is there any recommended workaround? Thank you.

[nikic]

What is "remove node" supposed to mean in this context? Say I have an ArrayDimFetch[$var, 'index'] node and you return REMOVE_NODE for $var ... what is that supposed to do?

This is why it's not supported.

[ondrejmirtes]

In my case it's hitting an else I want to remove:

if (true) {
} else {
}

I'm returning REMOVE_NODE when leaving the Else_. In my node visitor I'm ever returning REMOVE_NODE only for Node\Stmt. But I understand it probably cannot be generalized and not all properties that contain Node\Stmt are nullable so the current NodeTraverser implementation is probably right.

Also I realize in my case it's easily avoidable by modifying the If_ statement when leaving it.

[nikic]

I see... there really should be some way to do this. You can't replace a node with a null value, as null return value is interpreted as "do nothing". Either reusing REMOVE_NODE, or adding a new REPLACE_WITH_NULL flag or so would make sense. It might be a bit error prone, as there is no validation of the structure (no typed properties yet...)

[ondrejmirtes]

In the end this exception forced to me write a more error-prone code, which is exactly your point - we don't want null somewhere it doesn't belong :)

[nikic]

Implemented REPLACE_WITH_NULL in https://github.com/nikic/PHP-Parser/commit/afe1628a7221697726c9d677eb6c74fe394c792e.