Skip to content

Add elaboration when call fails all overloads but succeeds against the implementation signature - #41001

Merged
Wesley Wigham (weswigham) merged 5 commits into
microsoft:masterfrom
weswigham:overload-impl-elab
Oct 9, 2020
Merged

Wesley Wigham (weswigham) merged 5 commits into
microsoft:masterfrom
weswigham:overload-impl-elab

Conversation

@weswigham

Copy link
Copy Markdown
Member

Fixes #4797

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couple of questions about the technique you used.

I also wish there was some way to estimate how many times people will see this new error. Any ideas there?

Comment thread src/compiler/builder.ts
relatedInformation.length ?
relatedInformation.map(r => convertToDiagnosticRelatedInformation(r, newProgram, toPath)) :
emptyArray :
[] :

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

did we ever figure out what the performance impact of [] vs emptyArray is? Last I heard is that [] is better?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

...eh? In this case, it's just because we mutate relatedInformation in some places, so using a global singleton is bad.

Comment thread src/compiler/checker.ts Outdated
Comment thread src/compiler/checker.ts Outdated
const candidate = getSignatureFromDeclaration(implDecl as FunctionLikeDeclaration);
const isSingleNonGenericCandidate = !candidate.typeParameters;
if (!!chooseOverload([candidate], assignableRelation, isSingleNonGenericCandidate)) {
addRelatedInfo(d, createDiagnosticForNode(implDecl, Diagnostics.The_call_would_have_succeeded_against_this_implementation_but_implementation_signatures_of_overloads_are_not_externally_visible));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does this mutate the related list? (and is that why you clone into related above?)

@weswigham Wesley Wigham (weswigham) Oct 8, 2020 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

addRelatedInfo, as the name implies, has always appended to the existing list.

@weswigham

Copy link
Copy Markdown
Member Author

I also wish there was some way to estimate how many times people will see this new error. Any ideas there?

Pretty often - almost any overload error where the overload being compiled against is actually in your source code (and thus has a body) rather than a library.

@DanielRosenwasser

Copy link
Copy Markdown
Member

Pretty often

Yeah, people on the VS Code team hit this, and it's a beginner mistake. If you don't hear about it much, it's because you bang your head on it for a bit and then figure it out.

!!! error TS2769: Overload 2 of 2, '(bar: number): number', gave the following error.
!!! error TS2769: Argument of type 'boolean' is not assignable to parameter of type 'number'. No newline at end of file
!!! error TS2769: Argument of type 'boolean' is not assignable to parameter of type 'number'.
!!! related TS2793 tests/cases/compiler/functionOverloads2.ts:3:10: The call would have succeeded against this implementation, but implementation signatures of overloads are not externally visible. No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😍

Comment thread src/compiler/checker.ts Outdated
Comment thread src/compiler/checker.ts Outdated
Comment thread src/compiler/checker.ts Outdated
Comment thread src/compiler/checker.ts Outdated
@weswigham
Wesley Wigham (weswigham) merged commit 02288a4 into microsoft:master Oct 9, 2020
@weswigham
Wesley Wigham (weswigham) deleted the overload-impl-elab branch October 9, 2020 01:13
@sandersn

Copy link
Copy Markdown
Member

Causes crashes in Definitely Typed -- see types/agent-base for example. I'll file a bug.

@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve error when function overload set incorrectly authored

4 participants