Skip to content

Fix union type partial signature matching#5459

Merged
ahejlsberg merged 3 commits into
masterfrom
partialSignatureMatching
Nov 6, 2015
Merged

Fix union type partial signature matching#5459
ahejlsberg merged 3 commits into
masterfrom
partialSignatureMatching

Conversation

@ahejlsberg
Copy link
Copy Markdown
Member

Fixes #5401.

@ahejlsberg
Copy link
Copy Markdown
Member Author

@sandersn This is the best way to fix the issue. We were a bit too permissive in partial signature matches involving rest parameters.

Comment thread src/compiler/checker.ts Outdated
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.

Why does a partial match return Ternary.False when partialMatch === true?

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.

I don't believe that's what actually happens. If both sides of the || are Ternary.False, you'll return Ternary.False

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.

Ah, I didn't see the parentheses surrounding the whole thing. I thought it was !isMatchingSignature(...) ||. Now it makes sense.

@sandersn
Copy link
Copy Markdown
Member

👍 with the comment that I like Daniel's name change idea.

ahejlsberg added a commit that referenced this pull request Nov 6, 2015
Fix union type partial signature matching
@ahejlsberg ahejlsberg merged commit 9b4956f into master Nov 6, 2015
@ahejlsberg ahejlsberg deleted the partialSignatureMatching branch November 6, 2015 00:47
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants