Skip to content

Commit

Permalink
Do some basic inference for const object destructuring declarations i…
Browse files Browse the repository at this point in the history
…n TypedScopeCreator

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=206943596
  • Loading branch information
lauraharker authored and blickly committed Aug 1, 2018
1 parent c55b734 commit 5f5d38a
Show file tree
Hide file tree
Showing 5 changed files with 269 additions and 50 deletions.
139 changes: 139 additions & 0 deletions src/com/google/javascript/jscomp/DestructuredTarget.java
@@ -0,0 +1,139 @@
/*
* Copyright 2018 The Closure Compiler Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.javascript.jscomp;

import static com.google.common.base.Preconditions.checkArgument;

import com.google.common.base.Supplier;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.JSTypeNative;
import com.google.javascript.rhino.jstype.JSTypeRegistry;
import javax.annotation.Nullable;

/**
* Represents a single target inside a destructuring pattern, whether another pattern or a lhs
* expression.
*
* <p>Call {@code inferType} to do type inference on the target lazily. This class is designed so
* that a caller can get information about a target, use that information to do additional type
* inference, and finally call {@code inferType} if desired.
*/
final class DestructuredTarget {
private final JSTypeRegistry registry;
/**
* Holds the STRING_KEY or COMPUTED_PROPERTY for a target in an object pattern. Null for targets
* in array patterns.
*/
@Nullable private final Node objectPatternKey;
/**
* The target being assigned to. Can be a destructuring pattern or name, if from a declaration, or
* an arbitrary lhs expression in an assign.
*/
private final Node node;
/**
* A supplier to get the type of the pattern containing this target. e.g. for `a` in `const {a} =
* {a: 3}`, the supplier provides the record type `{a: number}`
*
* <p>This is only called by {@inferType}, not when calling {@code createTarget}.
*/
private final Supplier<JSType> patternTypeSupplier;

private DestructuredTarget(
Node node,
@Nullable Node objectPatternKey,
JSTypeRegistry registry,
Supplier<JSType> patternTypeSupplier) {
this.node = node;
this.objectPatternKey = objectPatternKey;
this.registry = registry;
this.patternTypeSupplier = patternTypeSupplier;
}

public Node getNode() {
return node;
}

static DestructuredTarget createTarget(
JSTypeRegistry registry, Supplier<JSType> destructuringPatternType, Node destructuringChild) {
checkArgument(destructuringChild.getParent().isDestructuringPattern(), destructuringChild);
final Node node;
Node objectLiteralKey = null;
switch (destructuringChild.getToken()) {
case STRING_KEY:
// const {objectLiteralKey: x} = ...
objectLiteralKey = destructuringChild;
Node value = destructuringChild.getFirstChild();
node = value.isDefaultValue() ? value.getFirstChild() : value;
break;

case COMPUTED_PROP:
// const {['objectLiteralKey']: x} = ...
objectLiteralKey = destructuringChild;
value = destructuringChild.getSecondChild();
node = value.isDefaultValue() ? value.getFirstChild() : value;
break;

case OBJECT_PATTERN: // const [{x}] = ...
case ARRAY_PATTERN: // const [[x]] = ...
case NAME: // const [x] = ...
node = destructuringChild;
break;

case DEFAULT_VALUE: // const [x = 3] = ...
case REST: // const [...x] = ...
node = destructuringChild.getFirstChild();
break;

default:
throw new IllegalStateException("Unexpected parameter node " + destructuringChild);
}
return new DestructuredTarget(node, objectLiteralKey, registry, destructuringPatternType);
}

Supplier<JSType> getInferredTypeSupplier() {
return () -> inferType();
}

JSType inferType() {
// TODO(b/203401365): handle array patterns
return objectPatternKey != null ? inferObjectPatternKeyType() : null;
}

private JSType inferObjectPatternKeyType() {
JSType patternType = patternTypeSupplier.get();
switch (objectPatternKey.getToken()) {
case STRING_KEY:
JSType propertyType = patternType.findPropertyType(objectPatternKey.getString());
return propertyType != null
? propertyType
: registry.getNativeType(JSTypeNative.UNKNOWN_TYPE);

case COMPUTED_PROP:
return patternType != null
? patternType
.getTemplateTypeMap()
.getResolvedTemplateType(registry.getObjectElementKey())
: registry.getNativeType(JSTypeNative.UNKNOWN_TYPE);

default:
// TODO(b/203401365): handle object rest
throw new IllegalStateException("Unexpected key " + objectPatternKey);
}

// TODO(b/77597706): handle default values
}
}
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/NodeUtil.java
Expand Up @@ -4814,6 +4814,10 @@ static boolean isConstantDeclaration(
// TODO(b/77597706): Update this method to handle destructured declarations.
if (node.isName() && node.getParent().isConst()) {
return true;
} else if (node.isName()
&& isLhsByDestructuring(node)
&& getRootTarget(node).getGrandparent().isConst()) {
return true;
}

if (info != null && info.isConstant()) {
Expand Down
115 changes: 76 additions & 39 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
Expand Up @@ -46,6 +46,7 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Supplier;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.HashMultiset;
Expand Down Expand Up @@ -737,7 +738,7 @@ void processObjectLitProperties(
Node value = keyNode.getFirstChild();
String memberName = NodeUtil.getObjectLitKeyName(keyNode);
JSDocInfo info = keyNode.getJSDocInfo();
JSType valueType = getDeclaredType(info, keyNode, value);
JSType valueType = getDeclaredType(info, keyNode, value, null);
JSType keyType =
objLitType.isEnumType()
? objLitType.toMaybeEnumType().getElementsType()
Expand Down Expand Up @@ -813,7 +814,7 @@ void defineCatch(Node n) {
// the catch declaration.
// e.g. `} catch ({message, errno}) {`
for (Node catchName : NodeUtil.findLhsNodesInNode(n)) {
JSType type = getDeclaredType(catchName.getJSDocInfo(), catchName, null);
JSType type = getDeclaredType(catchName.getJSDocInfo(), catchName, null, null);
new SlotDefiner()
.forDeclarationNode(catchName)
.forVariableName(catchName.getString())
Expand All @@ -831,38 +832,61 @@ void defineVars(Node n) {
JSDocInfo info = n.getJSDocInfo();
// `var` declarations are hoisted, but `let` and `const` are not.
TypedScope scope = n.isVar() ? currentHoistScope : currentScope;
// Get a list of all of the name nodes for all the variables being declared.
// This handles even complex destructuring cases.
// e.g.
// let x = 3, {y, someProp: [ a1, a2 ]} = someObject;
// varNames will contain x, y, a1, and a2
List<Node> varNames = NodeUtil.findLhsNodesInNode(n);
if (varNames.size() == 1) {
// e.g.
// /** @type {string} */
// let x = 'hi';
Node singleVarName = varNames.get(0);
if (info == null) {
// There might be inline JSDoc on the variable name.
// e.g.
// let /** string */ x = 'hi';
info = singleVarName.getJSDocInfo();

if (n.hasMoreThanOneChild() && info != null) {
report(JSError.make(n, MULTIPLE_VAR_DEF));
}

for (Node child : n.children()) {
defineVarChild(info, child, scope);
}
}

/** Defines a variable declared with `var`, `let`, or `const`. */
void defineVarChild(JSDocInfo declarationInfo, Node child, TypedScope scope) {
if (child.isName()) {
if (declarationInfo == null) {
declarationInfo = child.getJSDocInfo();
// TODO(bradfordcsmith): Report an error if both the declaration node and the name itself
// have JSDoc.
}
defineName(singleVarName, scope, info);
defineName(child, scope, declarationInfo);
} else {
if (info != null) {
// We do not allow a single JSDoc to apply to multiple variables in a single
// declaration.
report(JSError.make(n, MULTIPLE_VAR_DEF));
}
// TODO(bradfordcsmith): Should we report an error if there are actually 0 variable names?
// This can happen with destructuring patterns.
// e.g.
// let {} = foo();
for (Node nameNode : varNames) {
defineName(nameNode, scope, nameNode.getJSDocInfo());
checkState(child.isDestructuringLhs(), child);
Node pattern = child.getFirstChild();
Node value = child.getSecondChild();
defineDestructuringPatternInVarDeclaration(
pattern, scope, () -> getDeclaredRValueType(/* lValue= */ null, value));
}
}

void defineDestructuringPatternInVarDeclaration(
Node pattern, TypedScope scope, Supplier<JSType> patternTypeSupplier) {
for (Node child : pattern.children()) {
DestructuredTarget target =
DestructuredTarget.createTarget(typeRegistry, patternTypeSupplier, child);
if (target.getNode().isDestructuringPattern()) {
defineDestructuringPatternInVarDeclaration(
target.getNode(), scope, target.getInferredTypeSupplier());
} else {
Node name = target.getNode();
checkState(name.isName(), "This method is only for declaring variables: %s", name);

// variable's type
JSType type =
getDeclaredType(
name.getJSDocInfo(), name, /* rValue= */ null, target.getInferredTypeSupplier());
if (type == null) {
// The variable's type will be inferred.
type = name.isFromExterns() ? unknownType : null;
}
new SlotDefiner()
.forDeclarationNode(name)
.forVariableName(name.getString())
.inScope(scope)
.withType(type)
.allowLaterTypeInference(type == null)
.defineSlot();
}
}
}
Expand Down Expand Up @@ -946,7 +970,7 @@ private void defineName(Node name, TypedScope scope, JSDocInfo info) {
Node value = name.getFirstChild();

// variable's type
JSType type = getDeclaredType(info, name, value);
JSType type = getDeclaredType(info, name, value, /* declaredRValueTypeSupplier= */ null);
if (type == null) {
// The variable's type will be inferred.
type = name.isFromExterns() ? unknownType : null;
Expand Down Expand Up @@ -1668,14 +1692,20 @@ private TypedScope getLValueRootScope(Node n) {
}

/**
* Look for a type declaration on a property assignment
* (in an ASSIGN or an object literal key).
* Look for a type declaration on a property assignment (in an ASSIGN or an object literal key).
*
* @param info The doc info for this property.
* @param lValue The l-value node.
* @param rValue The node that {@code n} is being initialized to,
* or {@code null} if this is a stub declaration.
* @param rValue The node that {@code n} is being initialized to, or {@code null} if this is a
* stub declaration.
* @param declaredRValueTypeSupplier A supplier for the declared type of the rvalue, used for
* destructuring declarations where we have to do additional work on the rvalue.
*/
JSType getDeclaredType(JSDocInfo info, Node lValue, @Nullable Node rValue) {
JSType getDeclaredType(
JSDocInfo info,
Node lValue,
@Nullable Node rValue,
@Nullable Supplier<JSType> declaredRValueTypeSupplier) {
if (info != null && info.hasType()) {
return getDeclaredTypeInAnnotation(lValue, info);
} else if (rValue != null
Expand All @@ -1698,14 +1728,20 @@ && shouldUseFunctionLiteralType(
}

// Check if this is constant, and if it has a known type.
if (NodeUtil.isConstantDeclaration(
compiler.getCodingConvention(), info, lValue)) {
if (NodeUtil.isConstantDeclaration(compiler.getCodingConvention(), info, lValue)) {
if (rValue != null) {
JSType rValueType = getDeclaredRValueType(lValue, rValue);
maybeDeclareAliasType(lValue, rValue, rValueType);
if (rValueType != null) {
return rValueType;
}
} else if (declaredRValueTypeSupplier != null) {
JSType rValueType = declaredRValueTypeSupplier.get();
// TODO(b/77597706): make this work for destructuring?
// maybeDeclareAliasType(lValue, rValue, rValueType);
if (rValueType != null) {
return rValueType;
}
}
}

Expand All @@ -1722,6 +1758,7 @@ && shouldUseFunctionLiteralType(
* as a type name, depending on what other.name is defined to be.
*/
private void maybeDeclareAliasType(Node lValue, Node rValue, JSType rValueType) {
// TODO(b/77597706): handle destructuring here
if (!lValue.isQualifiedName() || !rValue.isQualifiedName()) {
return;
}
Expand Down Expand Up @@ -1980,7 +2017,7 @@ void maybeDeclareQualifiedName(NodeTraversal t, JSDocInfo info,
// about getting as much type information as possible for them.

// Determining type for #1 + #2 + #3 + #4
JSType valueType = getDeclaredType(info, n, rhsValue);
JSType valueType = getDeclaredType(info, n, rhsValue, null);
if (valueType == null && rhsValue != null) {
// Determining type for #5
valueType = rhsValue.getJSType();
Expand Down
12 changes: 10 additions & 2 deletions test/com/google/javascript/jscomp/NodeUtilTest.java
Expand Up @@ -2560,8 +2560,16 @@ public void testIsConstantDeclaration() {
assertIsConstantDeclaration(true, parse("var /** @const */ x = 1;").getFirstFirstChild());
assertIsConstantDeclaration(false, parse("var x, /** @const */ y = 1;").getFirstFirstChild());

// TODO(b/77597706): Update this NodeUtil.isConstantDeclaration() to handle destructured
// declarations.
assertIsConstantDeclaration(true, getNameNodeFrom("const [a] = [];", "a"));
assertIsConstantDeclaration(true, getNameNodeFrom("const [[[a]]] = [];", "a"));
assertIsConstantDeclaration(true, getNameNodeFrom("const [a = 1] = [];", "a"));
assertIsConstantDeclaration(true, getNameNodeFrom("const [...a] = [];", "a"));

assertIsConstantDeclaration(true, getNameNodeFrom("const {a} = {};", "a"));
assertIsConstantDeclaration(true, getNameNodeFrom("const {a = 1} = {};", "a"));
assertIsConstantDeclaration(true, getNameNodeFrom("const {[3]: a} = {};", "a"));
assertIsConstantDeclaration(true, getNameNodeFrom("const {a: [a]} = {};", "a"));

// TODO(bradfordcsmith): Add test cases for other coding conventions.
}

Expand Down

0 comments on commit 5f5d38a

Please sign in to comment.