diff --git a/src/com/google/javascript/jscomp/DestructuredTarget.java b/src/com/google/javascript/jscomp/DestructuredTarget.java index 5922a8827e7..403bf2149a9 100644 --- a/src/com/google/javascript/jscomp/DestructuredTarget.java +++ b/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 destructuringPatternType, Node destructuringChild) { checkArgument(destructuringChild.getParent().isDestructuringPattern(), destructuringChild); @@ -210,6 +216,35 @@ Supplier getInferredTypeSupplierWithoutDefaultValue() { return () -> inferTypeWithoutUsingDefaultValue(); } + /** + * Returns all the targets directly in the given pattern, except for EMPTY nodes + * + *

EMPTY nodes occur in array patterns with elisions, e.g. `[, , a] = []` + */ + static ImmutableList 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 + * + *

EMPTY nodes occur in array patterns with elisions, e.g. `[, , a] = []` + */ + static ImmutableList createAllNonEmptyTargetsInPattern( + JSTypeRegistry registry, Supplier patternType, Node pattern) { + checkArgument(pattern.isDestructuringPattern(), pattern); + ImmutableList.Builder 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. * diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index ed2f9955d21..3edc841edbc 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/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( diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java index 1f038a3d4f7..933d45585c7 100644 --- a/src/com/google/javascript/jscomp/TypeInference.java +++ b/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()) { diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index 3b320b37e45..4137fdb07d5 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/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 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()) { diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index e2e703a864c..ed3341282a5 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -3883,6 +3883,18 @@ public void testNestedDestructuringPatternDeclaration() { "required: string")); } + public void testArrayPatternDeclarationWithElision() { + testTypes( + lines( + "function f(/** !Iterable */ 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} numbers */ function f([, x, , y]) {}"); + } + public void testDictClass1() { testTypes("/** @dict */ var C = class { constructor() {} 'x'(){} };"); }