augment check for colliding declaration spaces

vladima · vladima · commit bb193fedb9cb · 2014-07-24T17:03:37.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -4941,28 +4941,34 @@ module ts {
                 }
             }
 
+            // run the check only for the first declaration in the list
             if (getDeclarationOfKind(symbol, node.kind) !== node) {
                 return;
             }
 
             // we use SymbolFlags.ExportValue, SymbolFlags.ExportType and SymbolFlags.ExportNamespace 
             // to denote disjoint declarationSpaces (without making new enum type).
-            var declarationSpaces: SymbolFlags = 0;
-            var hasExport: boolean;
+            var exportedDeclarationSpaces: SymbolFlags = 0;
+            var nonExportedDeclarationSpaces: SymbolFlags = 0;
+            forEach(symbol.declarations, d => {
+                var declarationSpaces = getDeclarationSpaces(d);
+                if (d.flags & NodeFlags.Export) {
+                    exportedDeclarationSpaces |= declarationSpaces;
+                }
+                else {
+                    nonExportedDeclarationSpaces |= declarationSpaces;
+                }
+            });
 
-            var declarations = symbol.declarations;
-            for (var i = 0, len = declarations.length; i < len; ++i) {
-                var currentDeclarationSpaces = getDeclarationSpaces(declarations[i]);
-                var currentDeclarationHasExport = (declarations[i].flags & NodeFlags.Export) !== 0;
-                if (declarationSpaces & currentDeclarationSpaces) {
-                    if (hasExport !== undefined) {
-                        if (hasExport !== currentDeclarationHasExport) {
-                            error(declarations[i].name, Diagnostics.All_declarations_of_merged_declaration_0_must_be_exported_or_not_exported, symbolToString(symbol));
-                        }
+            var commonDeclarationSpace = exportedDeclarationSpaces & nonExportedDeclarationSpaces
+
+            if (commonDeclarationSpace) {
+                // declaration spaces for exported and non-exported declarations intersect
+                forEach(symbol.declarations, d => {
+                    if (getDeclarationSpaces(d) & commonDeclarationSpace) {
+                        error(d.name, Diagnostics.Individual_declarations_in_a_merged_declaration_0_must_be_all_exported_or_all_local, identifierToString(d.name));
                     }
-                }
-                hasExport = hasExport || currentDeclarationHasExport;
-                declarationSpaces |= currentDeclarationSpaces;
+                });
             }
 
             function getDeclarationSpaces(d: Declaration): SymbolFlags {
@@ -4977,8 +4983,10 @@ module ts {
                     case SyntaxKind.EnumDeclaration:
                         return SymbolFlags.ExportType | SymbolFlags.ExportValue;
                     case SyntaxKind.ImportDeclaration:
+                        var result: SymbolFlags = 0;
                         var target = resolveImport(getSymbolOfNode(d));
-                        return target.flags & SymbolFlags.Export;
+                        forEach(target.declarations, d => { result |= getDeclarationSpaces(d); } ) 
+                        return result;
                     default:
                         return SymbolFlags.ExportValue;
                 }
@@ -5000,10 +5008,10 @@ module ts {
                 checkFunctionOrConstructorSymbol(localSymbol);
             }
 
-            if (symbol !== localSymbol) {
-                // here we'll check exported side of the symbol
-                Debug.assert(symbol.parent);
+            if (symbol.parent) {
+                // run check once for the first declaration
                 if (getDeclarationOfKind(symbol, node.kind) === node) {
+                    // run check on export symbol to check that modifiers agree across all exported declarations
                     checkFunctionOrConstructorSymbol(symbol);
                 }
             }
diff --git a/src/compiler/diagnosticInformationMap.generated.ts b/src/compiler/diagnosticInformationMap.generated.ts
@@ -137,7 +137,7 @@ module ts {
         A_signature_with_an_implementation_cannot_use_a_string_literal_type: { code: 2163, category: DiagnosticCategory.Error, key: "A signature with an implementation cannot use a string literal type." },
         Interface_0_cannot_simultaneously_extend_types_1_and_2_Colon: { code: 2189, category: DiagnosticCategory.Error, key: "Interface '{0}' cannot simultaneously extend types '{1}' and '{2}':" },
         Initializer_of_parameter_0_cannot_reference_identifier_1_declared_after_it: { code: 2190, category: DiagnosticCategory.Error, key: "Initializer of parameter '{0}' cannot reference identifier '{1}' declared after it." },
-        All_declarations_of_merged_declaration_0_must_be_exported_or_not_exported: { code: 2192, category: DiagnosticCategory.Error, key: "All declarations of merged declaration '{0}' must be exported or not exported." },
+        Individual_declarations_in_a_merged_declaration_0_must_be_all_exported_or_all_local: { code: 2192, category: DiagnosticCategory.Error, key: "Individual declarations in a merged declaration {0} must be all exported or all local." },
         super_cannot_be_referenced_in_constructor_arguments: { code: 2193, category: DiagnosticCategory.Error, key: "'super' cannot be referenced in constructor arguments." },
         Return_type_of_constructor_signature_must_be_assignable_to_the_instance_type_of_the_class: { code: 2194, category: DiagnosticCategory.Error, key: "Return type of constructor signature must be assignable to the instance type of the class" },
         Ambient_external_module_declaration_cannot_specify_relative_module_name: { code: 2196, category: DiagnosticCategory.Error, key: "Ambient external module declaration cannot specify relative module name." },
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -540,7 +540,7 @@
         "category": "Error",
         "code": 2190
     },
-    "All declarations of merged declaration '{0}' must be exported or not exported.": {
+    "Individual declarations in a merged declaration {0} must be all exported or all local.": {
         "category": "Error",
         "code": 2192
     },
diff --git a/tests/baselines/reference/anonymousModules.errors.txt b/tests/baselines/reference/anonymousModules.errors.txt
@@ -1,4 +1,4 @@
-==== tests/cases/compiler/anonymousModules.ts (12 errors) ====
+==== tests/cases/compiler/anonymousModules.ts (13 errors) ====
     module {
            ~
 !!! ';' expected.
@@ -18,13 +18,15 @@
     		export var bar = 1;
     		~~~~~~
 !!! Statement expected.
+    		           ~~~
+!!! Individual declarations in a merged declaration bar must be all exported or all local.
     	}
     	~
 !!! Declaration or statement expected.
     
     	var bar = 2;
     	    ~~~
-!!! All declarations of merged declaration 'bar' must be exported or not exported.
+!!! Individual declarations in a merged declaration bar must be all exported or all local.
     
     	module {
     	       ~
diff --git a/tests/baselines/reference/duplicateSymbolsExportMatching.errors.txt b/tests/baselines/reference/duplicateSymbolsExportMatching.errors.txt
@@ -1,4 +1,4 @@
-==== tests/cases/compiler/duplicateSymbolsExportMatching.ts (9 errors) ====
+==== tests/cases/compiler/duplicateSymbolsExportMatching.ts (18 errors) ====
     module M {
         export interface E { }
         interface I { }
@@ -23,62 +23,80 @@
     
     module N2 {
         interface I { }
+                  ~
+!!! Individual declarations in a merged declaration I must be all exported or all local.
         export interface I { } // error
                          ~
-!!! All declarations of merged declaration 'I' must be exported or not exported.
+!!! Individual declarations in a merged declaration I must be all exported or all local.
         export interface E { }
+                         ~
+!!! Individual declarations in a merged declaration E must be all exported or all local.
         interface E { } // error
                   ~
-!!! All declarations of merged declaration 'E' must be exported or not exported.
+!!! Individual declarations in a merged declaration E must be all exported or all local.
     }
     
     // Should report error only once for instantiated module
     module M {
         module inst {
+               ~~~~
+!!! Individual declarations in a merged declaration inst must be all exported or all local.
             var t;
         }
         export module inst { // one error
                       ~~~~
-!!! All declarations of merged declaration 'inst' must be exported or not exported.
+!!! Individual declarations in a merged declaration inst must be all exported or all local.
             var t;
         }
     }
     
     // Variables of the same / different type
     module M2 {
         var v: string;
+            ~
+!!! Individual declarations in a merged declaration v must be all exported or all local.
         export var v: string; // one error (visibility)
                    ~
-!!! All declarations of merged declaration 'v' must be exported or not exported.
+!!! Individual declarations in a merged declaration v must be all exported or all local.
         var w: number;
+            ~
+!!! Individual declarations in a merged declaration w must be all exported or all local.
         export var w: string; // two errors (visibility and type mismatch)
                    ~
-!!! All declarations of merged declaration 'w' must be exported or not exported.
+!!! Individual declarations in a merged declaration w must be all exported or all local.
     }
     
     module M {
         module F {
                ~
 !!! A module declaration cannot be located prior to a class or function with which it is merged
+               ~
+!!! Individual declarations in a merged declaration F must be all exported or all local.
             var t;
         }
         export function F() { } // Only one error for duplicate identifier (don't consider visibility)
                         ~
-!!! All declarations of merged declaration 'F' must be exported or not exported.
+!!! Individual declarations in a merged declaration F must be all exported or all local.
     }
     
     module M {
         class C { }
+              ~
+!!! Individual declarations in a merged declaration C must be all exported or all local.
         module C { }
+               ~
+!!! Individual declarations in a merged declaration C must be all exported or all local.
         export module C { // Two visibility errors (one for the clodule symbol, and one for the merged container symbol)
                       ~
-!!! All declarations of merged declaration 'C' must be exported or not exported.
+!!! Individual declarations in a merged declaration C must be all exported or all local.
             var t;
         }
     }
     
     // Top level
     interface D { }
+              ~
+!!! Individual declarations in a merged declaration D must be all exported or all local.
     export interface D { }
                      ~
-!!! All declarations of merged declaration 'D' must be exported or not exported.
+!!! Individual declarations in a merged declaration D must be all exported or all local.
diff --git a/tests/baselines/reference/mixedExports.js b/tests/baselines/reference/mixedExports.js
@@ -0,0 +1,14 @@
+//// [mixedExports.ts]
+declare module M {
+     function foo();
+     export function foo();
+     function foo();
+}
+
+module A {
+	interface X {x}
+	export module X {}
+	interface X {y}
+}
+
+//// [mixedExports.js]
diff --git a/tests/baselines/reference/multivar.errors.txt b/tests/baselines/reference/multivar.errors.txt
@@ -1,10 +1,12 @@
-==== tests/cases/compiler/multivar.ts (1 errors) ====
+==== tests/cases/compiler/multivar.ts (2 errors) ====
     var a,b,c;
     var x=1,y=2,z=3;
     
     module m2 {
     
         export var a, b2: number = 10, b;
+                      ~~
+!!! Individual declarations in a merged declaration b2 must be all exported or all local.
         var m1;
         var a2, b22: number = 10, b222;
         var m3;
@@ -22,7 +24,7 @@
         declare var d1, d2;
         var b2;
             ~~
-!!! All declarations of merged declaration 'b2' must be exported or not exported.
+!!! Individual declarations in a merged declaration b2 must be all exported or all local.
     
         declare var v1;
     }
diff --git a/tests/cases/compiler/mixedExports.ts b/tests/cases/compiler/mixedExports.ts
@@ -0,0 +1,11 @@
+declare module M {
+     function foo();
+     export function foo();
+     function foo();
+}
+
+module A {
+	interface X {x}
+	export module X {}
+	interface X {y}
+}

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-==== tests/cases/compiler/anonymousModules.ts (12 errors) ====`
	`1`	`+==== tests/cases/compiler/anonymousModules.ts (13 errors) ====`
`2`	`2`	`module {`
`3`	`3`	`~`
`4`	`4`	`!!! ';' expected.`
`@@ -18,13 +18,15 @@`
`18`	`18`	`export var bar = 1;`
`19`	`19`	`~~~~~~`
`20`	`20`	`!!! Statement expected.`
	`21`	`+ ~~~`
	`22`	`+!!! Individual declarations in a merged declaration bar must be all exported or all local.`
`21`	`23`	`}`
`22`	`24`	`~`
`23`	`25`	`!!! Declaration or statement expected.`
`24`	`26`
`25`	`27`	`var bar = 2;`
`26`	`28`	`~~~`
`27`		`-!!! All declarations of merged declaration 'bar' must be exported or not exported.`
	`29`	`+!!! Individual declarations in a merged declaration bar must be all exported or all local.`
`28`	`30`
`29`	`31`	`module {`
`30`	`32`	`~`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-==== tests/cases/compiler/duplicateSymbolsExportMatching.ts (9 errors) ====`
	`1`	`+==== tests/cases/compiler/duplicateSymbolsExportMatching.ts (18 errors) ====`
`2`	`2`	`module M {`
`3`	`3`	`export interface E { }`
`4`	`4`	`interface I { }`
`@@ -23,62 +23,80 @@`
`23`	`23`
`24`	`24`	`module N2 {`
`25`	`25`	`interface I { }`
	`26`	`+ ~`
	`27`	`+!!! Individual declarations in a merged declaration I must be all exported or all local.`
`26`	`28`	`export interface I { } // error`
`27`	`29`	`~`
`28`		`-!!! All declarations of merged declaration 'I' must be exported or not exported.`
	`30`	`+!!! Individual declarations in a merged declaration I must be all exported or all local.`
`29`	`31`	`export interface E { }`
	`32`	`+ ~`
	`33`	`+!!! Individual declarations in a merged declaration E must be all exported or all local.`
`30`	`34`	`interface E { } // error`
`31`	`35`	`~`
`32`		`-!!! All declarations of merged declaration 'E' must be exported or not exported.`
	`36`	`+!!! Individual declarations in a merged declaration E must be all exported or all local.`
`33`	`37`	`}`
`34`	`38`
`35`	`39`	`// Should report error only once for instantiated module`
`36`	`40`	`module M {`
`37`	`41`	`module inst {`
	`42`	`+ ~~~~`
	`43`	`+!!! Individual declarations in a merged declaration inst must be all exported or all local.`
`38`	`44`	`var t;`
`39`	`45`	`}`
`40`	`46`	`export module inst { // one error`
`41`	`47`	`~~~~`
`42`		`-!!! All declarations of merged declaration 'inst' must be exported or not exported.`
	`48`	`+!!! Individual declarations in a merged declaration inst must be all exported or all local.`
`43`	`49`	`var t;`
`44`	`50`	`}`
`45`	`51`	`}`
`46`	`52`
`47`	`53`	`// Variables of the same / different type`
`48`	`54`	`module M2 {`
`49`	`55`	`var v: string;`
	`56`	`+ ~`
	`57`	`+!!! Individual declarations in a merged declaration v must be all exported or all local.`
`50`	`58`	`export var v: string; // one error (visibility)`
`51`	`59`	`~`
`52`		`-!!! All declarations of merged declaration 'v' must be exported or not exported.`
	`60`	`+!!! Individual declarations in a merged declaration v must be all exported or all local.`
`53`	`61`	`var w: number;`
	`62`	`+ ~`
	`63`	`+!!! Individual declarations in a merged declaration w must be all exported or all local.`
`54`	`64`	`export var w: string; // two errors (visibility and type mismatch)`
`55`	`65`	`~`
`56`		`-!!! All declarations of merged declaration 'w' must be exported or not exported.`
	`66`	`+!!! Individual declarations in a merged declaration w must be all exported or all local.`
`57`	`67`	`}`
`58`	`68`
`59`	`69`	`module M {`
`60`	`70`	`module F {`
`61`	`71`	`~`
`62`	`72`	`!!! A module declaration cannot be located prior to a class or function with which it is merged`
	`73`	`+ ~`
	`74`	`+!!! Individual declarations in a merged declaration F must be all exported or all local.`
`63`	`75`	`var t;`
`64`	`76`	`}`
`65`	`77`	`export function F() { } // Only one error for duplicate identifier (don't consider visibility)`
`66`	`78`	`~`
`67`		`-!!! All declarations of merged declaration 'F' must be exported or not exported.`
	`79`	`+!!! Individual declarations in a merged declaration F must be all exported or all local.`
`68`	`80`	`}`
`69`	`81`
`70`	`82`	`module M {`
`71`	`83`	`class C { }`
	`84`	`+ ~`
	`85`	`+!!! Individual declarations in a merged declaration C must be all exported or all local.`
`72`	`86`	`module C { }`
	`87`	`+ ~`
	`88`	`+!!! Individual declarations in a merged declaration C must be all exported or all local.`
`73`	`89`	`export module C { // Two visibility errors (one for the clodule symbol, and one for the merged container symbol)`
`74`	`90`	`~`
`75`		`-!!! All declarations of merged declaration 'C' must be exported or not exported.`
	`91`	`+!!! Individual declarations in a merged declaration C must be all exported or all local.`
`76`	`92`	`var t;`
`77`	`93`	`}`
`78`	`94`	`}`
`79`	`95`
`80`	`96`	`// Top level`
`81`	`97`	`interface D { }`
	`98`	`+ ~`
	`99`	`+!!! Individual declarations in a merged declaration D must be all exported or all local.`
`82`	`100`	`export interface D { }`
`83`	`101`	`~`
`84`		`-!!! All declarations of merged declaration 'D' must be exported or not exported.`
	`102`	`+!!! Individual declarations in a merged declaration D must be all exported or all local.`