From 76fc2f63074c0b5dacfe084c1af71c395aafd1d3 Mon Sep 17 00:00:00 2001 From: dimvar Date: Fri, 1 Dec 2017 13:39:44 -0800 Subject: [PATCH] [NTI] Infer function signature for an unannotated callback during NTI because at that point we have better type information. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=177634311 --- .../jscomp/GlobalTypeInfoCollector.java | 24 ++- .../google/javascript/jscomp/NTIScope.java | 3 - .../javascript/jscomp/NewTypeInference.java | 98 ++++++++-- .../google/javascript/jscomp/NodeUtil.java | 20 +++ .../jscomp/newtypes/DeclaredFunctionType.java | 3 +- .../jscomp/newtypes/FunctionType.java | 14 +- .../jscomp/NewTypeInferenceTTLTest.java | 15 ++ .../jscomp/NewTypeInferenceTest.java | 167 +++++++++++++++++- .../jscomp/NewTypeInferenceTestBase.java | 11 +- ...NewTypeInferenceWithTranspilationTest.java | 62 +++++++ 10 files changed, 377 insertions(+), 40 deletions(-) diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java index f9f93887d4a..9c843edde34 100644 --- a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java +++ b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java @@ -2547,12 +2547,16 @@ private void maybeWarnFunctionDeclaration(Node funNode, DeclaredFunctionType fun } } - // We only return a non-null result if the arity of declNode matches the - // arity we get from declaredTypeAsJSType. + /** + * Computes a declared type for an unannotated callback using the corresponding formal + * type from the callee. + * We redo this in NewTypeInference and may find a better type for the callback there, + * but we still need to do it here as well, to infer constants inside the callback. + */ private DeclaredFunctionType computeFnDeclaredTypeFromCallee( - Node declNode, JSType declaredTypeAsJSType) { - checkArgument(declNode.isFunction()); - checkArgument(declNode.getParent().isCall()); + Node callback, JSType declaredTypeAsJSType) { + checkArgument(callback.isFunction()); + checkArgument(callback.getParent().isCall()); if (declaredTypeAsJSType == null) { return null; @@ -2567,14 +2571,8 @@ private DeclaredFunctionType computeFnDeclaredTypeFromCallee( if (declType == null) { return null; } - int numFormals = declNode.getSecondChild().getChildCount(); - int optArity = declType.getOptionalArity(); - boolean hasRestFormals = declType.hasRestFormals(); - if ((hasRestFormals && numFormals <= optArity + 1) - || (!hasRestFormals && numFormals <= optArity)) { - return declType; - } - return null; + return NodeUtil.getApproxRequiredArity(callback) <= declType.getMaxArity() + ? declType : null; } // Returns null if it can't find a suitable type in the context diff --git a/src/com/google/javascript/jscomp/NTIScope.java b/src/com/google/javascript/jscomp/NTIScope.java index 4e1161f4b2a..74b0fe31684 100644 --- a/src/com/google/javascript/jscomp/NTIScope.java +++ b/src/com/google/javascript/jscomp/NTIScope.java @@ -59,9 +59,6 @@ static enum VarKind { INFERRED } - /** - * Used for local variables. - */ static class LocalVarInfo implements Serializable { // When we don't know the type of a local variable, this field is null, not ?. private final JSType type; diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index 46f5e085253..7b2ef6f8e29 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -790,8 +790,12 @@ private void analyzeFunction(NTIScope scope) { println("=== Analyzing function: ", scope.getReadableName(), " ==="); currentScope = scope; exitEnvs = new ArrayList<>(); + Node scopeRoot = scope.getRoot(); + if (NodeUtil.isUnannotatedCallback(scopeRoot)) { + computeFnDeclaredTypeForCallback(scope); + } ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, false); - cfa.process(null, scope.getRoot()); + cfa.process(null, scopeRoot); this.cfg = cfa.getCfg(); println(this.cfg); // The size is > 1 when multiple files are compiled @@ -2230,6 +2234,7 @@ private EnvTypePair analyzeInvocationFwd( ImmutableMap typeMap = calcTypeInstantiationFwd(expr, receiver, firstArg, funType, envAfterCallee); funType = instantiateCalleeMaybeWithTTL(funType, typeMap); + callee.setTypeI(this.commonTypes.fromFunctionType(funType)); println("Instantiated function type: ", funType); } // argTypes collects types of actuals for deferred checks. @@ -2385,6 +2390,8 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) { if (!pair.type.isSubtypeOf(reqThisType)) { warnings.add(JSError.make(call, INVALID_THIS_TYPE_IN_BIND, errorMsgWithTypeDiff(reqThisType, pair.type))); + } else { + instantiateGoogBind(call.getFirstChild(), pair.type); } } @@ -2392,7 +2399,7 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) { bindComponents.parameters == null ? ImmutableList.of() : bindComponents.parameters.siblings(); - // We're passing an arraylist but don't do deferred checks for bind. + // We are passing an arraylist but don't do deferred checks for bind. env = analyzeInvocationArgumentsFwd( call, parametersIterable, boundFunType, new ArrayList(), env); // For any formal not bound here, add it to the resulting function type. @@ -2411,24 +2418,26 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) { builder.addRetType(boundFunType.getReturnType()).buildFunction())); } - private TypeEnv analyzeInvocationArgumentsFwd(Node node, Iterable args, + private TypeEnv analyzeInvocationArgumentsFwd(Node n, Iterable args, FunctionType funType, List argTypesForDeferredCheck, TypeEnv inEnv) { - checkState(NodeUtil.isCallOrNew(node) || node.isTemplateLit()); + checkState(NodeUtil.isCallOrNew(n) || n.isTemplateLit()); TypeEnv env = inEnv; - int i = node.isTemplateLit() ? 1 : 0; + int i = n.isTemplateLit() ? 1 : 0; for (Node arg : args) { JSType formalType = funType.getFormalType(i); checkState(!formalType.isBottom()); EnvTypePair pair = analyzeExprFwd(arg, env, formalType); + if (NodeUtil.isUnannotatedCallback(arg) && formalType.getFunType() != null) { + i++; + argTypesForDeferredCheck.add(null); // No deferred check needed. + continue; + } JSType argTypeForDeferredCheck = pair.type; // Allow passing undefined for an optional argument. if (funType.isOptionalArg(i) && pair.type.equals(UNDEFINED)) { argTypeForDeferredCheck = null; // No deferred check needed. } else if (!pair.type.isSubtypeOf(formalType)) { - String fnName = getReadableCalleeName(node.getFirstChild()); - JSError error = JSError.make(arg, INVALID_ARGUMENT_TYPE, Integer.toString(i + 1), fnName, - errorMsgWithTypeDiff(formalType, pair.type)); - registerMismatchAndWarn(error, pair.type, formalType); + warnAboutInvalidArgument(n, arg, i, formalType, pair.type); argTypeForDeferredCheck = null; // No deferred check needed. } else { registerImplicitUses(arg, pair.type, formalType); @@ -2440,6 +2449,73 @@ private TypeEnv analyzeInvocationArgumentsFwd(Node node, Iterable args, return env; } + private void instantiateGoogBind(Node n, JSType recvType) { + if (NodeUtil.isGoogBind(n)) { + JSType t = (JSType) n.getTypeI(); + if (t.isFunctionType()) { + FunctionType ft = t.getFunType(); + if (ft.isGeneric()) { + FunctionType instantiatedFunction = + ft.instantiateGenericsFromArgumentTypes(null, ImmutableList.of(UNKNOWN, recvType)); + n.setTypeI(this.commonTypes.fromFunctionType(instantiatedFunction)); + } + } + } + } + + private void warnAboutInvalidArgument( + Node call, Node arg, int argIndex, JSType formal, JSType actual) { + String fnName = getReadableCalleeName(call.getFirstChild()); + JSError error = JSError.make( + arg, INVALID_ARGUMENT_TYPE, Integer.toString(argIndex + 1), fnName, + errorMsgWithTypeDiff(formal, actual)); + registerMismatchAndWarn(error, actual, formal); + } + + /** + * Given a scope whose root is an unannotated callback, finds a declared type for the callback + * using the types in the callback's context. + * Similar to GlobalTypeInfoCollector#computeFnDeclaredTypeFromCallee, but not similar enough + * to use the same code for both. + */ + private void computeFnDeclaredTypeForCallback(NTIScope scope) { + Node callback = scope.getRoot(); + checkState(NodeUtil.isUnannotatedCallback(callback)); + Node call = callback.getParent(); + JSType calleeType = (JSType) call.getFirstChild().getTypeI(); + if (calleeType == null) { + return; + } + FunctionType calleeFunType = calleeType.getFunType(); + if (calleeFunType == null) { + return; + } + int argIndex = call.getIndexOfChild(callback) - 1; + JSType formalType = calleeFunType.getFormalType(argIndex); + if (formalType == null) { + return; + } + FunctionType ft = formalType.getFunType(); + if (ft == null || ft.isLoose()) { + return; + } + DeclaredFunctionType callbackDft = scope.getDeclaredFunctionType(); + JSType scopeType = this.commonTypes.fromFunctionType(callbackDft.toFunctionType()); + if (ft.isUniqueConstructor() || ft.isInterfaceDefinition()) { + warnAboutInvalidArgument(call, callback, argIndex, formalType, scopeType); + return; + } + DeclaredFunctionType declaredDft = checkNotNull(ft.toDeclaredFunctionType()); + // Only grab a type from the callee if the arity of the declared type is compatible with + // the arity of the callback. + if (ft.acceptsAnyArguments() + || callbackDft.getRequiredArity() <= declaredDft.getMaxArity()) { + scope.setDeclaredType(declaredDft); + } else { + warnAboutInvalidArgument(call, callback, argIndex, formalType, scopeType); + } + } + private EnvTypePair analyzeAssertionCall( Node callNode, TypeEnv env, AssertionFunctionSpec assertionFunctionSpec) { analyzeExprFwdIgnoreResult(callNode.getFirstChild(), env); @@ -4938,8 +5014,8 @@ private void runCheck(Map summaries, WarningReporter warnings) "Running deferred check of function: ", calleeScope.getReadableName(), " with FunctionSummary of: ", fnSummary, " and callsite ret: ", expectedRetType, " args: ", argTypes); - if (this.expectedRetType != null && - !fnSummary.getReturnType().isSubtypeOf(this.expectedRetType)) { + if (this.expectedRetType != null + && !fnSummary.getReturnType().isSubtypeOf(this.expectedRetType)) { warnings.add(JSError.make( this.callSite, INVALID_INFERRED_RETURN_TYPE, errorMsgWithTypeDiff( diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index f1f557c9788..7c88cd0794f 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -4605,6 +4605,26 @@ public static Node getFunctionParameters(Node fnNode) { return fnNode.getSecondChild(); } + /** + * Counts the parameters of a function that are not marked optional or varargs. + * In ES5 functions, that's all parameters, in ES6 it's a prefix of the parameters. + * The result is an overapproximation: if a parameter is not marked as optional, it may still + * be optional, but it doesn't have a default value, and wasn't marked as optional + * during transpilation. + */ + public static int getApproxRequiredArity(Node fun) { + checkArgument(fun.isFunction()); + checkArgument(getBestJSDocInfo(fun) == null, "Expected unannotated function, found: %s", fun); + int result = 0; + for (Node param : fun.getSecondChild().children()) { + if (param.isOptionalArg() || param.isVarArgs()) { + break; + } + result++; + } + return result; + } + static boolean isConstantVar(Node node, @Nullable Scope scope) { if (isConstantName(node)) { return true; diff --git a/src/com/google/javascript/jscomp/newtypes/DeclaredFunctionType.java b/src/com/google/javascript/jscomp/newtypes/DeclaredFunctionType.java index 22f134dab47..64a6df4ddd7 100644 --- a/src/com/google/javascript/jscomp/newtypes/DeclaredFunctionType.java +++ b/src/com/google/javascript/jscomp/newtypes/DeclaredFunctionType.java @@ -152,8 +152,7 @@ public int getMaxArity() { } private int getSyntacticArity() { - return this.getOptionalArity() - + (this.restFormals == null ? 0 : 1); + return this.getOptionalArity() + (this.restFormals == null ? 0 : 1); } public boolean hasRestFormals() { diff --git a/src/com/google/javascript/jscomp/newtypes/FunctionType.java b/src/com/google/javascript/jscomp/newtypes/FunctionType.java index bcd1843a1c2..2b52a1dc5be 100644 --- a/src/com/google/javascript/jscomp/newtypes/FunctionType.java +++ b/src/com/google/javascript/jscomp/newtypes/FunctionType.java @@ -350,11 +350,11 @@ public JSType getRestFormalsType() { /** * Returns the formal parameter in the given (0-indexed) position, * or null if the position is past the end of the parameter list. - * - * @throws IllegalStateException if this is the top function (rather than returning bottom). */ public JSType getFormalType(int argpos) { - checkState(!isTopFunction()); + if (isTopFunction()) { + return this.commonTypes.UNKNOWN; + } int numReqFormals = requiredFormals.size(); if (argpos < numReqFormals) { return requiredFormals.get(argpos); @@ -659,7 +659,7 @@ static void whyNotSubtypeOf(FunctionType f1, FunctionType f2, * it to represent, for example, a constructor of Foos with whatever * arguments. */ - private boolean acceptsAnyArguments() { + public boolean acceptsAnyArguments() { return this.requiredFormals.isEmpty() && this.optionalFormals.isEmpty() && this.restFormals != null && this.restFormals.isUnknown(); } @@ -719,8 +719,7 @@ private boolean isSubtypeOfHelper(FunctionType other, boolean isMethodOverrideCh } if (other.restFormals != null) { - int thisMaxTotalArity = - this.requiredFormals.size() + this.optionalFormals.size(); + int thisMaxTotalArity = this.requiredFormals.size() + this.optionalFormals.size(); if (this.restFormals != null) { thisMaxTotalArity++; } @@ -967,8 +966,7 @@ static FunctionType meet(FunctionType f1, FunctionType f2) { builder.addOptFormal(optFormalType); } if (f1.restFormals != null || f2.restFormals != null) { - JSType restFormalsType = - JSType.nullAcceptingJoin(f1.restFormals, f2.restFormals); + JSType restFormalsType = JSType.nullAcceptingJoin(f1.restFormals, f2.restFormals); if (restFormalsType.isBottom()) { return commonTypes.BOTTOM_FUNCTION; } diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTTLTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTTLTest.java index ffc0da02cc1..fa9f426dd44 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTTLTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTTLTest.java @@ -1079,4 +1079,19 @@ public void testPromiseThen() { "MyPromise.prototype.then = function(x, y, z) { return any(); };", "var x = (new MyPromise).then(null);")); } + + public void testInferUnannotatedCallback() { + typeCheck(LINE_JOINER.join( + "/**", + " * @param {!Promise} a", + " * @return {!Promise}", + " */", + "function f(a) {", + " var b = a;", + " return b.then(function(x) {", + " return x - 1;", + " });", + "}"), + NewTypeInference.INVALID_OPERAND_TYPE); + } } diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java index ce78dc6f4b6..d719698795f 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java @@ -1169,6 +1169,13 @@ public void testDeferredChecks() { " }", "}"), NewTypeInference.INVALID_ARGUMENT_TYPE); + + typeCheck(LINE_JOINER.join( + "function g() {", + " f(function() {});", + "}", + "/** @param {function()} x */", + "function f(x) {}")); } public void testShadowing() { @@ -12906,7 +12913,7 @@ public void testAbstractMethodsAreTypedCorrectly() { NewTypeInference.INVALID_ARGUMENT_TYPE); } - public void testUseJsdocOfCalleeForUnannotatedFunctionsInArgumentPosition() { + public void testInferUnannotatedCallbackFromCallee() { typeCheck(LINE_JOINER.join( "/** @constructor */", "function Foo() { /** @type {string} */ this.prop = 'asdf'; }", @@ -12937,7 +12944,7 @@ public void testUseJsdocOfCalleeForUnannotatedFunctionsInArgumentPosition() { typeCheck(LINE_JOINER.join( "/** @param {function(string, ...string)} fun */", "function f(fun) {}", - "f(function(str, maybeStrs) { str - 5; });"), + "f(function(str, maybeStr) { str - 5; });"), NewTypeInference.INVALID_OPERAND_TYPE); typeCheck(LINE_JOINER.join( @@ -13010,6 +13017,87 @@ public void testUseJsdocOfCalleeForUnannotatedFunctionsInArgumentPosition() { "function f(x, fun) { fun(x, x); }", "f(new Foo, function(x) { x.myprop = 'asdf'; });"), NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheck(LINE_JOINER.join( + "function f(/** function(number):? */ x) {}", + "f(function(x, y) {});"), + NewTypeInference.INVALID_ARGUMENT_TYPE); + + typeCheck(LINE_JOINER.join( + "/**", + " * @param {T} x", + " * @param {function(T)} y", + " * @template T", + " */", + "function f(x, y) {}", + "var x = 1;", + "f(x, function(x) { var /** string */ s = x; });"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheck(LINE_JOINER.join( + "var /** !Array */ a = [1,2,3];", + "var b = a;", + "b.forEach(function(x) {", + " var /** string */ s = x;", + "});"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheck(LINE_JOINER.join( + "/** @constructor */", + "function Foo() {", + " /** @type {number} */", + " this.p = 1;", + "}", + "Foo.prototype.m = function() {", + " var a = [1,2,3];", + " a.forEach(function(num) {", + " var /** string */ s = this.p;", + " }, this);", + "};"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheck(LINE_JOINER.join( + "/** @param {function(number, number=):number} fun */", + "function f(fun) {}", + "f(function(x, y) { return y || 0; });")); + + typeCheck(LINE_JOINER.join( + "/** @param {function(number, number=):number} fun */", + "function f(fun) {}", + "f(function(x, y, z) { return y || 0; });"), + NewTypeInference.INVALID_ARGUMENT_TYPE); + + typeCheck(LINE_JOINER.join( + "function f(x) {", + " /**", + " * @param {T} x", + " * @param {function(T=,T=)} fun", + " * @template T", + " */", + " function g(x, fun) {}", + " g(x, function(x, y) {});", + "}")); + + typeCheck(LINE_JOINER.join( + "/** @param {function(number, number=)} fun */", + "function f(fun) {}", + "f(function(x, y) {", + " /** @const */", + " var z = x;", + " function g() {", + " var /** string */ s = z;", + " }", + "});"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + typeCheck(LINE_JOINER.join( + "/** @param {function(number, number=)} fun */", + "function f(fun) {}", + "var g = f;", + "g(function(x, y) {", + " var /** string */ z = x;", + "});"), + NewTypeInference.MISTYPED_ASSIGN_RHS); } public void testNamespacesWithNonEmptyObjectLiteral() { @@ -13920,6 +14008,21 @@ public void testFunctionBind() { "(function(/** number */ x) {", " return this.p + x;", "}).bind(new Foo);")); + + typeCheck(LINE_JOINER.join( + "(function (x) {", + " var /** { a: number, b: string} */ y = this;", + "}).bind([1, 2, 3]);"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + + // TODO(dimvar): here, we could grab a signature for the anonymous function using the + // arguments to .bind, to get better type checking. Then, the warning would be + // MISTYPED_ASSIGN_RHS instead of INVALID_ARGUMENT_TYPE. + typeCheck(LINE_JOINER.join( + "(function (x) {", + " var /** string */ y = x;", + "}).bind(null, 123);"), + NewTypeInference.INVALID_ARGUMENT_TYPE); } public void testClosureStyleFunctionBind() { @@ -16769,6 +16872,15 @@ public void testConstructorInitializedWithCall() { "/** @constructor */", "Foo.Bar = Foo.bind(undefined, function() {});"), NewTypeInference.CANNOT_BIND_CTOR); + + // We can't grab a signature for the callback from the callee's type, but shouldn't crash. + typeCheckCustomExterns( + LINE_JOINER.join( + DEFAULT_EXTERNS, + "var TestCase;"), + LINE_JOINER.join( + "/** @constructor */", + "var Foo = TestCase(function(x) {});")); } public void testLocalWithCallableObjectType() { @@ -22573,4 +22685,55 @@ public void testInheritedPropertyTypes() { "function Baz() {}"), GlobalTypeInfoCollector.ANCESTOR_TYPES_HAVE_INCOMPATIBLE_PROPERTIES); } + + public void testNoSpuriousWarningWithGoogBind() { + typeCheck(LINE_JOINER.join( + "/**", + " * @param {?function(this:T, ...)} fn", + " * @param {T} selfObj", + " * @template T", + " */", + "function googBind(fn, selfObj, var_args) {};", + "googBind(function(x, y, z) {}, [1,2,3]);")); + } + + public void testNestedGoogBinds() { + typeCheck(LINE_JOINER.join( + CLOSURE_BASE, + "/** @constructor */", + "function Foo() {}", + "Foo.prototype.m = function() {", + " goog.bind(function outer() {", + " goog.bind(function inner() {}, this);", + " }, this);", + "};")); + + typeCheck(LINE_JOINER.join( + CLOSURE_BASE, + "/** @constructor */", + "function Bar() {}", + "/** @constructor */", + "function Foo() {}", + "Foo.prototype.m = function() {", + " goog.bind(function outer() {", + " goog.bind(function inner() {", + " var /** !Bar */ x = this;", + " }, this);", + " }, this);", + "};"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + } + + public void testInferCallbackEarlyToInferConst() { + typeCheck(LINE_JOINER.join( + "function f(/** function(number) */ x) {}", + "f(function (x) {", + " /** @const */", + " var y = x;", + " function g() {", + " var /** string */ s = y;", + " }", + "});"), + NewTypeInference.MISTYPED_ASSIGN_RHS); + } } diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java index c333a20807e..1aca6535eb3 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java @@ -70,7 +70,16 @@ boolean checkTranspiled() { "goog.asserts.assertObject;", "goog.asserts.assertString;", "goog.partial;", - "goog.bind;", + "/**", + " * @param {?function(this:T, ...)} fn", + " * @param {T} selfObj", + " * @param {...*} var_args", + " * @return {!Function}", + " * @template T", + " */", + "goog.bind = function(fn, selfObj, var_args) {", + " return function() {};", + "};", "goog.isNull;", "goog.isDef;", "goog.isDefAndNotNull;", diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceWithTranspilationTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceWithTranspilationTest.java index d35248eada5..31ea90d7b0b 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceWithTranspilationTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceWithTranspilationTest.java @@ -699,4 +699,66 @@ public void testAnalyzeInsideNamespaceObjectLiterals() { "};"), NewTypeInference.INVALID_OPERAND_TYPE); } + + public void testGetTypeFromCalleeForCallbackWithVariableArity() { + typeCheck(LINE_JOINER.join( + "function f(/** function(): ? */ x) {}", + "f((...args) => args);")); + + typeCheck(LINE_JOINER.join( + "function f(/** function(): ? */ x) {}", + "f((x, y, ...args) => args);"), + NewTypeInference.INVALID_ARGUMENT_TYPE); + + typeCheck(LINE_JOINER.join( + "function f(/** function(?, ?): ? */ x) {}", + "f((x, ...args) => args);")); + + typeCheck(LINE_JOINER.join( + "function f(/** function(number, ...number): ? */ x) {}", + "f((x, ...args) => { var /** !Array */ n = args; });")); + + // TODO(dimvar): fix when we typecheck ES6 natively + typeCheck(LINE_JOINER.join( + "function f(/** function(number, ...number): ? */ x) {}", + "f((x, ...args) => { var /** !Array */ n = args; });")); + + typeCheck(LINE_JOINER.join( + "/** @param {function(number, number=)} fun */", + "function f(fun) {}", + "f(function(x, y=0) {});")); + + typeCheck(LINE_JOINER.join( + "/** @param {function(number=, number=)} fun */", + "function f(fun) {}", + "f(function(x=0, y=0) {});")); + + typeCheck(LINE_JOINER.join( + "/** @param {function(number=, number=)} fun */", + "function f(fun) {}", + "f(function(x, y=0) {", + " var /** number */ n = x;", + "});"), + NewTypeInference.MISTYPED_ASSIGN_RHS); // x is number|undefined + + typeCheck(LINE_JOINER.join( + "/** @param {function(...number)} fun */", + "function f(fun) {}", + "f(function(x) { var /** number */ y = x || 0; });")); + + // TODO(dimvar): there should be a warning here because x is number|undefined + typeCheck(LINE_JOINER.join( + "/** @param {function(...number)} fun */", + "function f(fun) {}", + "f(function(x) { var /** number */ y = x; });")); + + // We could consider warning in this case (whenever the size of callback's param list, + // including optionals and rests, exceeds the declared type's maximum arity) because the later + // parameters will never be passed anything, so it's a code smell to have it. But it's not a + // type error; we would need a different diagnostic type than invalid_argument_type. + typeCheck(LINE_JOINER.join( + "/** @param {function(number)} fun */", + "function f(fun) {}", + "f(function(x=0, y=0) {});")); + } }