JS-230 Java ESTree AST should correctly build array expressions and p…

…atterns with empty elements (#4753)

packages/jsts/src/parsers/ast.ts

@@ -364,10 +364,9 @@ export function visitNode(node: estree.BaseNodeWithoutComments | undefined | nul
   }
 
   function visitArrayPattern(node: estree.ArrayPattern) {
-    // If the elements ever contain null together with other nodes, protobuf will fail to serialize the array.
-    // It is unclear from the estree documentation if this is possible, but as it is from a type perspective, we prefer to be safe.
+    // When an entry is empty, it is represented as null in the array.
     return {
-      elements: node.elements.filter(e => e !== null).map(visitNode),
+      elements: node.elements.map(visitArrayElement),
     };
   }
 
@@ -524,10 +523,17 @@ export function visitNode(node: estree.BaseNodeWithoutComments | undefined | nul
   }
 
   function visitArrayExpression(node: estree.ArrayExpression) {
-    // If the elements ever contain null together with other nodes, protobuf will fail to serialize the array.
-    // It is unclear from the estree documentation if this is possible, but as it is from a type perspective, we prefer to be safe.
+    // When an entry is empty, it is represented as null in the array.
     return {
-      elements: node.elements.filter(e => e !== null).map(visitNode),
+      elements: node.elements.map(visitArrayElement),
+    };
+  }
+
+  function visitArrayElement(
+    element: estree.Pattern | estree.Expression | estree.SpreadElement | null,
+  ) {
+    return {
+      element: visitNode(element),
     };
   }
 

packages/jsts/src/parsers/estree.proto

@@ -239,7 +239,7 @@ message RestElement {
   Node argument = 1;
 }
 message ArrayPattern {
-  repeated Node elements = 1;
+  repeated ArrayElement elements = 1;
 }
 message ObjectPattern {
   repeated Node properties = 1;
@@ -332,7 +332,10 @@ message ArrowFunctionExpression {
   optional bool async = 5;
 }
 message ArrayExpression {
-  repeated Node elements = 1;
+  repeated ArrayElement elements = 1;
+}
+message ArrayElement {
+  optional Node element = 1;
 }
 message ClassDeclaration {
   optional Node id = 1;

packages/jsts/tests/parsers/fixtures/ast/base.js

@@ -338,6 +338,8 @@ const [a1, b1] = [1, 2];
 const [a2, ...b2] = [1, 2, 3];
 const arr1 = [1, 2, 3];
 const arr2 = [...arr1, 4, 5];
+const arrWithEmptyElements = [, , 5];
+const [, pattern, withEmpty, , elements] = [1, 2, 3, 4, 5, 6]
 
 let a3;
 a3 = 1, 2, 3;

sonar-plugin/api/src/main/java/org/sonar/plugins/javascript/api/estree/ESTree.java

@@ -86,8 +86,8 @@ public sealed interface DirectiveOrModuleDeclarationOrStatement extends Node {}
   public sealed interface ExpressionOrPattern extends Node {}
   public sealed interface IdentifierOrLiteral extends Node {}
 
-  public record ArrayExpression(Location loc, List<ExpressionOrSpreadElement> elements) implements Expression {}
-  public record ArrayPattern(Location loc, List<Pattern> elements) implements Pattern {}
+  public record ArrayExpression(Location loc, List<Optional<ExpressionOrSpreadElement>> elements) implements Expression {}
+  public record ArrayPattern(Location loc, List<Optional<Pattern>> elements) implements Pattern {}
   public record ArrowFunctionExpression(Location loc, boolean expression, BlockStatementOrExpression body, List<Pattern> params, boolean generator, boolean async) implements Expression {}
   public record AssignmentExpression(Location loc, AssignmentOperator operator, MemberExpressionOrPattern left, Expression right) implements Expression {}
   public record AssignmentPattern(Location loc, Pattern left, Expression right) implements Pattern {}

sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/ESTreeFactory.java

@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Optional;
 import org.sonar.plugins.javascript.api.estree.ESTree;
+import org.sonar.plugins.javascript.bridge.protobuf.ArrayElement;
 import org.sonar.plugins.javascript.bridge.protobuf.ArrayExpression;
 import org.sonar.plugins.javascript.bridge.protobuf.ArrayPattern;
 import org.sonar.plugins.javascript.bridge.protobuf.ArrowFunctionExpression;
@@ -311,7 +312,11 @@ private static ESTree.RestElement fromRestElementType(Node node) {
   private static ESTree.ArrayPattern fromArrayPatternType(Node node) {
     ArrayPattern arrayPattern = node.getArrayPattern();
     return new ESTree.ArrayPattern(fromLocation(node.getLoc()),
-      from(arrayPattern.getElementsList(), ESTree.Pattern.class));
+      arrayPattern.getElementsList().stream().map(ESTreeFactory::fromArrayPatternElement).toList());
+  }
+
+  private static Optional<ESTree.Pattern> fromArrayPatternElement(ArrayElement element) {
+    return element.hasElement() ? Optional.of(from(element.getElement(), ESTree.Pattern.class)) : Optional.empty();
   }
 
   private static ESTree.ObjectPattern fromObjectPatternType(Node node) {
@@ -467,7 +472,11 @@ private static ESTree.ArrowFunctionExpression fromArrowFunctionExpressionType(No
   private static ESTree.ArrayExpression fromArrayExpressionType(Node node) {
     ArrayExpression arrayExpression = node.getArrayExpression();
     return new ESTree.ArrayExpression(fromLocation(node.getLoc()),
-      from(arrayExpression.getElementsList(), ESTree.ExpressionOrSpreadElement.class));
+      arrayExpression.getElementsList().stream().map(ESTreeFactory::fromArrayExpressionElement).toList());
+  }
+
+  private static Optional<ESTree.ExpressionOrSpreadElement> fromArrayExpressionElement(ArrayElement element) {
+    return element.hasElement() ? Optional.of(from(element.getElement(), ESTree.ExpressionOrSpreadElement.class)) : Optional.empty();
   }
 
   private static ESTree.ClassDeclaration fromClassDeclarationType(Node node) {

sonar-plugin/bridge/src/main/protobuf/estree.proto

@@ -239,7 +239,7 @@ message RestElement {
   Node argument = 1;
 }
 message ArrayPattern {
-  repeated Node elements = 1;
+  repeated ArrayElement elements = 1;
 }
 message ObjectPattern {
   repeated Node properties = 1;
@@ -332,7 +332,10 @@ message ArrowFunctionExpression {
   optional bool async = 5;
 }
 message ArrayExpression {
-  repeated Node elements = 1;
+  repeated ArrayElement elements = 1;
+}
+message ArrayElement {
+  optional Node element = 1;
 }
 message ClassDeclaration {
   optional Node id = 1;

sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/ESTreeFactoryTest.java

@@ -24,8 +24,12 @@
 import java.io.IOException;
 import java.math.BigInteger;
 import java.nio.file.Path;
+import java.util.Optional;
 import org.junit.jupiter.api.Test;
 import org.sonar.plugins.javascript.api.estree.ESTree;
+import org.sonar.plugins.javascript.bridge.protobuf.ArrayElement;
+import org.sonar.plugins.javascript.bridge.protobuf.ArrayExpression;
+import org.sonar.plugins.javascript.bridge.protobuf.ArrayPattern;
 import org.sonar.plugins.javascript.bridge.protobuf.AssignmentExpression;
 import org.sonar.plugins.javascript.bridge.protobuf.AssignmentPattern;
 import org.sonar.plugins.javascript.bridge.protobuf.BinaryExpression;
@@ -71,7 +75,7 @@ void should_create_nodes_from_serialized_data() throws IOException {
     ESTree.Node root = ESTreeFactory.from(node, ESTree.Node.class);
     assertThat(root).isInstanceOf(ESTree.Program.class);
     ESTree.Program program = (ESTree.Program) root;
-    assertThat(program.body()).hasSize(52);
+    assertThat(program.body()).hasSize(55);
     // Assert a few nodes.
     assertThat(program.body().get(0)).isInstanceOfSatisfying(ESTree.VariableDeclaration.class, variableDeclaration -> {
       assertThat(variableDeclaration.declarations()).hasSize(1);
@@ -90,6 +94,18 @@ void should_create_nodes_from_serialized_data() throws IOException {
       assertThat(ifStatement.consequent()).isInstanceOf(ESTree.BlockStatement.class);
       assertThat(ifStatement.alternate()).isEmpty();
     });
+
+    // Source code: [, , 5]
+    assertThat(program.body().get(42)).isInstanceOfSatisfying(ESTree.VariableDeclaration.class, declaration -> {
+      Optional<ESTree.Expression> initializer = declaration.declarations().get(0).init();
+      assertThat(initializer).isPresent();
+      assertThat(initializer.get()).isInstanceOfSatisfying(ESTree.ArrayExpression.class, arrayExpression -> {
+        assertThat(arrayExpression.elements()).hasSize(3);
+        assertThat(arrayExpression.elements().get(0)).isEmpty();
+        assertThat(arrayExpression.elements().get(1)).isEmpty();
+        assertThat(arrayExpression.elements().get(2)).isNotEmpty();
+      });
+    });
   }
 
   @Test
@@ -641,6 +657,46 @@ void directive_can_be_in_block_statement() {
     });
   }
 
+  @Test
+  void array_expression_can_contain_empty_elements() {
+    Node protobufNode = Node.newBuilder()
+      .setType(NodeType.ArrayExpressionType)
+      .setArrayExpression(ArrayExpression.newBuilder()
+        .addElements(ArrayElement.newBuilder().build())
+        .addElements(ArrayElement.newBuilder().setElement(Node.newBuilder().setType(NodeType.ThisExpressionType).build()).build())
+        .addElements(ArrayElement.newBuilder().build())
+        .build())
+      .build();
+
+    ESTree.Node estree = ESTreeFactory.from(protobufNode, ESTree.Node.class);
+    assertThat(estree).isInstanceOfSatisfying(ESTree.ArrayExpression.class, array -> {
+      assertThat(array.elements()).hasSize(3);
+      assertThat(array.elements().get(0)).isEmpty();
+      assertThat(array.elements().get(1).get()).isInstanceOf(ESTree.ThisExpression.class);
+      assertThat(array.elements().get(2)).isEmpty();
+    });
+  }
+
+  @Test
+  void array_pattern_can_contain_empty_elements() {
+    Node protobufNode = Node.newBuilder()
+      .setType(NodeType.ArrayPatternType)
+      .setArrayPattern(ArrayPattern.newBuilder()
+        .addElements(ArrayElement.newBuilder().build())
+        .addElements(ArrayElement.newBuilder().setElement(Node.newBuilder().setType(NodeType.IdentifierType).build()).build())
+        .addElements(ArrayElement.newBuilder().build())
+        .build())
+      .build();
+
+    ESTree.Node estree = ESTreeFactory.from(protobufNode, ESTree.Node.class);
+    assertThat(estree).isInstanceOfSatisfying(ESTree.ArrayPattern.class, array -> {
+      assertThat(array.elements()).hasSize(3);
+      assertThat(array.elements().get(0)).isEmpty();
+      assertThat(array.elements().get(1).get()).isInstanceOf(ESTree.Identifier.class);
+      assertThat(array.elements().get(2)).isEmpty();
+    });
+  }
+
   @Test
   void throw_exception_from_unrecognized_type() {
     Node protobufNode = Node.newBuilder()

tools/estree/output/estree.proto

@@ -239,7 +239,7 @@ message RestElement {
   Node argument = 1;
 }
 message ArrayPattern {
-  repeated Node elements = 1;
+  repeated ArrayElement elements = 1;
 }
 message ObjectPattern {
   repeated Node properties = 1;
@@ -332,7 +332,10 @@ message ArrowFunctionExpression {
   optional bool async = 5;
 }
 message ArrayExpression {
-  repeated Node elements = 1;
+  repeated ArrayElement elements = 1;
+}
+message ArrayElement {
+  optional Node element = 1;
 }
 message ClassDeclaration {
   optional Node id = 1;