From 8657dc10a74086d93058a59e072f2e7edcec5b2f Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 16 Nov 2017 09:31:50 -0800 Subject: [PATCH 1/3] Error on excess spread arguments Make the *technically* correct construction illegal: ```ts declare function f(n: number): void; declare var ns: number[]; f(1, ...ns); ``` This call only makes sense if `ns = []`, but in that case, why pass `ns` at all? Allowing this call masks other errors when functions are refactored to have fewer parameters, or to stop using rest parameters: ```ts declare function old(...ns: number[]): void; declare function new(ns: number | number[]): void; old(1, ...ns); // Fine! new(1, ...ns); // Should error! ``` This change the error for excess spread arguments to be more understandable: "Expected 3 arguments, but got least 4". Previously the error would have been "Expected 3 argument, but got at least 3", which is, again, technically correct, but not understandable. --- src/compiler/checker.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 022aa18a50e..e7f954f7295 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -15790,10 +15790,10 @@ namespace ts { return false; } - // If spread arguments are present, check that they correspond to a rest parameter. If so, no - // further checking is necessary. + // 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 isRestParameterIndex(signature, spreadArgIndex) || spreadArgIndex >= signature.minArgumentCount; + return isRestParameterIndex(signature, spreadArgIndex) || + signature.minArgumentCount <= spreadArgIndex && spreadArgIndex < signature.parameters.length; } // Too many arguments implies incorrect arity. @@ -16501,7 +16501,10 @@ namespace ts { const paramCount = hasRestParameter ? min : min < max ? min + "-" + max : min; - const argCount = args.length - (hasSpreadArgument ? 1 : 0); + let argCount = args.length; + if (argCount <= max && hasSpreadArgument) { + argCount--; + } const error = hasRestParameter && hasSpreadArgument ? Diagnostics.Expected_at_least_0_arguments_but_got_a_minimum_of_1 : hasRestParameter ? Diagnostics.Expected_at_least_0_arguments_but_got_1 : hasSpreadArgument ? Diagnostics.Expected_0_arguments_but_got_a_minimum_of_1 : From 865f3281bcf91fbf76a90aea5c86496af585f006 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 16 Nov 2017 09:39:31 -0800 Subject: [PATCH 2/3] Update spread tests w/new excess arguments error --- .../reference/callWithSpread2.errors.txt | 32 ++++++++++--------- tests/baselines/reference/callWithSpread2.js | 12 ++----- .../reference/callWithSpread2.symbols | 18 +---------- .../baselines/reference/callWithSpread2.types | 28 +--------------- .../functionCalls/callWithSpread2.ts | 6 +--- 5 files changed, 22 insertions(+), 74 deletions(-) diff --git a/tests/baselines/reference/callWithSpread2.errors.txt b/tests/baselines/reference/callWithSpread2.errors.txt index 89617ab6074..7272b1b9f3b 100644 --- a/tests/baselines/reference/callWithSpread2.errors.txt +++ b/tests/baselines/reference/callWithSpread2.errors.txt @@ -1,21 +1,23 @@ -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(30,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(22,1): error TS2556: Expected 1 arguments, but got a minimum of 2. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(23,1): error TS2556: Expected 0 arguments, but got a minimum of 1. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(26,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(31,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(27,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(32,13): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(28,13): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,13): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(29,13): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(30,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(35,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(31,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(36,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(37,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(38,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(32,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. -==== tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts (9 errors) ==== +==== tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts (11 errors) ==== declare function all(a?: number, b?: number): void; declare function weird(a?: number | string, b?: number | string): void; declare function prefix(s: string, a?: number, b?: number): void; @@ -36,13 +38,13 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(38,1): erro rest("d", ...ns) - // this covers the arguments case + // extra arguments normal("g", ...ns) - normal("h", ...mixed) - normal("i", ...tuple) + ~~~~~~~~~~~~~~~~~~ +!!! error TS2556: Expected 1 arguments, but got a minimum of 2. thunk(...ns) - thunk(...mixed) - thunk(...tuple) + ~~~~~~~~~~~~ +!!! error TS2556: Expected 0 arguments, but got a minimum of 1. // bad all(...mixed) diff --git a/tests/baselines/reference/callWithSpread2.js b/tests/baselines/reference/callWithSpread2.js index 55296d924f7..27bcc893ca8 100644 --- a/tests/baselines/reference/callWithSpread2.js +++ b/tests/baselines/reference/callWithSpread2.js @@ -19,13 +19,9 @@ prefix("a", ...ns) rest("d", ...ns) -// this covers the arguments case +// extra arguments normal("g", ...ns) -normal("h", ...mixed) -normal("i", ...tuple) thunk(...ns) -thunk(...mixed) -thunk(...tuple) // bad all(...mixed) @@ -47,13 +43,9 @@ weird.apply(void 0, mixed); weird.apply(void 0, tuple); prefix.apply(void 0, ["a"].concat(ns)); rest.apply(void 0, ["d"].concat(ns)); -// this covers the arguments case +// extra arguments normal.apply(void 0, ["g"].concat(ns)); -normal.apply(void 0, ["h"].concat(mixed)); -normal.apply(void 0, ["i"].concat(tuple)); thunk.apply(void 0, ns); -thunk.apply(void 0, mixed); -thunk.apply(void 0, tuple); // bad all.apply(void 0, mixed); all.apply(void 0, tuple); diff --git a/tests/baselines/reference/callWithSpread2.symbols b/tests/baselines/reference/callWithSpread2.symbols index 169e8c8d47e..8ce7fb6f903 100644 --- a/tests/baselines/reference/callWithSpread2.symbols +++ b/tests/baselines/reference/callWithSpread2.symbols @@ -64,31 +64,15 @@ rest("d", ...ns) >ns : Symbol(ns, Decl(callWithSpread2.ts, 7, 11)) -// this covers the arguments case +// extra arguments normal("g", ...ns) >normal : Symbol(normal, Decl(callWithSpread2.ts, 3, 83)) >ns : Symbol(ns, Decl(callWithSpread2.ts, 7, 11)) -normal("h", ...mixed) ->normal : Symbol(normal, Decl(callWithSpread2.ts, 3, 83)) ->mixed : Symbol(mixed, Decl(callWithSpread2.ts, 8, 11)) - -normal("i", ...tuple) ->normal : Symbol(normal, Decl(callWithSpread2.ts, 3, 83)) ->tuple : Symbol(tuple, Decl(callWithSpread2.ts, 9, 11)) - thunk(...ns) >thunk : Symbol(thunk, Decl(callWithSpread2.ts, 4, 41)) >ns : Symbol(ns, Decl(callWithSpread2.ts, 7, 11)) -thunk(...mixed) ->thunk : Symbol(thunk, Decl(callWithSpread2.ts, 4, 41)) ->mixed : Symbol(mixed, Decl(callWithSpread2.ts, 8, 11)) - -thunk(...tuple) ->thunk : Symbol(thunk, Decl(callWithSpread2.ts, 4, 41)) ->tuple : Symbol(tuple, Decl(callWithSpread2.ts, 9, 11)) - // bad all(...mixed) >all : Symbol(all, Decl(callWithSpread2.ts, 0, 0)) diff --git a/tests/baselines/reference/callWithSpread2.types b/tests/baselines/reference/callWithSpread2.types index 75f0f18d287..4c87b9bd74d 100644 --- a/tests/baselines/reference/callWithSpread2.types +++ b/tests/baselines/reference/callWithSpread2.types @@ -78,7 +78,7 @@ rest("d", ...ns) >ns : number[] -// this covers the arguments case +// extra arguments normal("g", ...ns) >normal("g", ...ns) : void >normal : (s: string) => void @@ -86,38 +86,12 @@ normal("g", ...ns) >...ns : number >ns : number[] -normal("h", ...mixed) ->normal("h", ...mixed) : void ->normal : (s: string) => void ->"h" : "h" ->...mixed : string | number ->mixed : (string | number)[] - -normal("i", ...tuple) ->normal("i", ...tuple) : void ->normal : (s: string) => void ->"i" : "i" ->...tuple : string | number ->tuple : [number, string] - thunk(...ns) >thunk(...ns) : string >thunk : () => string >...ns : number >ns : number[] -thunk(...mixed) ->thunk(...mixed) : string ->thunk : () => string ->...mixed : string | number ->mixed : (string | number)[] - -thunk(...tuple) ->thunk(...tuple) : string ->thunk : () => string ->...tuple : string | number ->tuple : [number, string] - // bad all(...mixed) >all(...mixed) : void diff --git a/tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts b/tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts index 9151f52f00d..abf445a700e 100644 --- a/tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts +++ b/tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts @@ -18,13 +18,9 @@ prefix("a", ...ns) rest("d", ...ns) -// this covers the arguments case +// extra arguments normal("g", ...ns) -normal("h", ...mixed) -normal("i", ...tuple) thunk(...ns) -thunk(...mixed) -thunk(...tuple) // bad all(...mixed) From 6a08820fe7d4e3352646cbf278fa8b7ac3c5c6c9 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 16 Nov 2017 10:28:17 -0800 Subject: [PATCH 3/3] Improve spread arity mismatch error messages Thanks @weswigham for the improved wording. --- src/compiler/checker.ts | 4 ++-- src/compiler/diagnosticMessages.json | 4 ++-- .../reference/callWithSpread2.errors.txt | 20 +++++++++---------- .../reference/iteratorSpreadInCall.errors.txt | 4 ++-- .../iteratorSpreadInCall10.errors.txt | 4 ++-- .../iteratorSpreadInCall2.errors.txt | 4 ++-- .../iteratorSpreadInCall4.errors.txt | 4 ++-- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e7f954f7295..09572c4c5b9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -16505,9 +16505,9 @@ namespace ts { if (argCount <= max && hasSpreadArgument) { argCount--; } - const error = hasRestParameter && hasSpreadArgument ? Diagnostics.Expected_at_least_0_arguments_but_got_a_minimum_of_1 : + 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_a_minimum_of_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)); } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 964264ff6e8..1c2a80d364e 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1896,11 +1896,11 @@ "category": "Error", "code": 2555 }, - "Expected {0} arguments, but got a minimum of {1}.": { + "Expected {0} arguments, but got {1} or more.": { "category": "Error", "code": 2556 }, - "Expected at least {0} arguments, but got a minimum of {1}.": { + "Expected at least {0} arguments, but got {1} or more.": { "category": "Error", "code": 2557 }, diff --git a/tests/baselines/reference/callWithSpread2.errors.txt b/tests/baselines/reference/callWithSpread2.errors.txt index 7272b1b9f3b..9028d37af09 100644 --- a/tests/baselines/reference/callWithSpread2.errors.txt +++ b/tests/baselines/reference/callWithSpread2.errors.txt @@ -1,5 +1,5 @@ -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(22,1): error TS2556: Expected 1 arguments, but got a minimum of 2. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(23,1): error TS2556: Expected 0 arguments, but got a minimum of 1. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(22,1): error TS2556: Expected 1 arguments, but got 2 or more. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(23,1): error TS2556: Expected 0 arguments, but got 1 or more. tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(26,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(27,5): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. @@ -12,9 +12,9 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(30,11): err Type 'string' is not assignable to type 'number'. tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(31,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'. Type 'string' is not assignable to type 'number'. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(32,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. -tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,1): error TS2556: Expected 1-3 arguments, but got a minimum of 0. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(32,1): error TS2556: Expected 1-3 arguments, but got 0 or more. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,1): error TS2556: Expected 1-3 arguments, but got 0 or more. +tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,1): error TS2556: Expected 1-3 arguments, but got 0 or more. ==== tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts (11 errors) ==== @@ -41,10 +41,10 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,1): erro // extra arguments normal("g", ...ns) ~~~~~~~~~~~~~~~~~~ -!!! error TS2556: Expected 1 arguments, but got a minimum of 2. +!!! error TS2556: Expected 1 arguments, but got 2 or more. thunk(...ns) ~~~~~~~~~~~~ -!!! error TS2556: Expected 0 arguments, but got a minimum of 1. +!!! error TS2556: Expected 0 arguments, but got 1 or more. // bad all(...mixed) @@ -73,11 +73,11 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,1): erro !!! error TS2345: Type 'string' is not assignable to type 'number'. prefix(...ns) // required parameters are required ~~~~~~~~~~~~~ -!!! error TS2556: Expected 1-3 arguments, but got a minimum of 0. +!!! error TS2556: Expected 1-3 arguments, but got 0 or more. prefix(...mixed) ~~~~~~~~~~~~~~~~ -!!! error TS2556: Expected 1-3 arguments, but got a minimum of 0. +!!! error TS2556: Expected 1-3 arguments, but got 0 or more. prefix(...tuple) ~~~~~~~~~~~~~~~~ -!!! error TS2556: Expected 1-3 arguments, but got a minimum of 0. +!!! error TS2556: Expected 1-3 arguments, but got 0 or more. \ No newline at end of file diff --git a/tests/baselines/reference/iteratorSpreadInCall.errors.txt b/tests/baselines/reference/iteratorSpreadInCall.errors.txt index 1831013b6e7..93a4f4df01a 100644 --- a/tests/baselines/reference/iteratorSpreadInCall.errors.txt +++ b/tests/baselines/reference/iteratorSpreadInCall.errors.txt @@ -1,4 +1,4 @@ -tests/cases/conformance/es6/spread/iteratorSpreadInCall.ts(15,1): error TS2556: Expected 1 arguments, but got a minimum of 0. +tests/cases/conformance/es6/spread/iteratorSpreadInCall.ts(15,1): error TS2556: Expected 1 arguments, but got 0 or more. ==== tests/cases/conformance/es6/spread/iteratorSpreadInCall.ts (1 errors) ==== @@ -18,4 +18,4 @@ tests/cases/conformance/es6/spread/iteratorSpreadInCall.ts(15,1): error TS2556: foo(...new SymbolIterator); ~~~~~~~~~~~~~~~~~~~~~~~~~~ -!!! error TS2556: Expected 1 arguments, but got a minimum of 0. \ No newline at end of file +!!! error TS2556: Expected 1 arguments, but got 0 or more. \ No newline at end of file diff --git a/tests/baselines/reference/iteratorSpreadInCall10.errors.txt b/tests/baselines/reference/iteratorSpreadInCall10.errors.txt index 0d10c9f25e5..2012a878549 100644 --- a/tests/baselines/reference/iteratorSpreadInCall10.errors.txt +++ b/tests/baselines/reference/iteratorSpreadInCall10.errors.txt @@ -1,4 +1,4 @@ -tests/cases/conformance/es6/spread/iteratorSpreadInCall10.ts(15,1): error TS2556: Expected 1 arguments, but got a minimum of 0. +tests/cases/conformance/es6/spread/iteratorSpreadInCall10.ts(15,1): error TS2556: Expected 1 arguments, but got 0 or more. ==== tests/cases/conformance/es6/spread/iteratorSpreadInCall10.ts (1 errors) ==== @@ -18,4 +18,4 @@ tests/cases/conformance/es6/spread/iteratorSpreadInCall10.ts(15,1): error TS2556 foo(...new SymbolIterator); ~~~~~~~~~~~~~~~~~~~~~~~~~~ -!!! error TS2556: Expected 1 arguments, but got a minimum of 0. \ No newline at end of file +!!! error TS2556: Expected 1 arguments, but got 0 or more. \ No newline at end of file diff --git a/tests/baselines/reference/iteratorSpreadInCall2.errors.txt b/tests/baselines/reference/iteratorSpreadInCall2.errors.txt index 03cc296b8ab..32559883e67 100644 --- a/tests/baselines/reference/iteratorSpreadInCall2.errors.txt +++ b/tests/baselines/reference/iteratorSpreadInCall2.errors.txt @@ -1,4 +1,4 @@ -tests/cases/conformance/es6/spread/iteratorSpreadInCall2.ts(15,1): error TS2556: Expected 1 arguments, but got a minimum of 0. +tests/cases/conformance/es6/spread/iteratorSpreadInCall2.ts(15,1): error TS2556: Expected 1 arguments, but got 0 or more. ==== tests/cases/conformance/es6/spread/iteratorSpreadInCall2.ts (1 errors) ==== @@ -18,4 +18,4 @@ tests/cases/conformance/es6/spread/iteratorSpreadInCall2.ts(15,1): error TS2556: foo(...new SymbolIterator); ~~~~~~~~~~~~~~~~~~~~~~~~~~ -!!! error TS2556: Expected 1 arguments, but got a minimum of 0. \ No newline at end of file +!!! error TS2556: Expected 1 arguments, but got 0 or more. \ No newline at end of file diff --git a/tests/baselines/reference/iteratorSpreadInCall4.errors.txt b/tests/baselines/reference/iteratorSpreadInCall4.errors.txt index 899ac1d9e90..17a76cce148 100644 --- a/tests/baselines/reference/iteratorSpreadInCall4.errors.txt +++ b/tests/baselines/reference/iteratorSpreadInCall4.errors.txt @@ -1,4 +1,4 @@ -tests/cases/conformance/es6/spread/iteratorSpreadInCall4.ts(15,1): error TS2557: Expected at least 1 arguments, but got a minimum of 0. +tests/cases/conformance/es6/spread/iteratorSpreadInCall4.ts(15,1): error TS2557: Expected at least 1 arguments, but got 0 or more. ==== tests/cases/conformance/es6/spread/iteratorSpreadInCall4.ts (1 errors) ==== @@ -18,4 +18,4 @@ tests/cases/conformance/es6/spread/iteratorSpreadInCall4.ts(15,1): error TS2557: foo(...new SymbolIterator); ~~~~~~~~~~~~~~~~~~~~~~~~~~ -!!! error TS2557: Expected at least 1 arguments, but got a minimum of 0. \ No newline at end of file +!!! error TS2557: Expected at least 1 arguments, but got 0 or more. \ No newline at end of file