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.

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 Declarations for Classes and Interfaces #3333

Merged
merged 37 commits into from
Jul 2, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
813d227
classInterface: changed excludes flags
May 27, 2015
18abf47
Revert "classInterface: changed excludes flags"
Jun 1, 2015
05500f4
Added merged declarations for ambient class/interfaces
Jun 1, 2015
6b8d033
Merge branch 'master' into mergedDeclarationClassInterface
Jun 1, 2015
6c98c67
added conformance tests
Jun 1, 2015
b864df2
New Baselines for class-interface merging
Jun 1, 2015
ed46bc3
Fixed intendation typo
Jun 1, 2015
d25b910
fixed indentation
Jun 1, 2015
1f74b13
fixed style, added comment
Jun 2, 2015
2d77cbd
cleaner hasNonAmbientClass
Jun 2, 2015
edc4611
removed comma
Jun 2, 2015
a50ab3e
Remove checking in declareSymbol
Jun 2, 2015
90e1955
merge compatiblity now performed in checker.ts
Jun 2, 2015
c629c3f
deleted redundant tests
Jun 2, 2015
e4bc29e
Updated tests
Jun 2, 2015
b293da4
updated baselines
Jun 2, 2015
936aea8
fixed merge conflict
Jun 2, 2015
0917582
removed extra newlines
Jun 2, 2015
fa06f3e
fixed merge conflict.
Jun 2, 2015
fa9b6fc
fixed loops, merged baseline
Jun 3, 2015
9e1ab92
merged with master
Jun 3, 2015
f3278e2
fixed a grammatical issue
Jun 3, 2015
2b899f1
simplified check
Jun 3, 2015
29c9286
Updated error message
Jun 3, 2015
d000e01
updated baselines to reflect new error message
Jun 3, 2015
143890b
New test
Jun 3, 2015
323ce24
new baselines got mergeClassInterfaceAndModule
Jun 3, 2015
365ea3d
Check for ambient context instead of ambient flag
Jun 3, 2015
015c2c1
Merge branch 'master' into mergedDeclarationClassInterface
Jun 3, 2015
5ef426c
new baselines
Jun 3, 2015
3a3479d
New Test and Baseline
Jun 4, 2015
19b0c51
merged master
Jun 17, 2015
91e3a5c
updated baselines
Jun 18, 2015
851c7e4
fixed comment, spacing
Jun 18, 2015
4878cce
merged with master
Jul 2, 2015
c06e5eb
Update test
Jul 2, 2015
3af3177
update baselines
Jul 2, 2015
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
18 changes: 14 additions & 4 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ namespace ts {
return node.name ? declarationNameToString(node.name) : getDeclarationName(node);
}

