Skip to content

Fix get candidate for overload failure checking #36744

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

Merged
merged 3 commits into from
Feb 12, 2020
Merged

Fix get candidate for overload failure checking #36744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cdfd3f
getCandidateForOverloadFailure:call resolveUntypedCall
sandersn Feb 12, 2020
582f5e0
Update baselines and remove new test
sandersn Feb 12, 2020
83ef5c3
Defer resolveUntypedCall on resolution failure to give priority to pa…
weswigham Feb 12, 2020
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
13 changes: 12 additions & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22600,6 +22600,7 @@ namespace ts {

function checkJsxSelfClosingElementDeferred(node: JsxSelfClosingElement) {
checkJsxOpeningLikeElementOrOpeningFragment(node);
resolveUntypedCall(node); // ensure type arguments and parameters are typechecked, even if there is an arity error
}

function checkJsxSelfClosingElement(node: JsxSelfClosingElement, _checkMode: CheckMode | undefined): Type {
Expand Down Expand Up @@ -24987,14 +24988,14 @@ namespace ts {
}

// No signature was applicable. We have already reported the errors for the invalid signature.
// If this is a type resolution session, e.g. Language Service, try to get better information than anySignature.
function getCandidateForOverloadFailure(
node: CallLikeExpression,
candidates: Signature[],
args: readonly Expression[],
hasCandidatesOutArray: boolean,
): Signature {
Debug.assert(candidates.length > 0); // Else should not have called this.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sandersn do you want to consider deferring this to the end of somehow (since it just needs to happen once for errors to be reported and usage to be painted, rather than on every attempted resolution)? Like how we have checkNodeDeferred? This does fix parameter types, and looking at the baselines, the parameter types we get from deferring this may be nicer that what's here.

Like this change is what I'm proposing - it looks like it minimizes the baseline diff a lot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. I'll give it a try.

checkNodeDeferred(node);
// Normally we will combine overloads. Skip this if they have type parameters since that's hard to combine.
// Don't do this if there is a `candidatesOutArray`,
// because then we want the chosen best candidate to be one of the overloads, not a combination.
Expand Down Expand Up @@ -33913,6 +33914,16 @@ namespace ts {
currentNode = node;
instantiationCount = 0;
switch (node.kind) {
case SyntaxKind.CallExpression:
case SyntaxKind.NewExpression:
case SyntaxKind.TaggedTemplateExpression:
case SyntaxKind.Decorator:
case SyntaxKind.JsxOpeningElement:
// These node kinds are deferred checked when overload resolution fails
// To save on work, we ensure the arguments are checked just once, in
// a deferred way
resolveUntypedCall(node as CallLikeExpression);
break;
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
case SyntaxKind.MethodDeclaration:
Expand Down
8 changes: 7 additions & 1 deletion tests/baselines/reference/baseCheck.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
tests/cases/compiler/baseCheck.ts(9,18): error TS2552: Cannot find name 'loc'. Did you mean 'ELoc'?
tests/cases/compiler/baseCheck.ts(17,53): error TS2554: Expected 2 arguments, but got 1.
tests/cases/compiler/baseCheck.ts(17,59): error TS17009: 'super' must be called before accessing 'this' in the constructor of a derived class.
tests/cases/compiler/baseCheck.ts(18,62): error TS17009: 'super' must be called before accessing 'this' in the constructor of a derived class.
tests/cases/compiler/baseCheck.ts(19,59): error TS2345: Argument of type '"hello"' is not assignable to parameter of type 'number'.
tests/cases/compiler/baseCheck.ts(19,68): error TS17009: 'super' must be called before accessing 'this' in the constructor of a derived class.
tests/cases/compiler/baseCheck.ts(22,9): error TS2304: Cannot find name 'x'.
tests/cases/compiler/baseCheck.ts(23,7): error TS2304: Cannot find name 'x'.
tests/cases/compiler/baseCheck.ts(26,9): error TS2304: Cannot find name 'x'.


==== tests/cases/compiler/baseCheck.ts (7 errors) ====
==== tests/cases/compiler/baseCheck.ts (9 errors) ====
class C { constructor(x: number, y: number) { } }
class ELoc extends C {
constructor(x: number) {
Expand All @@ -31,12 +33,16 @@ tests/cases/compiler/baseCheck.ts(26,9): error TS2304: Cannot find name 'x'.
~~~~~~~~~~~~~
!!! error TS2554: Expected 2 arguments, but got 1.
!!! related TS6210 tests/cases/compiler/baseCheck.ts:1:34: An argument for 'y' was not provided.
~~~~
!!! error TS17009: 'super' must be called before accessing 'this' in the constructor of a derived class.
class E extends C { constructor(public z: number) { super(0, this.z) } }
~~~~
!!! error TS17009: 'super' must be called before accessing 'this' in the constructor of a derived class.
class F extends C { constructor(public z: number) { super("hello", this.z) } } // first param type
~~~~~~~
!!! error TS2345: Argument of type '"hello"' is not assignable to parameter of type 'number'.
~~~~
!!! error TS17009: 'super' must be called before accessing 'this' in the constructor of a derived class.

function f() {
if (x<10) {
Expand Down
14 changes: 13 additions & 1 deletion tests/baselines/reference/grammarAmbiguities1.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
tests/cases/compiler/grammarAmbiguities1.ts(8,3): error TS2365: Operator '<' cannot be applied to types '<T, U>(x: any) => any' and 'typeof A'.
tests/cases/compiler/grammarAmbiguities1.ts(8,10): error TS2365: Operator '>' cannot be applied to types 'typeof B' and 'number'.
tests/cases/compiler/grammarAmbiguities1.ts(8,10): error TS2554: Expected 1 arguments, but got 2.
tests/cases/compiler/grammarAmbiguities1.ts(9,3): error TS2365: Operator '<' cannot be applied to types '<T, U>(x: any) => any' and 'typeof A'.
tests/cases/compiler/grammarAmbiguities1.ts(9,10): error TS2365: Operator '>' cannot be applied to types 'typeof B' and 'number'.
tests/cases/compiler/grammarAmbiguities1.ts(9,10): error TS2554: Expected 1 arguments, but got 2.


==== tests/cases/compiler/grammarAmbiguities1.ts (2 errors) ====
==== tests/cases/compiler/grammarAmbiguities1.ts (6 errors) ====
class A { foo() { } }
class B { bar() { }}
function f(x) { return x; }
Expand All @@ -11,9 +15,17 @@ tests/cases/compiler/grammarAmbiguities1.ts(9,10): error TS2554: Expected 1 argu

f(g<A, B>(7));
f(g < A, B > 7);
~~~~~
!!! error TS2365: Operator '<' cannot be applied to types '<T, U>(x: any) => any' and 'typeof A'.
~~~~~
!!! error TS2365: Operator '>' cannot be applied to types 'typeof B' and 'number'.
~~~~~
!!! error TS2554: Expected 1 arguments, but got 2.
f(g < A, B > +(7));
~~~~~
!!! error TS2365: Operator '<' cannot be applied to types '<T, U>(x: any) => any' and 'typeof A'.
~~~~~~~~
!!! error TS2365: Operator '>' cannot be applied to types 'typeof B' and 'number'.
~~~~~~~~
!!! error TS2554: Expected 1 arguments, but got 2.

5 changes: 4 additions & 1 deletion tests/baselines/reference/noImplicitAnyLoopCrash.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
tests/cases/compiler/noImplicitAnyLoopCrash.ts(4,16): error TS2556: Expected 0 arguments, but got 1 or more.
tests/cases/compiler/noImplicitAnyLoopCrash.ts(4,19): error TS2461: Type 'number' is not an array type.


==== tests/cases/compiler/noImplicitAnyLoopCrash.ts (1 errors) ====
==== tests/cases/compiler/noImplicitAnyLoopCrash.ts (2 errors) ====
let foo = () => {};
let bar;
while (1) {
bar = ~foo(...bar);
~~~~~~
!!! error TS2556: Expected 0 arguments, but got 1 or more.
~~~
!!! error TS2461: Type 'number' is not an array type.
}

5 changes: 4 additions & 1 deletion tests/baselines/reference/undeclaredModuleError.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
tests/cases/compiler/undeclaredModuleError.ts(1,21): error TS2307: Cannot find module 'fs'.
tests/cases/compiler/undeclaredModuleError.ts(8,29): error TS2345: Argument of type '() => void' is not assignable to parameter of type '(stat: any, name: string) => boolean'.
Type 'void' is not assignable to type 'boolean'.
tests/cases/compiler/undeclaredModuleError.ts(11,41): error TS2304: Cannot find name 'IDoNotExist'.


==== tests/cases/compiler/undeclaredModuleError.ts (2 errors) ====
==== tests/cases/compiler/undeclaredModuleError.ts (3 errors) ====
import fs = require('fs');
~~~~
!!! error TS2307: Cannot find module 'fs'.
Expand All @@ -20,6 +21,8 @@ tests/cases/compiler/undeclaredModuleError.ts(8,29): error TS2345: Argument of t
} , (error: Error, files: {}[]) => {
files.forEach((file) => {
var fullPath = join(IDoNotExist);
~~~~~~~~~~~
!!! error TS2304: Cannot find name 'IDoNotExist'.
} );
} );
} );
Expand Down