From 87ad612e701fe68c1a4f953250fc5a56d3b6a027 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 11 Jul 2018 17:26:35 -1000 Subject: [PATCH 1/3] Additional arity check following instantiation of generic rest parameter --- src/compiler/checker.ts | 67 +++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2e48c4ca1bc..5c1855a8acf 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -18200,7 +18200,6 @@ namespace ts { function hasCorrectArity(node: CallLikeExpression, args: ReadonlyArray, signature: Signature, signatureHelpTrailingComma = false) { let argCount: number; // Apparent number of arguments we will have in this call - let typeArguments: NodeArray | undefined; // Type arguments (undefined if none) let callIsIncomplete = false; // In incomplete call we want to be lenient when we have too few arguments let spreadArgIndex = -1; @@ -18213,7 +18212,6 @@ namespace ts { // Even if the call is incomplete, we'll have a missing expression as our last argument, // so we can say the count is just the arg list length argCount = args.length; - typeArguments = node.typeArguments; if (node.template.kind === SyntaxKind.TemplateExpression) { // If a tagged template expression lacks a tail literal, the call is incomplete. @@ -18231,7 +18229,6 @@ namespace ts { } } else if (node.kind === SyntaxKind.Decorator) { - typeArguments = undefined; argCount = getEffectiveArgumentCount(node, /*args*/ undefined!, signature); } else { @@ -18246,14 +18243,9 @@ namespace ts { // If we are missing the close parenthesis, the call is incomplete. callIsIncomplete = node.arguments.end === node.end; - typeArguments = node.typeArguments; spreadArgIndex = getSpreadArgumentIndex(args); } - if (!hasCorrectTypeArgumentArity(signature, typeArguments)) { - return false; - } - // If a spread argument is present, check that it corresponds to a rest parameter or at least that it's in the valid range. if (spreadArgIndex >= 0) { return spreadArgIndex >= getMinArgumentCount(signature) && (hasEffectiveRestParameter(signature) || spreadArgIndex < getParameterCount(signature)); @@ -18903,6 +18895,29 @@ namespace ts { } } + function getArgumentArityError(node: Node, signatures: ReadonlyArray, args: ReadonlyArray) { + let min = Number.POSITIVE_INFINITY; + let max = Number.NEGATIVE_INFINITY; + for (const sig of signatures) { + min = Math.min(min, getMinArgumentCount(sig)); + max = Math.max(max, getParameterCount(sig)); + } + const hasRestParameter = some(signatures, hasEffectiveRestParameter); + const hasSpreadArgument = getSpreadArgumentIndex(args) > -1; + const paramCount = hasRestParameter ? min : + min < max ? min + "-" + max : + min; + let argCount = args.length; + if (argCount <= max && hasSpreadArgument) { + argCount--; + } + const error = hasRestParameter && hasSpreadArgument ? Diagnostics.Expected_at_least_0_arguments_but_got_1_or_more : + hasRestParameter ? Diagnostics.Expected_at_least_0_arguments_but_got_1 : + hasSpreadArgument ? Diagnostics.Expected_0_arguments_but_got_1_or_more : + Diagnostics.Expected_0_arguments_but_got_1; + return createDiagnosticForNode(node, error, paramCount, argCount); + } + function getTypeArgumentArityError(node: Node, signatures: ReadonlyArray, typeArguments: NodeArray) { let min = Infinity; let max = -Infinity; @@ -18993,6 +19008,7 @@ namespace ts { // foo(0); // let candidateForArgumentError: Signature | undefined; + let candidateForArgumentArityError: Signature | undefined; let candidateForTypeArgumentError: Signature | undefined; let result: Signature | undefined; @@ -19037,6 +19053,9 @@ namespace ts { // an error, we don't need to exclude any arguments, although it would cause no harm to do so. checkApplicableSignature(node, args!, candidateForArgumentError, assignableRelation, /*excludeArgument*/ undefined, /*reportErrors*/ true); } + else if (candidateForArgumentArityError) { + diagnostics.add(getArgumentArityError(node, [candidateForArgumentArityError], args!)); + } else if (candidateForTypeArgumentError) { checkTypeArguments(candidateForTypeArgumentError, (node as CallExpression | TaggedTemplateExpression).typeArguments!, /*reportErrors*/ true, fallbackError); } @@ -19044,26 +19063,7 @@ namespace ts { diagnostics.add(getTypeArgumentArityError(node, signatures, typeArguments)); } else if (args) { - let min = Number.POSITIVE_INFINITY; - let max = Number.NEGATIVE_INFINITY; - for (const sig of signatures) { - min = Math.min(min, getMinArgumentCount(sig)); - max = Math.max(max, getParameterCount(sig)); - } - const hasRestParameter = some(signatures, hasEffectiveRestParameter); - const hasSpreadArgument = getSpreadArgumentIndex(args) > -1; - const paramCount = hasRestParameter ? min : - min < max ? min + "-" + max : - min; - let argCount = args.length; - if (argCount <= max && hasSpreadArgument) { - argCount--; - } - const error = hasRestParameter && hasSpreadArgument ? Diagnostics.Expected_at_least_0_arguments_but_got_1_or_more : - hasRestParameter ? Diagnostics.Expected_at_least_0_arguments_but_got_1 : - hasSpreadArgument ? Diagnostics.Expected_0_arguments_but_got_1_or_more : - Diagnostics.Expected_0_arguments_but_got_1; - diagnostics.add(createDiagnosticForNode(node, error, paramCount, argCount)); + diagnostics.add(getArgumentArityError(node, signatures, args)); } else if (fallbackError) { diagnostics.add(createDiagnosticForNode(node, fallbackError)); @@ -19104,11 +19104,12 @@ namespace ts { function chooseOverload(candidates: Signature[], relation: Map, signatureHelpTrailingComma = false) { candidateForArgumentError = undefined; + candidateForArgumentArityError = undefined; candidateForTypeArgumentError = undefined; if (isSingleNonGenericCandidate) { const candidate = candidates[0]; - if (!hasCorrectArity(node, args!, candidate, signatureHelpTrailingComma)) { + if (typeArguments || !hasCorrectArity(node, args!, candidate, signatureHelpTrailingComma)) { return undefined; } if (!checkApplicableSignature(node, args!, candidate, relation, excludeArgument, /*reportErrors*/ false)) { @@ -19120,7 +19121,7 @@ namespace ts { for (let candidateIndex = 0; candidateIndex < candidates.length; candidateIndex++) { const originalCandidate = candidates[candidateIndex]; - if (!hasCorrectArity(node, args!, originalCandidate, signatureHelpTrailingComma)) { + if (!hasCorrectTypeArgumentArity(originalCandidate, typeArguments) || !hasCorrectArity(node, args!, originalCandidate, signatureHelpTrailingComma)) { continue; } @@ -19148,6 +19149,12 @@ namespace ts { } const isJavascript = isInJavaScriptFile(candidate.declaration); candidate = getSignatureInstantiation(candidate, typeArgumentTypes, isJavascript); + // If the original signature has a rest type parameter, instantiation may produce a + // signature with different arity and we need to perform another arity check. + if (getRestTypeParameter(originalCandidate) && !hasCorrectArity(node, args!, candidate, signatureHelpTrailingComma)) { + candidateForArgumentArityError = candidate; + break; + } } if (!checkApplicableSignature(node, args!, candidate, relation, excludeArgument, /*reportErrors*/ false)) { candidateForArgumentError = candidate; From 55180f7725c0eb3ef478d07f7611a66b8de9d4a6 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 12 Jul 2018 07:07:13 -1000 Subject: [PATCH 2/3] Add tests --- tests/cases/conformance/types/rest/genericRestArity.ts | 8 ++++++++ .../conformance/types/rest/genericRestArityStrict.ts | 10 ++++++++++ 2 files changed, 18 insertions(+) create mode 100644 tests/cases/conformance/types/rest/genericRestArity.ts create mode 100644 tests/cases/conformance/types/rest/genericRestArityStrict.ts diff --git a/tests/cases/conformance/types/rest/genericRestArity.ts b/tests/cases/conformance/types/rest/genericRestArity.ts new file mode 100644 index 00000000000..c417b271b3b --- /dev/null +++ b/tests/cases/conformance/types/rest/genericRestArity.ts @@ -0,0 +1,8 @@ +// Repro from #25559 + +declare function call( + handler: (...args: TS) => void, + ...args: TS): void; + +call((x: number, y: number) => x + y); +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); diff --git a/tests/cases/conformance/types/rest/genericRestArityStrict.ts b/tests/cases/conformance/types/rest/genericRestArityStrict.ts new file mode 100644 index 00000000000..673479fea59 --- /dev/null +++ b/tests/cases/conformance/types/rest/genericRestArityStrict.ts @@ -0,0 +1,10 @@ +// @strict: true + +// Repro from #25559 + +declare function call( + handler: (...args: TS) => void, + ...args: TS): void; + +call((x: number, y: number) => x + y); +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); From e88f63295b29dc39108ff70bd3bc7dd6c2b2d8a0 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 12 Jul 2018 07:07:23 -1000 Subject: [PATCH 3/3] Accept new baselines --- .../reference/genericRestArity.errors.txt | 18 ++++++++ tests/baselines/reference/genericRestArity.js | 15 +++++++ .../reference/genericRestArity.symbols | 30 +++++++++++++ .../reference/genericRestArity.types | 43 +++++++++++++++++++ .../genericRestArityStrict.errors.txt | 15 +++++++ .../reference/genericRestArityStrict.js | 16 +++++++ .../reference/genericRestArityStrict.symbols | 30 +++++++++++++ .../reference/genericRestArityStrict.types | 43 +++++++++++++++++++ 8 files changed, 210 insertions(+) create mode 100644 tests/baselines/reference/genericRestArity.errors.txt create mode 100644 tests/baselines/reference/genericRestArity.js create mode 100644 tests/baselines/reference/genericRestArity.symbols create mode 100644 tests/baselines/reference/genericRestArity.types create mode 100644 tests/baselines/reference/genericRestArityStrict.errors.txt create mode 100644 tests/baselines/reference/genericRestArityStrict.js create mode 100644 tests/baselines/reference/genericRestArityStrict.symbols create mode 100644 tests/baselines/reference/genericRestArityStrict.types diff --git a/tests/baselines/reference/genericRestArity.errors.txt b/tests/baselines/reference/genericRestArity.errors.txt new file mode 100644 index 00000000000..71b814e324f --- /dev/null +++ b/tests/baselines/reference/genericRestArity.errors.txt @@ -0,0 +1,18 @@ +tests/cases/conformance/types/rest/genericRestArity.ts(7,1): error TS2554: Expected 3 arguments, but got 1. +tests/cases/conformance/types/rest/genericRestArity.ts(8,1): error TS2554: Expected 3 arguments, but got 8. + + +==== tests/cases/conformance/types/rest/genericRestArity.ts (2 errors) ==== + // Repro from #25559 + + declare function call( + handler: (...args: TS) => void, + ...args: TS): void; + + call((x: number, y: number) => x + y); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2554: Expected 3 arguments, but got 1. + call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2554: Expected 3 arguments, but got 8. + \ No newline at end of file diff --git a/tests/baselines/reference/genericRestArity.js b/tests/baselines/reference/genericRestArity.js new file mode 100644 index 00000000000..a59ab66e1a8 --- /dev/null +++ b/tests/baselines/reference/genericRestArity.js @@ -0,0 +1,15 @@ +//// [genericRestArity.ts] +// Repro from #25559 + +declare function call( + handler: (...args: TS) => void, + ...args: TS): void; + +call((x: number, y: number) => x + y); +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); + + +//// [genericRestArity.js] +// Repro from #25559 +call(function (x, y) { return x + y; }); +call(function (x, y) { return x + y; }, 1, 2, 3, 4, 5, 6, 7); diff --git a/tests/baselines/reference/genericRestArity.symbols b/tests/baselines/reference/genericRestArity.symbols new file mode 100644 index 00000000000..65292356029 --- /dev/null +++ b/tests/baselines/reference/genericRestArity.symbols @@ -0,0 +1,30 @@ +=== tests/cases/conformance/types/rest/genericRestArity.ts === +// Repro from #25559 + +declare function call( +>call : Symbol(call, Decl(genericRestArity.ts, 0, 0)) +>TS : Symbol(TS, Decl(genericRestArity.ts, 2, 22)) + + handler: (...args: TS) => void, +>handler : Symbol(handler, Decl(genericRestArity.ts, 2, 44)) +>args : Symbol(args, Decl(genericRestArity.ts, 3, 14)) +>TS : Symbol(TS, Decl(genericRestArity.ts, 2, 22)) + + ...args: TS): void; +>args : Symbol(args, Decl(genericRestArity.ts, 3, 35)) +>TS : Symbol(TS, Decl(genericRestArity.ts, 2, 22)) + +call((x: number, y: number) => x + y); +>call : Symbol(call, Decl(genericRestArity.ts, 0, 0)) +>x : Symbol(x, Decl(genericRestArity.ts, 6, 6)) +>y : Symbol(y, Decl(genericRestArity.ts, 6, 16)) +>x : Symbol(x, Decl(genericRestArity.ts, 6, 6)) +>y : Symbol(y, Decl(genericRestArity.ts, 6, 16)) + +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); +>call : Symbol(call, Decl(genericRestArity.ts, 0, 0)) +>x : Symbol(x, Decl(genericRestArity.ts, 7, 6)) +>y : Symbol(y, Decl(genericRestArity.ts, 7, 16)) +>x : Symbol(x, Decl(genericRestArity.ts, 7, 6)) +>y : Symbol(y, Decl(genericRestArity.ts, 7, 16)) + diff --git a/tests/baselines/reference/genericRestArity.types b/tests/baselines/reference/genericRestArity.types new file mode 100644 index 00000000000..61bd75d9470 --- /dev/null +++ b/tests/baselines/reference/genericRestArity.types @@ -0,0 +1,43 @@ +=== tests/cases/conformance/types/rest/genericRestArity.ts === +// Repro from #25559 + +declare function call( +>call : (handler: (...args: TS) => void, ...args: TS) => void +>TS : TS + + handler: (...args: TS) => void, +>handler : (...args: TS) => void +>args : TS +>TS : TS + + ...args: TS): void; +>args : TS +>TS : TS + +call((x: number, y: number) => x + y); +>call((x: number, y: number) => x + y) : any +>call : (handler: (...args: TS) => void, ...args: TS) => void +>(x: number, y: number) => x + y : (x: number, y: number) => number +>x : number +>y : number +>x + y : number +>x : number +>y : number + +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); +>call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7) : any +>call : (handler: (...args: TS) => void, ...args: TS) => void +>(x: number, y: number) => x + y : (x: number, y: number) => number +>x : number +>y : number +>x + y : number +>x : number +>y : number +>1 : 1 +>2 : 2 +>3 : 3 +>4 : 4 +>5 : 5 +>6 : 6 +>7 : 7 + diff --git a/tests/baselines/reference/genericRestArityStrict.errors.txt b/tests/baselines/reference/genericRestArityStrict.errors.txt new file mode 100644 index 00000000000..9a77f8d8498 --- /dev/null +++ b/tests/baselines/reference/genericRestArityStrict.errors.txt @@ -0,0 +1,15 @@ +tests/cases/conformance/types/rest/genericRestArityStrict.ts(7,6): error TS2345: Argument of type '(x: number, y: number) => number' is not assignable to parameter of type '() => void'. + + +==== tests/cases/conformance/types/rest/genericRestArityStrict.ts (1 errors) ==== + // Repro from #25559 + + declare function call( + handler: (...args: TS) => void, + ...args: TS): void; + + call((x: number, y: number) => x + y); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2345: Argument of type '(x: number, y: number) => number' is not assignable to parameter of type '() => void'. + call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); + \ No newline at end of file diff --git a/tests/baselines/reference/genericRestArityStrict.js b/tests/baselines/reference/genericRestArityStrict.js new file mode 100644 index 00000000000..5628eada2f5 --- /dev/null +++ b/tests/baselines/reference/genericRestArityStrict.js @@ -0,0 +1,16 @@ +//// [genericRestArityStrict.ts] +// Repro from #25559 + +declare function call( + handler: (...args: TS) => void, + ...args: TS): void; + +call((x: number, y: number) => x + y); +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); + + +//// [genericRestArityStrict.js] +"use strict"; +// Repro from #25559 +call(function (x, y) { return x + y; }); +call(function (x, y) { return x + y; }, 1, 2, 3, 4, 5, 6, 7); diff --git a/tests/baselines/reference/genericRestArityStrict.symbols b/tests/baselines/reference/genericRestArityStrict.symbols new file mode 100644 index 00000000000..df51a85f95d --- /dev/null +++ b/tests/baselines/reference/genericRestArityStrict.symbols @@ -0,0 +1,30 @@ +=== tests/cases/conformance/types/rest/genericRestArityStrict.ts === +// Repro from #25559 + +declare function call( +>call : Symbol(call, Decl(genericRestArityStrict.ts, 0, 0)) +>TS : Symbol(TS, Decl(genericRestArityStrict.ts, 2, 22)) + + handler: (...args: TS) => void, +>handler : Symbol(handler, Decl(genericRestArityStrict.ts, 2, 44)) +>args : Symbol(args, Decl(genericRestArityStrict.ts, 3, 14)) +>TS : Symbol(TS, Decl(genericRestArityStrict.ts, 2, 22)) + + ...args: TS): void; +>args : Symbol(args, Decl(genericRestArityStrict.ts, 3, 35)) +>TS : Symbol(TS, Decl(genericRestArityStrict.ts, 2, 22)) + +call((x: number, y: number) => x + y); +>call : Symbol(call, Decl(genericRestArityStrict.ts, 0, 0)) +>x : Symbol(x, Decl(genericRestArityStrict.ts, 6, 6)) +>y : Symbol(y, Decl(genericRestArityStrict.ts, 6, 16)) +>x : Symbol(x, Decl(genericRestArityStrict.ts, 6, 6)) +>y : Symbol(y, Decl(genericRestArityStrict.ts, 6, 16)) + +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); +>call : Symbol(call, Decl(genericRestArityStrict.ts, 0, 0)) +>x : Symbol(x, Decl(genericRestArityStrict.ts, 7, 6)) +>y : Symbol(y, Decl(genericRestArityStrict.ts, 7, 16)) +>x : Symbol(x, Decl(genericRestArityStrict.ts, 7, 6)) +>y : Symbol(y, Decl(genericRestArityStrict.ts, 7, 16)) + diff --git a/tests/baselines/reference/genericRestArityStrict.types b/tests/baselines/reference/genericRestArityStrict.types new file mode 100644 index 00000000000..53c784d0938 --- /dev/null +++ b/tests/baselines/reference/genericRestArityStrict.types @@ -0,0 +1,43 @@ +=== tests/cases/conformance/types/rest/genericRestArityStrict.ts === +// Repro from #25559 + +declare function call( +>call : (handler: (...args: TS) => void, ...args: TS) => void +>TS : TS + + handler: (...args: TS) => void, +>handler : (...args: TS) => void +>args : TS +>TS : TS + + ...args: TS): void; +>args : TS +>TS : TS + +call((x: number, y: number) => x + y); +>call((x: number, y: number) => x + y) : any +>call : (handler: (...args: TS) => void, ...args: TS) => void +>(x: number, y: number) => x + y : (x: number, y: number) => number +>x : number +>y : number +>x + y : number +>x : number +>y : number + +call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7); +>call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7) : void +>call : (handler: (...args: TS) => void, ...args: TS) => void +>(x: number, y: number) => x + y : (x: number, y: number) => number +>x : number +>y : number +>x + y : number +>x : number +>y : number +>1 : 1 +>2 : 2 +>3 : 3 +>4 : 4 +>5 : 5 +>6 : 6 +>7 : 7 +