Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify arity errors, rewording spread errors #43855

Merged
merged 9 commits into from
Apr 29, 2021
Merged

Simplify arity errors, rewording spread errors #43855

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b3d521a
Scribbles + tests
sandersn Apr 13, 2021
170cade
Merge branch 'master' into reform-arity-errors
sandersn Apr 26, 2021
90e59ab
Basically working
sandersn Apr 27, 2021
7644161
trim out too-real test case
sandersn Apr 27, 2021
42158f6
Finish cleanup
sandersn Apr 27, 2021
33d00c9
Simplify and reword spread errors
sandersn Apr 27, 2021
30e131a
handle spreads first
sandersn Apr 27, 2021
8a265fb
update baselines
sandersn Apr 27, 2021
24583bf
Address PR comments
sandersn Apr 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 49 additions & 70 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28559,88 +28559,67 @@ namespace ts {
}

function getArgumentArityError(node: CallLikeExpression, signatures: readonly Signature[], args: readonly Expression[]) {
let min = Number.POSITIVE_INFINITY;
let max = Number.NEGATIVE_INFINITY;
let belowArgCount = Number.NEGATIVE_INFINITY;
let aboveArgCount = Number.POSITIVE_INFINITY;
const spreadIndex = getSpreadArgumentIndex(args);
if (spreadIndex > -1) {
return createDiagnosticForNode(args[spreadIndex], Diagnostics.A_spread_must_either_have_a_tuple_type_or_be_passed_to_a_rest_parameter);
}
let min = Number.POSITIVE_INFINITY; // smallest parameter count
let max = Number.NEGATIVE_INFINITY; // larger parameter count
sandersn marked this conversation as resolved.
Show resolved Hide resolved
let maxBelow = Number.NEGATIVE_INFINITY; // largest parameter count that is smaller than the number of arguments
let minAbove = Number.POSITIVE_INFINITY; // smallest parameter count that is larger than the number of arguments

let argCount = args.length;
let closestSignature: Signature | undefined;
for (const sig of signatures) {
const minCount = getMinArgumentCount(sig);
const maxCount = getParameterCount(sig);
if (minCount < argCount && minCount > belowArgCount) belowArgCount = minCount;
if (argCount < maxCount && maxCount < aboveArgCount) aboveArgCount = maxCount;
if (minCount < min) {
min = minCount;
const minParameter = getMinArgumentCount(sig);
const maxParameter = getParameterCount(sig);
// shortest/longest parameter counts
if (minParameter < min) {
min = minParameter;
closestSignature = sig;
}
max = Math.max(max, maxCount);
}

if (min < argCount && argCount < max) {
return getDiagnosticForCallNode(node, Diagnostics.No_overload_expects_0_arguments_but_overloads_do_exist_that_expect_either_1_or_2_arguments, argCount, belowArgCount, aboveArgCount);
max = Math.max(max, maxParameter);
// shortest parameter count *longer than the call*/longest parameter count *shorter than the call*
if (minParameter < args.length && minParameter > maxBelow) maxBelow = minParameter;
if (args.length < maxParameter && maxParameter < minAbove) minAbove = maxParameter;
}

const hasRestParameter = some(signatures, hasEffectiveRestParameter);
const paramRange = hasRestParameter ? min :
min < max ? min + "-" + max :
min;
const hasSpreadArgument = getSpreadArgumentIndex(args) > -1;
if (argCount <= max && hasSpreadArgument) {
argCount--;
}

let spanArray: NodeArray<Node>;
let related: DiagnosticWithLocation | undefined;

const error = hasRestParameter || hasSpreadArgument ?
hasRestParameter && hasSpreadArgument ?
Diagnostics.Expected_at_least_0_arguments_but_got_1_or_more :
hasRestParameter ?
Diagnostics.Expected_at_least_0_arguments_but_got_1 :
Diagnostics.Expected_0_arguments_but_got_1_or_more :
paramRange === 1 && argCount === 0 && isPromiseResolveArityError(node) ?
Diagnostics.Expected_0_arguments_but_got_1_Did_you_forget_to_include_void_in_your_type_argument_to_Promise :
Diagnostics.Expected_0_arguments_but_got_1;

if (closestSignature && getMinArgumentCount(closestSignature) > argCount && closestSignature.declaration) {
const paramDecl = closestSignature.declaration.parameters[closestSignature.thisParameter ? argCount + 1 : argCount];
if (paramDecl) {
related = createDiagnosticForNode(
paramDecl,
isBindingPattern(paramDecl.name) ? Diagnostics.An_argument_matching_this_binding_pattern_was_not_provided :
isRestParameter(paramDecl) ? Diagnostics.Arguments_for_the_rest_parameter_0_were_not_provided : Diagnostics.An_argument_for_0_was_not_provided,
!paramDecl.name ? argCount : !isBindingPattern(paramDecl.name) ? idText(getFirstIdentifier(paramDecl.name)) : undefined
const parameterRange = hasRestParameter ? min
: min < max ? min + "-" + max
: min;
const error = hasRestParameter ? Diagnostics.Expected_at_least_0_arguments_but_got_1
: parameterRange === 1 && args.length === 0 && isPromiseResolveArityError(node) ? Diagnostics.Expected_0_arguments_but_got_1_Did_you_forget_to_include_void_in_your_type_argument_to_Promise
: Diagnostics.Expected_0_arguments_but_got_1;
if (min < args.length && args.length < max) {
// between min and max, but with no matching overload
return getDiagnosticForCallNode(node, Diagnostics.No_overload_expects_0_arguments_but_overloads_do_exist_that_expect_either_1_or_2_arguments, args.length, maxBelow, minAbove);
}
else if (args.length < min) {
// too short: put the error span on the call expression, not any of the args
const diagnostic = getDiagnosticForCallNode(node, error, parameterRange, args.length);
const parameter = closestSignature?.declaration?.parameters[closestSignature.thisParameter ? args.length + 1 : args.length];
if (parameter) {
const parameterError = createDiagnosticForNode(
parameter,
isBindingPattern(parameter.name) ? Diagnostics.An_argument_matching_this_binding_pattern_was_not_provided
: isRestParameter(parameter) ? Diagnostics.Arguments_for_the_rest_parameter_0_were_not_provided
: Diagnostics.An_argument_for_0_was_not_provided,
!parameter.name ? args.length : !isBindingPattern(parameter.name) ? idText(getFirstIdentifier(parameter.name)) : undefined
);
return addRelatedInfo(diagnostic, parameterError);
}
}

if (!hasSpreadArgument && argCount < min) {
const diagnostic = getDiagnosticForCallNode(node, error, paramRange, argCount);
return related ? addRelatedInfo(diagnostic, related) : diagnostic;
}

if (hasRestParameter || hasSpreadArgument) {
spanArray = factory.createNodeArray(args);
if (hasSpreadArgument && argCount) {
const nextArg = elementAt(args, getSpreadArgumentIndex(args) + 1) || undefined;
spanArray = factory.createNodeArray(args.slice(max > argCount && nextArg ? args.indexOf(nextArg) : Math.min(max, args.length - 1)));
}
return diagnostic;
}
else {
spanArray = factory.createNodeArray(args.slice(max));
}

const pos = first(spanArray).pos;
let end = last(spanArray).end;
if (end === pos) {
end++;
// too long; error goes on the excess parameters
const errorSpan = factory.createNodeArray(args.slice(max));
const pos = first(errorSpan).pos;
let end = last(errorSpan).end;
if (end === pos) {
end++;
}
setTextRangePosEnd(errorSpan, pos, end);
return createDiagnosticForNodeArray(getSourceFileOfNode(node), errorSpan, error, parameterRange, args.length);
}
setTextRangePosEnd(spanArray, pos, end);
const diagnostic = createDiagnosticForNodeArray(
getSourceFileOfNode(node), spanArray, error, paramRange, argCount);
return related ? addRelatedInfo(diagnostic, related) : diagnostic;
}

function getTypeArgumentArityError(node: Node, signatures: readonly Signature[], typeArguments: NodeArray<TypeNode>) {
Expand Down
8 changes: 2 additions & 6 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@
"'extends' clause already seen.": {
"category": "Error",
"code": 1172
},
},
"'extends' clause must precede 'implements' clause.": {
"category": "Error",
"code": 1173
Expand Down Expand Up @@ -2386,14 +2386,10 @@
"category": "Error",
"code": 2555
},
"Expected {0} arguments, but got {1} or more.": {
"A spread must either have a tuple type or be passed to a rest parameter.": {
sandersn marked this conversation as resolved.
Show resolved Hide resolved
"category": "Error",
"code": 2556
},
"Expected at least {0} arguments, but got {1} or more.": {
"category": "Error",
"code": 2557
},
"Expected {0} type arguments, but got {1}.": {
"category": "Error",
"code": 2558
Expand Down
5 changes: 2 additions & 3 deletions tests/baselines/reference/callOverload.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
tests/cases/conformance/expressions/functionCalls/callOverload.ts(7,7): error TS2554: Expected 1 arguments, but got 4.
tests/cases/conformance/expressions/functionCalls/callOverload.ts(8,15): error TS2554: Expected 2 arguments, but got 4.
tests/cases/conformance/expressions/functionCalls/callOverload.ts(10,1): error TS2555: Expected at least 1 arguments, but got 0.
tests/cases/conformance/expressions/functionCalls/callOverload.ts(11,10): error TS2557: Expected at least 1 arguments, but got 0 or more.
tests/cases/conformance/expressions/functionCalls/callOverload.ts(11,10): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.


==== tests/cases/conformance/expressions/functionCalls/callOverload.ts (4 errors) ====
Expand All @@ -24,5 +24,4 @@ tests/cases/conformance/expressions/functionCalls/callOverload.ts(11,10): error
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callOverload.ts:3:27: An argument for 'a' was not provided.
withRest(...n);
~~~~
!!! error TS2557: Expected at least 1 arguments, but got 0 or more.
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callOverload.ts:3:27: An argument for 'a' was not provided.
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
23 changes: 10 additions & 13 deletions tests/baselines/reference/callWithSpread2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(23,13): error TS2556: Expected 1 arguments, but got 2 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(24,7): error TS2556: Expected 0 arguments, but got 1 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(23,13): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(24,7): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
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(28,5): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
Expand All @@ -9,10 +9,10 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(30,13): err
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,11): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,8): error TS2556: Expected 1-3 arguments, but got 0 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,8): error TS2556: Expected 1-3 arguments, but got 0 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,8): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,8): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(35,8): error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(36,14): error TS2556: Expected 2-4 arguments, but got 1 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(36,14): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.


==== tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts (12 errors) ====
Expand Down Expand Up @@ -40,10 +40,10 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(36,14): err
// extra arguments
normal("g", ...ns)
~~~~~
!!! error TS2556: Expected 1 arguments, but got 2 or more.
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
thunk(...ns)
~~~~~
!!! error TS2556: Expected 0 arguments, but got 1 or more.
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.

// bad
all(...mixed)
Expand All @@ -69,17 +69,14 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(36,14): err
!!! error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
prefix(...ns) // required parameters are required
~~~~~
!!! error TS2556: Expected 1-3 arguments, but got 0 or more.
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts:3:25: An argument for 's' was not provided.
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
prefix(...mixed)
~~~~~~~~
!!! error TS2556: Expected 1-3 arguments, but got 0 or more.
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts:3:25: An argument for 's' was not provided.
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
prefix(...tuple)
~~~~~~~~
!!! error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.
prefix2("g", ...ns);
~~~~~
!!! error TS2556: Expected 2-4 arguments, but got 1 or more.
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts:7:37: An argument for 'n' was not provided.
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.

24 changes: 11 additions & 13 deletions tests/baselines/reference/callWithSpread3.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(16,15): err
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(17,15): error TS2554: Expected 2 arguments, but got 6.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(18,12): error TS2554: Expected 2 arguments, but got 3.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(19,5): error TS2554: Expected 2 arguments, but got 3.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(20,6): error TS2557: Expected at least 2 arguments, but got 0 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(20,6): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(21,6): error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(22,13): error TS2557: Expected at least 2 arguments, but got 1 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(23,13): error TS2557: Expected at least 2 arguments, but got 2 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(22,6): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(23,6): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(25,7): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(31,13): error TS2557: Expected at least 2 arguments, but got 4 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(31,6): error TS2556: A spread must either have a tuple type or be passed to a rest parameter.


==== tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts (12 errors) ====
Expand Down Expand Up @@ -46,18 +46,16 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(31,13): err
!!! error TS2554: Expected 2 arguments, but got 3.
fs2_(...s_); // error on ...s_
~~~~~
!!! error TS2557: Expected at least 2 arguments, but got 0 or more.
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts:9:23: An argument for 'a' was not provided.
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
fs2_(...s2n_); // error on ...s2n_
~~~~~~~
!!! error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.
fs2_(...s_, ...s_); // error FIXME: bad error message
sandersn marked this conversation as resolved.
Show resolved Hide resolved
~~~~~
!!! error TS2557: Expected at least 2 arguments, but got 1 or more.
!!! related TS6210 tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts:9:34: An argument for 'b' was not provided.
~~~~~
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
fs2_(...s_, ...s_, ...s_); // error FIXME: worse error message
~~~~~~~~~~~~
!!! error TS2557: Expected at least 2 arguments, but got 2 or more.
~~~~~
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
// fs2n_(...s2, ...s_); // FIXME: should be a type error
fs2n_(...s2_); // error on ...s2_
~~~~~~
Expand All @@ -68,8 +66,8 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread3.ts(31,13): err
fs2_(...s2_, ...s_);
fs2_(...s2_, ...s2_);
fs2_(...s_, ...s2_);
~~~~~~
!!! error TS2557: Expected at least 2 arguments, but got 4 or more.
~~~~~
!!! error TS2556: A spread must either have a tuple type or be passed to a rest parameter.
fs2n_(...s2n_);
fs2n_(...s2);
// fs2n_(...s2, ...n_); // FIXME: should compile
Expand Down
Loading