From 93373405ab302b66d580b492c7549af5ee8afe69 Mon Sep 17 00:00:00 2001 From: lharker Date: Wed, 16 May 2018 17:22:48 -0700 Subject: [PATCH] Update TypeInference#backwardsInferenceFromCallSite for tagged template literals ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=196916150 --- .../javascript/jscomp/TypeInference.java | 90 +++++++++++-------- .../jscomp/TypeCheckNoTranspileTest.java | 25 ++++-- 2 files changed, 73 insertions(+), 42 deletions(-) diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java index 7e85cbe510b..152c26aec2f 100644 --- a/src/com/google/javascript/jscomp/TypeInference.java +++ b/src/com/google/javascript/jscomp/TypeInference.java @@ -23,6 +23,7 @@ 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.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.NUMBER_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.NUMBER_VALUE_OR_OBJECT_TYPE; @@ -32,6 +33,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.javascript.jscomp.CodingConvention.AssertionFunctionSpec; @@ -376,7 +378,8 @@ private FlowScope traverse(Node n, FlowScope scope) { break; case CALL: - scope = traverseCall(n, scope); + scope = traverseFunctionInvocation(n, scope); + scope = tightenTypesAfterAssertions(scope, n); break; case NEW: @@ -452,7 +455,7 @@ private FlowScope traverse(Node n, FlowScope scope) { break; case TAGGED_TEMPLATELIT: - scope = traverseTaggedTemplateLiteral(n, scope); + scope = traverseFunctionInvocation(n, scope); break; case DELPROP: @@ -1076,7 +1079,9 @@ private FlowScope traverseHook(Node n, FlowScope scope) { 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); Node left = n.getFirstChild(); @@ -1088,23 +1093,6 @@ private FlowScope traverseCall(Node n, FlowScope scope) { } else if (functionType.isEquivalentTo( 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 { n.setJSType(getNativeType(UNKNOWN_TYPE)); } @@ -1198,7 +1186,7 @@ private void backwardsInferenceFromCallSite(Node n, FunctionType fnType, FlowSco if (updatedFnType) { fnType = n.getFirstChild().getJSType().toMaybeFunctionType(); } - updateTypeOfParameters(n, fnType); + updateTypeOfArguments(n, fnType); updateBind(n); } @@ -1240,26 +1228,34 @@ private void updateBind(Node n) { } /** - * For functions with function parameters, type inference will set the type of - * a function literal argument from the function parameter type. + * For functions with function parameters, type inference will set the type of a function literal + * argument from the function parameter type. */ - private void updateTypeOfParameters(Node n, FunctionType fnType) { - checkState(n.isCall() || n.isNew(), n); - int i = 0; - int childCount = n.getChildCount(); - Node iArgument = n.getFirstChild(); - for (Node iParameter : fnType.getParameters()) { - if (i + 1 >= childCount) { - // TypeCheck#visitParametersList will warn so we bail. + private void updateTypeOfArguments(Node n, FunctionType fnType) { + checkState(NodeUtil.isInvocation(n), n); + Iterator parameters = fnType.getParameters().iterator(); + if (n.isTaggedTemplateLit()) { + // Skip the first parameter because it corresponds to a constructed array of the template lit + // subs, not an actual AST node, so there's nothing to update. + if (!parameters.hasNext()) { + // TypeCheck will warn if there is no first parameter. Just bail out here. return; } + parameters.next(); + } + Iterator 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 - // it is correct to call getNext on the first iteration. - iArgument = iArgument.getNext(); + // Note: if there are too many or too few arguments, TypeCheck will warn. + while (parameters.hasNext() && arguments.hasNext()) { + iArgument = arguments.next(); JSType iArgumentType = getJSType(iArgument); + iParameter = parameters.next(); JSType iParameterType = getJSType(iParameter); + inferPropertyTypesToMatchConstraint(iArgumentType, iParameterType); // If the parameter to the call is a function expression, propagate the @@ -1289,7 +1285,6 @@ private void updateTypeOfParameters(Node n, FunctionType fnType) { boolean declared = argJsdoc != null && argJsdoc.containsDeclaration(); iArgument.setJSType(matchFunction(restrictedParameter, argFnType, declared)); } - i++; } } @@ -1322,6 +1317,7 @@ private FunctionType matchFunction( return currentType; } + /** @param call A CALL, NEW, or TAGGED_TEMPLATELIT node */ private Map inferTemplateTypesFromParameters( FunctionType fnType, Node call) { if (fnType.getTemplateTypeMap().getTemplateKeys().isEmpty()) { @@ -1341,10 +1337,30 @@ private Map inferTemplateTypesFromParameters( seenTypes); } - if (call.hasMoreThanOneChild()) { + if (call.isTaggedTemplateLit()) { + Iterator 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( fnType.getParameters(), - call.getSecondChild().siblings(), + NodeUtil.getInvocationArgsAsIterable(call), resolvedTypes, seenTypes); } diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index b825157c82b..d878f96ce59 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -1677,10 +1677,6 @@ public void testTaggedTemplateLiteral_returnType1() { } 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( lines( "function takesString(/** string */ s) {}", @@ -1693,7 +1689,26 @@ public void testTaggedTemplateLiteral_returnType2() { " */", "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() {