From c2455bb4dd04c5267d2c3ce2cdbafe8bf9dbaf02 Mon Sep 17 00:00:00 2001 From: lharker Date: Wed, 16 May 2018 09:43:53 -0700 Subject: [PATCH] Add support for tagged template literals in the typechecker This does some type inference of tagged template literals, but is missing a few features it has for regular function calls. The type inference doesn't support back inference for tagged template literals 'calls' (like it does in transpiled code). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=196841118 --- .../jscomp/FunctionTypeBuilder.java | 18 +- .../google/javascript/jscomp/TypeCheck.java | 86 +++++++- .../javascript/jscomp/TypeInference.java | 22 ++ .../javascript/jscomp/TypeValidator.java | 8 + .../rhino/jstype/JSTypeRegistry.java | 16 ++ .../javascript/jscomp/CompilerTestCase.java | 5 + .../jscomp/TypeCheckNoTranspileTest.java | 194 +++++++++++++++++- .../javascript/jscomp/TypeInferenceTest.java | 7 + .../rhino/jstype/JSTypeRegistryTest.java | 8 + 9 files changed, 346 insertions(+), 18 deletions(-) diff --git a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java index 64fbe35ee73..281a4ccbc32 100644 --- a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java +++ b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java @@ -799,15 +799,21 @@ private FunctionType getOrCreateConstructor() { returnType, classTemplateTypeNames, isAbstract); - // We use "getTypeForScope" to specifically check if this was defined for getScopeDeclaredIn() - // so we don't pick up types that are going to be shadowed. - JSType existingType = typeRegistry.getTypeForScope(getScopeDeclaredIn(), fnName); if (makesStructs) { fnType.setStruct(); } else if (makesDicts) { fnType.setDict(); } + + // There are two cases where this type already exists in the current scope: + // 1. The type is a built-in that we initalized in JSTypeRegistry and is also defined in + // externs. + // 2. Cases like "class C {} C = class {}" + // See https://github.com/google/closure-compiler/issues/2928 for some related bugs. + // We use "getTypeForScope" to specifically check if this was defined for getScopeDeclaredIn() + // so we don't pick up types that are going to be shadowed. + JSType existingType = typeRegistry.getTypeForScope(getScopeDeclaredIn(), fnName); if (existingType != null) { boolean isInstanceObject = existingType.isInstanceType(); if (isInstanceObject || fnName.equals("Function")) { @@ -825,6 +831,12 @@ private FunctionType getOrCreateConstructor() { fnType.toString(), existingFn.toString()); } + // If the existing function is a built-in type, set its base type in case it @extends + // another function (since we don't set its prototype in JSTypeRegistry) + if (existingFn.isNativeObjectType()) { + maybeSetBaseType(existingFn); + } + return existingFn; } else { // We fall through and return the created type, even though it will fail diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index a5ab350e641..b8092bc7984 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/src/com/google/javascript/jscomp/TypeCheck.java @@ -640,10 +640,15 @@ public void visit(NodeTraversal t, Node n, Node parent) { ensureTyped(t, n, STRING_TYPE); break; - case TEMPLATELIT: + case TEMPLATELIT: ensureTyped(t, n, STRING_TYPE); break; + case TAGGED_TEMPLATELIT: + visitTaggedTemplateLit(t, n); + ensureTyped(t, n); + break; + case BITNOT: childType = getJSType(n.getFirstChild()); if (!childType.matchesNumberContext()) { @@ -1846,7 +1851,7 @@ private void visitNew(NodeTraversal t, Node n) { if (ctorType != null && ctorType.isAbstract()) { report(t, n, INSTANTIATE_ABSTRACT_CLASS); } - visitParameterList(t, n, fnType); + visitArgumentList(t, n, fnType); ensureTyped(t, n, fnType.getInstanceType()); } else { ensureTyped(t, n); @@ -2073,7 +2078,7 @@ private void visitCall(NodeTraversal t, Node n) { } checkAbstractMethodCall(t, n); - visitParameterList(t, n, functionType); + visitArgumentList(t, n, functionType); ensureTyped(t, n, functionType.getReturnType()); } else { ensureTyped(t, n); @@ -2118,17 +2123,32 @@ private void checkAbstractMethodCall(NodeTraversal t, Node call) { } } + /** Visits the parameters of a CALL or a NEW node. */ + private void visitArgumentList(NodeTraversal t, Node call, FunctionType functionType) { + Iterator parameters = functionType.getParameters().iterator(); + Iterator arguments = NodeUtil.getInvocationArgsAsIterable(call).iterator(); + checkArgumentsMatchParameters(t, call, functionType, arguments, parameters, 0); + } + /** - * Visits the parameters of a CALL or a NEW node. + * Checks that a list of arguments match a list of formal parameters + * + *

