Skip to content

Commit

Permalink
Update TypeInference#backwardsInferenceFromCallSite for tagged templa…
Browse files Browse the repository at this point in the history 
…te literals

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=196916150
  • Loading branch information
@lauraharker @blickly
lauraharker authored and blickly committed May 17, 2018
1 parent f6cba36 commit 9337340
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 42 deletions.
90 changes: 53 additions & 37 deletions src/com/google/javascript/jscomp/TypeInference.java
View file
Expand Up @@ -23,6 +23,7 @@
import static com.google.javascript.rhino.jstype.JSTypeNative.BOOLEAN_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.BOOLEAN_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.CHECKED_UNKNOWN_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.CHECKED_UNKNOWN_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.ITERABLE_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.ITERABLE_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.I_TEMPLATE_ARRAY_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.NULL_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.NULL_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.NUMBER_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.NUMBER_TYPE;
import static com.google.javascript.rhino.jstype.JSTypeNative.NUMBER_VALUE_OR_OBJECT_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.NUMBER_VALUE_OR_OBJECT_TYPE;
Expand All @@ -32,6 +33,7 @@


import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.javascript.jscomp.CodingConvention.AssertionFunctionSpec; import com.google.javascript.jscomp.CodingConvention.AssertionFunctionSpec;
Expand Down Expand Up @@ -376,7 +378,8 @@ private FlowScope traverse(Node n, FlowScope scope) {
break; break;


case CALL: case CALL:
scope = traverseCall(n, scope); scope = traverseFunctionInvocation(n, scope);
scope = tightenTypesAfterAssertions(scope, n);
break; break;


case NEW: case NEW:
Expand Down Expand Up @@ -452,7 +455,7 @@ private FlowScope traverse(Node n, FlowScope scope) {
break; break;


case TAGGED_TEMPLATELIT: case TAGGED_TEMPLATELIT:
scope = traverseTaggedTemplateLiteral(n, scope); scope = traverseFunctionInvocation(n, scope);
break; break;


case DELPROP: case DELPROP:
Expand Down Expand Up @@ -1076,7 +1079,9 @@ private FlowScope traverseHook(Node n, FlowScope scope) {
return scope.createChildFlowScope(); return scope.createChildFlowScope();
} }


private FlowScope traverseCall(Node n, FlowScope scope) { /** @param n A non-constructor function invocation, i.e. CALL or TAGGED_TEMPLATELIT */
private FlowScope traverseFunctionInvocation(Node n, FlowScope scope) {
checkArgument(n.isCall() || n.isTaggedTemplateLit(), n);
scope = traverseChildren(n, scope); scope = traverseChildren(n, scope);


Node left = n.getFirstChild(); Node left = n.getFirstChild();
Expand All @@ -1088,23 +1093,6 @@ private FlowScope traverseCall(Node n, FlowScope scope) {
} else if (functionType.isEquivalentTo( } else if (functionType.isEquivalentTo(
getNativeType(CHECKED_UNKNOWN_TYPE))) { getNativeType(CHECKED_UNKNOWN_TYPE))) {
n.setJSType(getNativeType(CHECKED_UNKNOWN_TYPE)); n.setJSType(getNativeType(CHECKED_UNKNOWN_TYPE));
}

scope = tightenTypesAfterAssertions(scope, n);
return scope;
}

private FlowScope traverseTaggedTemplateLiteral(Node n, FlowScope scope) {
scope = traverseChildren(n, scope);

Node left = n.getFirstChild();
JSType functionType = getJSType(left).restrictByNotNullOrUndefined();
if (functionType.isFunctionType()) {
FunctionType fnType = functionType.toMaybeFunctionType();
n.setJSType(fnType.getReturnType());
// TODO(b/78891530): make "backwardsInferenceFromCallSite" work for tagged template literals
} else if (functionType.isEquivalentTo(getNativeType(CHECKED_UNKNOWN_TYPE))) {
n.setJSType(getNativeType(CHECKED_UNKNOWN_TYPE));
} else { } else {
n.setJSType(getNativeType(UNKNOWN_TYPE)); n.setJSType(getNativeType(UNKNOWN_TYPE));
} }
Expand Down Expand Up @@ -1198,7 +1186,7 @@ private void backwardsInferenceFromCallSite(Node n, FunctionType fnType, FlowSco
if (updatedFnType) { if (updatedFnType) {
fnType = n.getFirstChild().getJSType().toMaybeFunctionType(); fnType = n.getFirstChild().getJSType().toMaybeFunctionType();
} }
updateTypeOfParameters(n, fnType); updateTypeOfArguments(n, fnType);
updateBind(n); updateBind(n);
} }


Expand Down Expand Up @@ -1240,26 +1228,34 @@ private void updateBind(Node n) {
} }


