Skip over EMPTY nodes in array patterns during typechecking (e.g. [a,…

… , b] = arr)

Right now we crash on these. We can just ignore them because right now we don't have tuple types.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=208567573

src/com/google/javascript/jscomp/DestructuredTarget.java

@@ -19,6 +19,7 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableList;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.jstype.JSType;
 import com.google.javascript.rhino.jstype.JSTypeNative;
@@ -147,6 +148,11 @@ static DestructuredTarget createTarget(
     return createTarget(registry, () -> destructuringPatternType, destructuringChild);
   }
 
+  /**
+   * Converts a given child of a destructuring pattern (in the AST) to an instance of this class.
+   *
+   * NOTE: does not accept EMPTY nodes
+   */
   static DestructuredTarget createTarget(
       JSTypeRegistry registry, Supplier<JSType> destructuringPatternType, Node destructuringChild) {
     checkArgument(destructuringChild.getParent().isDestructuringPattern(), destructuringChild);
@@ -210,6 +216,35 @@ Supplier<JSType> getInferredTypeSupplierWithoutDefaultValue() {
     return () -> inferTypeWithoutUsingDefaultValue();
   }
 
+  /**
+   * Returns all the targets directly in the given pattern, except for EMPTY nodes
+   *
+   * <p>EMPTY nodes occur in array patterns with elisions, e.g. `[, , a] = []`
+   */
+  static ImmutableList<DestructuredTarget> createAllNonEmptyTargetsInPattern(
+      JSTypeRegistry registry, JSType patternType, Node pattern) {
+    return createAllNonEmptyTargetsInPattern(registry, () -> patternType, pattern);
+  }
+
+  /**
+   * Returns all the targets directly in the given pattern, except for EMPTY nodes
+   *
+   * <p>EMPTY nodes occur in array patterns with elisions, e.g. `[, , a] = []`
+   */
+  static ImmutableList<DestructuredTarget> createAllNonEmptyTargetsInPattern(
+      JSTypeRegistry registry, Supplier<JSType> patternType, Node pattern) {
+    checkArgument(pattern.isDestructuringPattern(), pattern);
+    ImmutableList.Builder<DestructuredTarget> builder = ImmutableList.builder();
+    for (Node child : pattern.children()) {
+      if (child.isEmpty()) {
+        continue;
+      }
+
+      builder.add(createTarget(registry, patternType, child));
+    }
+    return builder.build();
+  }
+
   /**
    * Infers the type of this target before the default value (if any) is applied.
    *

src/com/google/javascript/jscomp/TypeCheck.java

@@ -1092,8 +1092,9 @@ private void checkDestructuringAssignment(
           getNativeType(JSTypeNative.OBJECT_TYPE));
     }
 
-    for (Node child : pattern.children()) {
-      DestructuredTarget target = DestructuredTarget.createTarget(typeRegistry, rightType, child);
+    for (DestructuredTarget target :
+        DestructuredTarget.createAllNonEmptyTargetsInPattern(typeRegistry, rightType, pattern)) {
+
       // TODO(b/77597706): this is not very efficient because it re-infers the types below,
       // which we already did once in TypeInference. don't repeat the work.
       checkCanAssignToWithScope(

src/com/google/javascript/jscomp/TypeInference.java

@@ -1131,8 +1131,8 @@ private FlowScope traverseDestructuringPatternHelper(
       Node pattern, FlowScope scope, JSType patternType, TypeDeclaringCallback declarer) {
     checkArgument(pattern.isDestructuringPattern(), pattern);
     checkNotNull(patternType);
-    for (Node key : pattern.children()) {
-      DestructuredTarget target = DestructuredTarget.createTarget(registry, patternType, key);
+    for (DestructuredTarget target :
+        DestructuredTarget.createAllNonEmptyTargetsInPattern(registry, patternType, pattern)) {
 
       // The computed property is always evaluated first.
       if (target.hasComputedProperty()) {

src/com/google/javascript/jscomp/TypedScopeCreator.java

@@ -866,9 +866,9 @@ void defineVarChild(JSDocInfo declarationInfo, Node child, TypedScope scope) {
 
     void defineDestructuringPatternInVarDeclaration(
         Node pattern, TypedScope scope, Supplier<JSType> patternTypeSupplier) {
-      for (Node child : pattern.children()) {
-        DestructuredTarget target =
-            DestructuredTarget.createTarget(typeRegistry, patternTypeSupplier, child);
+      for (DestructuredTarget target :
+          DestructuredTarget.createAllNonEmptyTargetsInPattern(
+              typeRegistry, patternTypeSupplier, pattern)) {
 
         // Currently we only do type inference for string key nodes in object patterns here, to
         // handle aliasing types. e.g
@@ -2626,9 +2626,10 @@ private void declareNamesInPositionalParameter(
     /** Declares all names inside a destructuring pattern in a parameter list in the scope */
     private void declareDestructuringParameter(
         boolean isInferred, Node pattern, JSType patternType) {
-      for (Node child : pattern.children()) {
-        DestructuredTarget target =
-            DestructuredTarget.createTarget(typeRegistry, () -> patternType, child);
+
+      for (DestructuredTarget target :
+          DestructuredTarget.createAllNonEmptyTargetsInPattern(
+              typeRegistry, patternType, pattern)) {
         JSType inferredType = target.inferTypeWithoutUsingDefaultValue();
 
         if (target.hasDefaultValue()) {

test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java

@@ -3883,6 +3883,18 @@ public void testNestedDestructuringPatternDeclaration() {
             "required: string"));
   }
 
+  public void testArrayPatternDeclarationWithElision() {
+    testTypes(
+        lines(
+            "function f(/** !Iterable<number> */ numbers) {",
+            "  const [, /** number */ x, , /** string */ y] = numbers;",
+            "}"),
+        lines(
+            "initializing variable", //
+            "found   : number",
+            "required: string"));
+  }
+
   public void testBasicArrayPatternAssign() {
     testTypes(
         lines(
@@ -4024,6 +4036,10 @@ public void testDefaultValueForNestedArrayPatternMustBeIterable() {
             "required: Iterable"));
   }
 
+  public void testArrayDestructuringParameterWithElision() {
+    testTypes("/** @param {!Array<number>} numbers */ function f([, x, , y]) {}");
+  }
+
   public void testDictClass1() {
     testTypes("/** @dict */ var C = class { constructor() {} 'x'(){} };");
   }