If given a TAGGED_TEMPLATE_LIT, the given Iterator should only contain the parameters + * corresponding to the actual template lit sub arguments, skipping over the first parameter. + * + * @param firstParameterIndex The index of the first parameter in the given Iterator in the + * function type's parameter list. */ - private void visitParameterList(NodeTraversal t, Node call, - FunctionType functionType) { - Iterator arguments = call.children().iterator(); - arguments.next(); // skip the function name + private void checkArgumentsMatchParameters( + NodeTraversal t, + Node call, + FunctionType functionType, + Iterator arguments, + Iterator parameters, + int firstParameterIndex) { - Iterator parameters = functionType.getParameters().iterator(); int spreadArgumentCount = 0; - int normalArgumentCount = 0; + int normalArgumentCount = firstParameterIndex; boolean checkArgumentTypeAgainstParameter = true; Node parameter = null; Node argument = null; @@ -2332,6 +2352,52 @@ static JSType getTemplateTypeOfGenerator(JSTypeRegistry typeRegistry, JSType gen return typeRegistry.getNativeType(UNKNOWN_TYPE); } + private void visitTaggedTemplateLit(NodeTraversal t, Node n) { + Node tag = n.getFirstChild(); + JSType tagType = tag.getJSType().restrictByNotNullOrUndefined(); + + if (!tagType.canBeCalled()) { + report(t, n, NOT_CALLABLE, tagType.toString()); + return; + } else if (!tagType.isFunctionType()) { + // A few types, like the unknown, regexp, and bottom types, can be called as if they are + // functions. Return if we have one of those types that is not actually a known function. + return; + } + + FunctionType tagFnType = tagType.toMaybeFunctionType(); + Iterator parameters = tagFnType.getParameters().iterator(); + + // The tag function gets an array of all the template lit substitutions as its first argument, + // but there's no actual AST node representing that array so we typecheck it separately from + // the other tag arguments. + + // Validate that the tag function takes at least one parameter + if (!parameters.hasNext()) { + report( + t, + n, + WRONG_ARGUMENT_COUNT, + typeRegistry.getReadableTypeNameNoDeref(tag), + String.valueOf(NodeUtil.getInvocationArgsCount(n)), + "0", + " and no more than 0 argument(s)"); + return; + } + + // Validate that the first parameter is a supertype of ITemplateArray + Node firstParameter = parameters.next(); + JSType parameterType = firstParameter.getJSType().restrictByNotNullOrUndefined(); + if (parameterType != null) { + validator.expectITemplateArraySupertype( + t, firstParameter, parameterType, "Invalid type for the first parameter of tag function"); + } + + // Validate the remaining parameters (the template literal substitutions) + checkArgumentsMatchParameters( + t, n, tagFnType, NodeUtil.getInvocationArgsAsIterable(n).iterator(), parameters, 1); + } + /** * This function unifies the type checking involved in the core binary * operators and the corresponding assignment operators. The representation diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java index 2f26dff436b..dd814d6f84a 100644 --- a/src/com/google/javascript/jscomp/TypeInference.java +++ b/src/com/google/javascript/jscomp/TypeInference.java @@ -445,6 +445,10 @@ private FlowScope traverse(Node n, FlowScope scope) { scope = traverseChildren(n, scope); break; + case TAGGED_TEMPLATELIT: + scope = traverseTaggedTemplateLiteral(n, scope); + break; + case DELPROP: case LT: case LE: @@ -1084,6 +1088,24 @@ private FlowScope traverseCall(Node n, FlowScope scope) { 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)); + } + + return scope; + } + private FlowScope tightenTypesAfterAssertions(FlowScope scope, Node callNode) { Node left = callNode.getFirstChild(); Node firstParam = left.getNext(); diff --git a/src/com/google/javascript/jscomp/TypeValidator.java b/src/com/google/javascript/jscomp/TypeValidator.java index f3a867c54aa..a10a7ab08b6 100644 --- a/src/com/google/javascript/jscomp/TypeValidator.java +++ b/src/com/google/javascript/jscomp/TypeValidator.java @@ -24,6 +24,7 @@ import static com.google.javascript.rhino.jstype.JSTypeNative.BOOLEAN_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.GENERATOR_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.NO_OBJECT_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.NULL_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.NUMBER_STRING; @@ -304,6 +305,13 @@ void expectGeneratorSupertype(NodeTraversal t, Node n, JSType type, String msg) } } + /** Expect the type to be an ITemplateArray or supertype of ITemplateArray. */ + void expectITemplateArraySupertype(NodeTraversal t, Node n, JSType type, String msg) { + if (!getNativeType(I_TEMPLATE_ARRAY_TYPE).isSubtypeOf(type)) { + mismatch(t, n, msg, type, I_TEMPLATE_ARRAY_TYPE); + } + } + /** * Expect the type to be a string, or a type convertible to string. If the * expectation is not met, issue a warning at the provided node's source code diff --git a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java index 1485f167ac5..8c9c9a6b303 100644 --- a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java +++ b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java @@ -442,6 +442,21 @@ private void initializeBuiltInTypes() { ObjectType ARRAY_TYPE = ARRAY_FUNCTION_TYPE.getInstanceType(); registerNativeType(JSTypeNative.ARRAY_TYPE, ARRAY_TYPE); + // ITemplateArray extends !Array + FunctionType iTemplateArrayFunctionType = + new FunctionType( + this, + "ITemplateArray", + /* source= */ null, + createArrowType(), + /* typeOfThis= */ null, + /* templateTypeMap= */ null, + Kind.CONSTRUCTOR, + /* nativeType= */ true, + /* isAbstract= */ false); + registerNativeType( + JSTypeNative.I_TEMPLATE_ARRAY_TYPE, iTemplateArrayFunctionType.getInstanceType()); + FunctionType iterableFunctionType = new FunctionType( this, @@ -852,6 +867,7 @@ private void initializeRegistry() { registerGlobalType(getNativeType(JSTypeNative.GENERATOR_TYPE)); registerGlobalType(getNativeType(JSTypeNative.DATE_TYPE)); registerGlobalType(getNativeType(JSTypeNative.I_OBJECT_TYPE)); + registerGlobalType(getNativeType(JSTypeNative.I_TEMPLATE_ARRAY_TYPE)); registerGlobalType(getNativeType(JSTypeNative.I_THENABLE_TYPE)); registerGlobalType(getNativeType(JSTypeNative.NULL_TYPE)); registerGlobalType(getNativeType(JSTypeNative.NULL_TYPE), "Null"); diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index 8ca7ccc5f72..fbb6a72bbd5 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTestCase.java @@ -541,6 +541,11 @@ public abstract class CompilerTestCase extends TestCase { " * @template RESULT", " */", "Promise.prototype.catch = function(onRejected) {};", + "/**", + " * @constructor", + " * @extends {Array}", + " */", + "function ITemplateArray() {}", ACTIVE_X_OBJECT_DEF); /** diff --git a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java index 36c95b8ee47..ba13184cdff 100644 --- a/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java @@ -1398,13 +1398,197 @@ public void testTemplateLiteral_isStringType() { "required: number")); } - public void disabled_testTaggedTemplateLiteral1() { - // TODO(b/78891530): Make the typechecker handle tagged template lits. Currently this crashes. + // The first "argument" to a template literal tag function has type !ITemplateArray. + public void testTaggedTemplateLiteral_tagParameters1() { + // ITemplateArray works as the first parameter + testTypesWithExtraExterns( + lines( + "/**", + " * @param {!ITemplateArray} template", + " * @param {...*} var_args Substitution values.", + " * @return {string}", + " */", + "String.raw = function(template, var_args) {};"), + "String.raw`one ${1} two`"); + } + + public void testTaggedTemplateLiteral_tagParameters2() { + // !Array works as the first parameter + testTypesWithCommonExterns( + lines( + "function tag(/** !Array */ strings){}", // preserve newline + "tag`template string`;")); + } + + public void testTaggedTemplateLiteral_tagParameters3() { + // ?Array works as the first parameter + testTypesWithCommonExterns( + lines( + "function tag(/** ?Array */ strings){}", // preserve newline + "tag`template string`;")); + } + + public void testTaggedTemplateLiteral_tagParameters4() { + // Object works as the first parameter + testTypes( + lines( + "function tag(/** Object */ strings){}", // preserve newline + "tag `template string`;")); + } + + public void testTaggedTemplateLiteral_tagParameters5() { + // unknown type works as the first parameter. + testTypes( + lines( + "function tag(/** ? */ strings){}", // preserve newline + "tag `template string`;")); + } + + public void testTaggedTemplateLiteral_invalidTagParameters1() { + // Random object does not work as first parameter testTypes( lines( - "function tag(/** !Array */ strings, /** number */ num) {", - " return num;", + "function tag(/** {a: number} */ strings){}", // preserve newline + "tag `template string`;"), + lines( + "Invalid type for the first parameter of tag function", + "found : {a: number}", + "required: ITemplateArray")); + } + + public void testTaggedTemplateLiteral_invalidTagParameters2() { + // !Array does not work as first parameter + testTypes( + lines( + "function tag(/** !Array */ strings) {}", // preserve newline + "tag`template string`;"), + lines( + "Invalid type for the first parameter of tag function", + "found : Array", + "required: ITemplateArray")); + } + + public void testTaggedTemplateLiteral_invalidTagParameters3() { + // Tag function must have at least one parameter + testTypes( + "function tag(){} tag``;", + "Function tag: called with 1 argument(s). " + + "Function requires at least 0 argument(s) and no more than 0 argument(s)."); + } + + public void testTaggedTemplateLiteral_tagNotAFunction() { + testTypes("const tag = 42; tag `template string`;", "number expressions are not callable"); + } + + public void testTaggedTemplateLiteral_nullableTagFunction() { + testTypes( + lines( + "function f(/** ?function(!ITemplateArray) */ tag) {", // preserve newline + " tag `template string`;", + "}")); + } + + public void testTaggedTemplateLiteral_unknownTagFunction() { + testTypes( + lines( + "function f(/** ? */ tag) {", // preserve newline + " tag `template string`;", + "}")); + } + + public void testTaggedTemplateLiteral_tooFewArguments() { + testTypes( + "function tag(strings, x, y) {} tag`${1}`;", + "Function tag: called with 2 argument(s). " + + "Function requires at least 3 argument(s) and no more than 3 argument(s)."); + } + + public void testTaggedTemplateLiteral_tooManyArguments() { + testTypes( + "function tag(strings, x) {} tag`${0} ${1}`;", + "Function tag: called with 3 argument(s). " + + "Function requires at least 2 argument(s) and no more than 2 argument(s)."); + } + + public void testTaggedTemplateLiteral_argumentTypeMismatch() { + testTypes( + lines( + "function tag(strings, /** string */ s) {}", // preserve newline + "tag`${123}`;"), + lines( + "actual parameter 2 of tag does not match formal parameter", + "found : number", + "required: string")); + } + + public void testTaggedTemplateLiteral_optionalArguments() { + testTypes( + lines( + "/** @param {number=} y */ function tag(strings, y){}", // preserve newline + "tag``;")); + } + + public void testTaggedTemplateLiteral_varArgs() { + testTypes( + lines( + "function tag(strings, /** number */ var_args){}", // preserve newline + "tag`${1} ${'str'}`;"), + lines( + "actual parameter 3 of tag does not match formal parameter", + "found : string", + "required: (number|undefined)")); + } + + public void testTaggedTemplateLiteral_returnType1() { + // Infer the TAGGED_TEMPLATELIT to have the return type of the tag function + testTypes( + lines( + "function takesString(/** string */ s) {}", + "", + "/** @return {number} */", + "function returnsNumber(strings){", + " return 1;", "}", - "tag`foo ${3} bar`")); + "takesString(returnsNumber`str`);"), + lines( + "actual parameter 1 of takesString does not match formal parameter", + "found : number", + "required: string")); + } + + 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) {}", + "/**", + " * @param {!ITemplateArray} strings", + " * @param {T} subExpr", + " * @param {*} var_args", + " * @return {T}", + " * @template T", + " */", + "function getFirstTemplateLitSub(strings, subExpr, var_args) { return subExpr; }", + "", + "takesString(getFirstTemplateLitSub`${1}`);")); + } + + public void testITemplateArray1() { + // Test that ITemplateArray is Iterable and iterating over it produces a string + testTypesWithCommonExterns( + lines( + "function takesNumber(/** number */ n) {}", + "function f(/** !ITemplateArray */ arr) {", + " for (let str of arr) {", + " takesNumber(str);", + " }", + "}"), + lines( + "actual parameter 1 of takesNumber does not match formal parameter", + "found : string", + "required: number")); } } diff --git a/test/com/google/javascript/jscomp/TypeInferenceTest.java b/test/com/google/javascript/jscomp/TypeInferenceTest.java index 972c9df77df..b98084ca49b 100644 --- a/test/com/google/javascript/jscomp/TypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/TypeInferenceTest.java @@ -1735,6 +1735,13 @@ public void testSpreadExpression() { assertTypeOfExpression("X").toStringIsEqualTo("string"); } + public void testTaggedTemplateLiteral1() { + assuming("getNumber", registry.createFunctionType(registry.getNativeType(NUMBER_TYPE))); + inFunction("var num = getNumber``; NUM: num;"); + + assertTypeOfExpression("NUM").isNumber(); + } + private ObjectType getNativeObjectType(JSTypeNative t) { return registry.getNativeObjectType(t); } diff --git a/test/com/google/javascript/rhino/jstype/JSTypeRegistryTest.java b/test/com/google/javascript/rhino/jstype/JSTypeRegistryTest.java index a929f08504e..da170471d8e 100644 --- a/test/com/google/javascript/rhino/jstype/JSTypeRegistryTest.java +++ b/test/com/google/javascript/rhino/jstype/JSTypeRegistryTest.java @@ -45,6 +45,7 @@ import static com.google.javascript.rhino.jstype.JSTypeNative.GENERATOR_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.ITERABLE_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.ITERATOR_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_VOID; import static com.google.javascript.rhino.jstype.JSTypeNative.NUMBER_TYPE; @@ -88,6 +89,13 @@ public void testGetBuiltInType_generator() { typeRegistry.getNativeType(GENERATOR_TYPE), typeRegistry.getGlobalType("Generator")); } + public void testGetBuildInType_iTemplateArray() { + JSTypeRegistry typeRegistry = new JSTypeRegistry(null); + assertTypeEquals( + typeRegistry.getNativeType(I_TEMPLATE_ARRAY_TYPE), + typeRegistry.getGlobalType("ITemplateArray")); + } + public void testGetDeclaredType() { JSTypeRegistry typeRegistry = new JSTypeRegistry(null); JSType type = typeRegistry.createAnonymousObjectType(null);