Skip to content

Merge inherits interface members#5359

Merged
sandersn merged 8 commits into
masterfrom
merge-inherits-interface-members
Oct 23, 2015
Merged

Merge inherits interface members#5359
sandersn merged 8 commits into
masterfrom
merge-inherits-interface-members

Conversation

@sandersn
Copy link
Copy Markdown
Member

Fixes #5347

This bug has been around since July but we didn't notice it until recently. getBaseTypes assumed that if a class was present in the merge, only the class base types needed to be used -- it skipped the interface base types entirely. This became false when [ambient] classes and interfaces could be merged.

Previously it assumed that if a class was present in the merge, only the
class base types needed to be used. This became false when classes and
interfaces could be merged.
Both interfaces and classes have base classes/interfaces; all members are
available on an instance of the merged child.
A couple of tests were previously updated to give an incorrect error
message.
@sandersn
Copy link
Copy Markdown
Member Author

I forgot to update the baselines in the last commit. I'll update them shortly to fix the build.

I found that merging a class that has no base with an interface that has a
base class causes a crash because `getDefaultConstructSignatures` assumes
that any base must be a class base. Which was true in the previously buggy
state.
Covers the case when the merged interface extends an interface, but the
merged class does not extend a class, then trying to extend that class.
Comment thread src/compiler/checker.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i would move this to the bottom where it is used. this way we are not creating an empty array literal for every class. so that classes with no base types can share the sentinel empty array value.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Oct 23, 2015

👍

sandersn added a commit that referenced this pull request Oct 23, 2015
@sandersn sandersn merged commit d26a4c8 into master Oct 23, 2015
@sandersn sandersn deleted the merge-inherits-interface-members branch October 23, 2015 21:51
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Oct 23, 2015

We will need to cherrypick this into release-1.7

@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.

3 participants