JavaScript: Fix compactChildNodes dropping node list fields

[marcoroth]

In the JavaScript bindings compactChildNodes() is generated two ways. When a node has at least one OptionalNodeField it builds up a compact array, otherwise it returns an array literal.

The literal branch spreads node lists correctly, but the compact branch used concat and threw the result away:

compactChildNodes() {
  const compact = [];

  compact.concat(this.conditions); // <- return value discarded

  if (this.statements) {
    compact.push(this.statements);
  }

  return compact;
}

BasicVisitor#visitChildNodes walks compactChildNodes(), so a Visitor silently never descends into those children:

class Collector extends Visitor {
  found = []

  visitInstanceVariableReadNode(node) { 
    this.found.push(node.name) 
  }
}

collect("if a\n  @foo\nend")            // => ["@foo"]
collect("case a\nwhen 1\n  @foo\nend")  // => []
collect("@foo, @bar = 1, 2")            // => []
collect("def call(a = @foo); end")      // => []

This affects 17 fields across 11 node types:

• ArrayPatternNode
• BlockParametersNode
• CaseMatchNode
• CaseNode
• FindPatternNode
• HashPatternNode
• MultiTargetNode
• MultiWriteNode
• ParametersNode
• RescueNode and
• WhenNode

Nodes without an optional field take the literal branch and are fine, which is why StatementsNode works and the problem is easy to miss.

[Earlopain]

I think this was closed by accident?

[marcoroth]

lol whoops, I didn't realize it would close the issue here in ruby/prism. Thanks for the heads up @Earlopain! 🙏🏼

[Earlopain]

Thanks! Would have worked with ruby semantics 😉