/**
* Declares a Symbol for the node and adds it to symbols. Reports errors for conflicting identifier names.
* @param symbolTable - The symbol table which node will be added to.
* @param parent - node's parent declaration.
* @param node - The declaration to be added to the symbol table
* @param includes - The SymbolFlags that node has in addition to its declaration type (eg: export, ambient, etc.)
* @param excludes - The flags which node cannot be declared alongside in a symbol table. Used to report forbidden declarations.
*/
function declareSymbol(symbolTable: SymbolTable, parent: Symbol, node: Declaration, includes: SymbolFlags, excludes: SymbolFlags): Symbol {
Debug.assert(!hasDynamicName(node));

Expand All @@ -181,10 +189,11 @@ namespace ts {

let symbol: Symbol;
if (name !== undefined) {

// Check and see if the symbol table already has a symbol with this name. If not,
// create a new symbol with this name and add it to the table. Note that we don't
// give the new symbol any flags *yet*. This ensures that it will not conflict
// witht he 'excludes' flags we pass in.
// with the 'excludes' flags we pass in.
//
// If we do get an existing symbol, see if it conflicts with the new symbol we're
// creating. For example, a 'var' symbol and a 'class' symbol will conflict within
Expand All @@ -202,10 +211,10 @@ namespace ts {
symbol = hasProperty(symbolTable, name)
? symbolTable[name]
: (symbolTable[name] = createSymbol(SymbolFlags.None, name));

if (name && (includes & SymbolFlags.Classifiable)) {
classifiableNames[name] = name;
}
classifiableNames[name] = name;
}

if (symbol.flags & excludes) {
if (node.name) {
Expand Down Expand Up @@ -314,6 +323,7 @@ namespace ts {

addToContainerChain(container);
}

else if (containerFlags & ContainerFlags.IsBlockScopedContainer) {
blockScopeContainer = node;
blockScopeContainer.locals = undefined;
Expand Down
17 changes: 17 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12034,6 +12034,12 @@ namespace ts {
grammarErrorOnFirstToken(node, Diagnostics.A_class_declaration_without_the_default_modifier_must_have_a_name);
}
checkClassLikeDeclaration(node);

// Interfaces cannot be merged with non-ambient classes.
if (getSymbolOfNode(node).flags & SymbolFlags.Interface && !isInAmbientContext(node)) {
error(node, Diagnostics.Only_an_ambient_class_can_be_merged_with_an_interface)
}

forEach(node.members, checkSourceElement);
}

Expand Down Expand Up @@ -12306,13 +12312,24 @@ namespace ts {
checkIndexConstraints(type);
}
}

// Interfaces cannot merge with non-ambient classes.
if (symbol && symbol.declarations) {
for (let declaration of symbol.declarations) {
if (declaration.kind === SyntaxKind.ClassDeclaration && !isInAmbientContext(declaration)) {
error(node, Diagnostics.Only_an_ambient_class_can_be_merged_with_an_interface);
break;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Same here, you can just check symbol.flags & SymbolFlags.Class.

Copy link
Contributor

Choose a reason for hiding this comment

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

Again, need to check if the class is in an ambient context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't work for

interface I {}
declare class I {}
module I {}

Since the module occupies the value space, the blanket check looking at the symbol will trigger an error, but the merge should be able to go through.

}
forEach(getInterfaceBaseTypeNodes(node), heritageElement => {
if (!isSupportedExpressionWithTypeArguments(heritageElement)) {
error(heritageElement.expression, Diagnostics.An_interface_can_only_extend_an_identifier_Slashqualified_name_with_optional_type_arguments);
}
checkTypeReferenceNode(heritageElement);
});

forEach(node.members, checkSourceElement);

if (produceDiagnostics) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ namespace ts {
Non_abstract_class_0_does_not_implement_inherited_abstract_member_1_from_class_2: { code: 2515, category: DiagnosticCategory.Error, key: "Non-abstract class '{0}' does not implement inherited abstract member '{1}' from class '{2}'." },
All_declarations_of_an_abstract_method_must_be_consecutive: { code: 2516, category: DiagnosticCategory.Error, key: "All declarations of an abstract method must be consecutive." },
Constructor_objects_of_abstract_type_cannot_be_assigned_to_constructor_objects_of_non_abstract_type: { code: 2517, category: DiagnosticCategory.Error, key: "Constructor objects of abstract type cannot be assigned to constructor objects of non-abstract type" },
Only_an_ambient_class_can_be_merged_with_an_interface: { code: 2518, category: DiagnosticCategory.Error, key: "Only an ambient class can be merged with an interface." },
Duplicate_identifier_0_Compiler_uses_declaration_1_to_support_async_functions: { code: 2520, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'. Compiler uses declaration '{1}' to support async functions." },
Expression_resolves_to_variable_declaration_0_that_compiler_uses_to_support_async_functions: { code: 2521, category: DiagnosticCategory.Error, key: "Expression resolves to variable declaration '{0}' that compiler uses to support async functions." },
The_arguments_object_cannot_be_referenced_in_an_async_arrow_function_Consider_using_a_standard_async_function_expression: { code: 2522, category: DiagnosticCategory.Error, key: "The 'arguments' object cannot be referenced in an async arrow function. Consider using a standard async function expression." },
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1621,6 +1621,10 @@
"category": "Error",
"code":2517
},
"Only an ambient class can be merged with an interface.": {
"category": "Error",
"code": 2518
},
"Duplicate identifier '{0}'. Compiler uses declaration '{1}' to support async functions.": {
"category": "Error",
"code": 2520
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1597,8 +1597,8 @@ namespace ts {
PropertyExcludes = Value,
EnumMemberExcludes = Value,
FunctionExcludes = Value & ~(Function | ValueModule),
ClassExcludes = (Value | Type) & ~ValueModule,
InterfaceExcludes = Type & ~Interface,
ClassExcludes = (Value | Type) & ~(ValueModule | Interface), // class-interface mergability done in checker.ts
InterfaceExcludes = Type & ~(Interface | Class),
RegularEnumExcludes = (Value | Type) & ~(RegularEnum | ValueModule), // regular enums merge only with regular enums and modules
ConstEnumExcludes = (Value | Type) & ~ConstEnum, // const enums merge only with const enums
ValueModuleExcludes = Value & ~(Function | Class | RegularEnum | ValueModule),
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/augmentedTypesClass2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/compiler/augmentedTypesClass2.ts(4,7): error TS2300: Duplicate identifier 'c11'.
tests/cases/compiler/augmentedTypesClass2.ts(10,11): error TS2300: Duplicate identifier 'c11'.
tests/cases/compiler/augmentedTypesClass2.ts(4,7): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/compiler/augmentedTypesClass2.ts(10,11): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/compiler/augmentedTypesClass2.ts(16,7): error TS2300: Duplicate identifier 'c33'.
tests/cases/compiler/augmentedTypesClass2.ts(21,6): error TS2300: Duplicate identifier 'c33'.

Expand All @@ -10,15 +10,15 @@ tests/cases/compiler/augmentedTypesClass2.ts(21,6): error TS2300: Duplicate iden
// class then interface
class c11 { // error
~~~
!!! error TS2300: Duplicate identifier 'c11'.
!!! error TS2518: Only an ambient class can be merged with an interface.
foo() {
return 1;
}
}

interface c11 { // error
~~~
!!! error TS2300: Duplicate identifier 'c11'.
!!! error TS2518: Only an ambient class can be merged with an interface.
bar(): void;
}

Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/augmentedTypesInterface.errors.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/compiler/augmentedTypesInterface.ts(12,11): error TS2300: Duplicate identifier 'i2'.
tests/cases/compiler/augmentedTypesInterface.ts(16,7): error TS2300: Duplicate identifier 'i2'.
tests/cases/compiler/augmentedTypesInterface.ts(12,11): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/compiler/augmentedTypesInterface.ts(16,7): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/compiler/augmentedTypesInterface.ts(23,11): error TS2300: Duplicate identifier 'i3'.
tests/cases/compiler/augmentedTypesInterface.ts(26,6): error TS2300: Duplicate identifier 'i3'.

Expand All @@ -18,13 +18,13 @@ tests/cases/compiler/augmentedTypesInterface.ts(26,6): error TS2300: Duplicate i
// interface then class
interface i2 { // error
~~
!!! error TS2300: Duplicate identifier 'i2'.
!!! error TS2518: Only an ambient class can be merged with an interface.
foo(): void;
}

class i2 { // error
~~
!!! error TS2300: Duplicate identifier 'i2'.
!!! error TS2518: Only an ambient class can be merged with an interface.
bar() {
return 1;
}
Expand Down
11 changes: 0 additions & 11 deletions tests/baselines/reference/class1.errors.txt

This file was deleted.

10 changes: 0 additions & 10 deletions tests/baselines/reference/class1.js

This file was deleted.

71 changes: 54 additions & 17 deletions tests/baselines/reference/classAbstractMergedDeclaration.errors.txt
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(7,16): error TS2300: Duplicate identifier 'CI'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(8,11): error TS2300: Duplicate identifier 'CI'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(10,11): error TS2300: Duplicate identifier 'IC'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(11,16): error TS2300: Duplicate identifier 'IC'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(7,16): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(8,11): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(10,11): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(11,16): error TS2518: Only an ambient class can be merged with an interface.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(13,16): error TS2300: Duplicate identifier 'CC1'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(14,7): error TS2300: Duplicate identifier 'CC1'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(16,7): error TS2300: Duplicate identifier 'CC2'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(17,16): error TS2300: Duplicate identifier 'CC2'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(19,1): error TS2511: Cannot create an instance of the abstract class 'CM'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(20,1): error TS2511: Cannot create an instance of the abstract class 'MC'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(21,1): error TS2511: Cannot create an instance of the abstract class 'CI'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(22,5): error TS2304: Cannot find name 'IC'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(23,1): error TS2511: Cannot create an instance of the abstract class 'CC1'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(25,24): error TS2300: Duplicate identifier 'DCC1'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(26,15): error TS2300: Duplicate identifier 'DCC1'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(28,15): error TS2300: Duplicate identifier 'DCC2'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(29,24): error TS2300: Duplicate identifier 'DCC2'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(31,1): error TS2511: Cannot create an instance of the abstract class 'CM'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(32,1): error TS2511: Cannot create an instance of the abstract class 'MC'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(33,1): error TS2511: Cannot create an instance of the abstract class 'CI'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(34,1): error TS2511: Cannot create an instance of the abstract class 'IC'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(35,1): error TS2511: Cannot create an instance of the abstract class 'CC1'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(37,1): error TS2511: Cannot create an instance of the abstract class 'DCI'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(38,1): error TS2511: Cannot create an instance of the abstract class 'DIC'.
tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts(39,1): error TS2511: Cannot create an instance of the abstract class 'DCC1'.


==== tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts (13 errors) ====
==== tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbstractMergedDeclaration.ts (20 errors) ====
abstract class CM {}
module CM {}

Expand All @@ -22,17 +29,17 @@ tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbst

abstract class CI {}
~~
!!! error TS2300: Duplicate identifier 'CI'.
!!! error TS2518: Only an ambient class can be merged with an interface.
interface CI {}
~~
!!! error TS2300: Duplicate identifier 'CI'.
!!! error TS2518: Only an ambient class can be merged with an interface.

interface IC {}
~~
!!! error TS2300: Duplicate identifier 'IC'.
!!! error TS2518: Only an ambient class can be merged with an interface.
abstract class IC {}
~~
!!! error TS2300: Duplicate identifier 'IC'.
!!! error TS2518: Only an ambient class can be merged with an interface.

abstract class CC1 {}
~~~
Expand All @@ -48,6 +55,26 @@ tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbst
~~~
!!! error TS2300: Duplicate identifier 'CC2'.

declare abstract class DCI {}
interface DCI {}

interface DIC {}
declare abstract class DIC {}

declare abstract class DCC1 {}
~~~~
!!! error TS2300: Duplicate identifier 'DCC1'.
declare class DCC1 {}
~~~~
!!! error TS2300: Duplicate identifier 'DCC1'.

declare class DCC2 {}
~~~~
!!! error TS2300: Duplicate identifier 'DCC2'.
declare abstract class DCC2 {}
~~~~
!!! error TS2300: Duplicate identifier 'DCC2'.

new CM;
~~~~~~
!!! error TS2511: Cannot create an instance of the abstract class 'CM'.
Expand All @@ -58,9 +85,19 @@ tests/cases/conformance/classes/classDeclarations/classAbstractKeyword/classAbst
~~~~~~
!!! error TS2511: Cannot create an instance of the abstract class 'CI'.
new IC;
~~
!!! error TS2304: Cannot find name 'IC'.
~~~~~~
!!! error TS2511: Cannot create an instance of the abstract class 'IC'.
new CC1;
~~~~~~~
!!! error TS2511: Cannot create an instance of the abstract class 'CC1'.
new CC2;
new CC2;
new DCI;
~~~~~~~
!!! error TS2511: Cannot create an instance of the abstract class 'DCI'.
new DIC;
~~~~~~~
!!! error TS2511: Cannot create an instance of the abstract class 'DIC'.
new DCC1;
~~~~~~~~
!!! error TS2511: Cannot create an instance of the abstract class 'DCC1'.
new DCC2;
22 changes: 21 additions & 1 deletion tests/baselines/reference/classAbstractMergedDeclaration.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,28 @@ class CC1 {}
class CC2 {}
abstract class CC2 {}

declare abstract class DCI {}
interface DCI {}

interface DIC {}
declare abstract class DIC {}

declare abstract class DCC1 {}
declare class DCC1 {}

declare class DCC2 {}
declare abstract class DCC2 {}

new CM;
new MC;
new CI;
new IC;
new CC1;
new CC2;
new CC2;
new DCI;
new DIC;
new DCC1;
new DCC2;

//// [classAbstractMergedDeclaration.js]
var CM = (function () {
Expand Down Expand Up @@ -71,3 +87,7 @@ new CI;
new IC;
new CC1;
new CC2;
new DCI;
new DIC;
new DCC1;
new DCC2;
11 changes: 0 additions & 11 deletions tests/baselines/reference/classAndInterface1.errors.txt

This file was deleted.

10 changes: 0 additions & 10 deletions tests/baselines/reference/classAndInterface1.js

This file was deleted.

Loading