/** /**
* For functions with function parameters, type inference will set the type of * For functions with function parameters, type inference will set the type of a function literal
* a function literal argument from the function parameter type. * argument from the function parameter type.
*/ */
private void updateTypeOfParameters(Node n, FunctionType fnType) { private void updateTypeOfArguments(Node n, FunctionType fnType) {
checkState(n.isCall() || n.isNew(), n); checkState(NodeUtil.isInvocation(n), n);
int i = 0; Iterator<Node> parameters = fnType.getParameters().iterator();
int childCount = n.getChildCount(); if (n.isTaggedTemplateLit()) {
Node iArgument = n.getFirstChild(); // Skip the first parameter because it corresponds to a constructed array of the template lit
for (Node iParameter : fnType.getParameters()) { // subs, not an actual AST node, so there's nothing to update.
if (i + 1 >= childCount) { if (!parameters.hasNext()) {
// TypeCheck#visitParametersList will warn so we bail. // TypeCheck will warn if there is no first parameter. Just bail out here.
return; return;
} }
parameters.next();
}
Iterator<Node> arguments = NodeUtil.getInvocationArgsAsIterable(n).iterator();

Node iParameter;
Node iArgument;


// NOTE: the first child of the call node is the call target, which we want to skip, so // Note: if there are too many or too few arguments, TypeCheck will warn.
// it is correct to call getNext on the first iteration. while (parameters.hasNext() && arguments.hasNext()) {
iArgument = iArgument.getNext(); iArgument = arguments.next();
JSType iArgumentType = getJSType(iArgument); JSType iArgumentType = getJSType(iArgument);


iParameter = parameters.next();
JSType iParameterType = getJSType(iParameter); JSType iParameterType = getJSType(iParameter);

inferPropertyTypesToMatchConstraint(iArgumentType, iParameterType); inferPropertyTypesToMatchConstraint(iArgumentType, iParameterType);


// If the parameter to the call is a function expression, propagate the // If the parameter to the call is a function expression, propagate the
Expand Down Expand Up @@ -1289,7 +1285,6 @@ private void updateTypeOfParameters(Node n, FunctionType fnType) {
boolean declared = argJsdoc != null && argJsdoc.containsDeclaration(); boolean declared = argJsdoc != null && argJsdoc.containsDeclaration();
iArgument.setJSType(matchFunction(restrictedParameter, argFnType, declared)); iArgument.setJSType(matchFunction(restrictedParameter, argFnType, declared));
} }
i++;
} }
} }


Expand Down Expand Up @@ -1322,6 +1317,7 @@ private FunctionType matchFunction(
return currentType; return currentType;
} }


/** @param call A CALL, NEW, or TAGGED_TEMPLATELIT node */
private Map<TemplateType, JSType> inferTemplateTypesFromParameters( private Map<TemplateType, JSType> inferTemplateTypesFromParameters(
FunctionType fnType, Node call) { FunctionType fnType, Node call) {
if (fnType.getTemplateTypeMap().getTemplateKeys().isEmpty()) { if (fnType.getTemplateTypeMap().getTemplateKeys().isEmpty()) {
Expand All @@ -1341,10 +1337,30 @@ private Map<TemplateType, JSType> inferTemplateTypesFromParameters(
seenTypes); seenTypes);
} }


if (call.hasMoreThanOneChild()) { if (call.isTaggedTemplateLit()) {
Iterator<Node> fnParameters = fnType.getParameters().iterator();
if (!fnParameters.hasNext()) {
// TypeCheck will warn if there are too few function parameters
return resolvedTypes;
}
// The first argument to the tag function is an array of strings (typed as ITemplateArray)
// but not an actual AST node
maybeResolveTemplatedType(
fnParameters.next().getJSType(),
getNativeType(I_TEMPLATE_ARRAY_TYPE),
resolvedTypes,
seenTypes);

// Resolve the remaining template types from the template literal substitutions.
maybeResolveTemplateTypeFromNodes(
Iterables.skip(fnType.getParameters(), 1),
NodeUtil.getInvocationArgsAsIterable(call),
resolvedTypes,
seenTypes);
} else if (call.hasMoreThanOneChild()) {
maybeResolveTemplateTypeFromNodes( maybeResolveTemplateTypeFromNodes(
fnType.getParameters(), fnType.getParameters(),
call.getSecondChild().siblings(), NodeUtil.getInvocationArgsAsIterable(call),
resolvedTypes, resolvedTypes,
seenTypes); seenTypes);
} }
Expand Down
25 changes: 20 additions & 5 deletions test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java
View file
Expand Up @@ -1677,10 +1677,6 @@ public void testTaggedTemplateLiteral_returnType1() {
} }


public void testTaggedTemplateLiteral_returnType2() { public void testTaggedTemplateLiteral_returnType2() {
// TODO(b/78891530): this should throw a type mismatch error
// We need to infer the template type for tag functions in TypeInference.
// See comment in TypeInference#traverseCallOrTag about back inference
// Note: this does warn if we transpile the template literal before typechecking.
testTypes( testTypes(
lines( lines(
"function takesString(/** string */ s) {}", "function takesString(/** string */ s) {}",
Expand All @@ -1693,7 +1689,26 @@ public void testTaggedTemplateLiteral_returnType2() {
" */", " */",
"function getFirstTemplateLitSub(strings, subExpr, var_args) { return subExpr; }", "function getFirstTemplateLitSub(strings, subExpr, var_args) { return subExpr; }",
"", "",
"takesString(getFirstTemplateLitSub`${1}`);")); "takesString(getFirstTemplateLitSub`${1}`);"),
lines(
"actual parameter 1 of takesString does not match formal parameter",
"found : number",
"required: string"));
}

public void testTaggedTemplateLiteral_backInference() {
// Test that we update the type of the template lit sub after back inference
testTypes(
lines(
"/**",
"* @param {T} x",
"* @param {function(this:T, ...?)} z",
"* @template T",
"*/",
"function f(x, z) {}",
// infers that "this" is ITemplateArray inside the function literal
"f`${ function() { /** @type {string} */ var x = this } }`;"),
lines("initializing variable", "found : ITemplateArray", "required: string"));
} }


public void testITemplateArray1() { public void testITemplateArray1() {
Expand Down

0 comments on commit 9337340

Please sign in to comment.