Merge pull request #569 from Microsoft/collisions

Report error if exports or require is used for declaration name in external module

Author: sheetalkamat

src/compiler/checker.ts

@@ -4957,6 +4957,7 @@ module ts {
             if (fullTypeCheck) {
                 checkCollisionWithCapturedSuperVariable(node, node.name);
                 checkCollisionWithCapturedThisVariable(node, node.name);
+                checkCollistionWithRequireExportsInGeneratedCode(node, node.name);
                 checkCollisionWithArgumentsInGeneratedCode(node);
                 if (program.getCompilerOptions().noImplicitAny && !node.type) {
                     switch (node.kind) {
@@ -5699,6 +5700,24 @@ module ts {
             }
         }
 
+        function checkCollistionWithRequireExportsInGeneratedCode(node: Node, name: Identifier) {
+            if (!needCollisionCheckForIdentifier(node, name, "require") && !needCollisionCheckForIdentifier(node, name, "exports")) {
+                return;
+            }
+
+            // Uninstantiated modules shouldnt do this check
+            if (node.kind === SyntaxKind.ModuleDeclaration && !isInstantiated(node)) {
+                return;
+            }
+
+            // In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
+            var parent = node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent : node.parent;
+            if (parent.kind === SyntaxKind.SourceFile && isExternalModule(<SourceFile>parent)) {
+                // If the declaration happens to be in external module, report error that require and exports are reserved keywords
+                error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_an_external_module, name.text, name.text);
+            }
+        }
+
         function checkVariableDeclaration(node: VariableDeclaration) {
             checkSourceElement(node.type);
             checkExportsOnMergedDeclarations(node);
@@ -5726,6 +5745,7 @@ module ts {
 
                 checkCollisionWithCapturedSuperVariable(node, node.name);
                 checkCollisionWithCapturedThisVariable(node, node.name);
+                checkCollistionWithRequireExportsInGeneratedCode(node, node.name);
                 if (!useTypeFromValueDeclaration) {
                     // TypeScript 1.0 spec (April 2014): 5.1
                     // Multiple declarations for the same variable name in the same declaration space are permitted,
@@ -5995,6 +6015,7 @@ module ts {
             checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
             checkTypeParameters(node.typeParameters);
             checkCollisionWithCapturedThisVariable(node, node.name);
+            checkCollistionWithRequireExportsInGeneratedCode(node, node.name);
             checkExportsOnMergedDeclarations(node);
             var symbol = getSymbolOfNode(node);
             var type = <InterfaceType>getDeclaredTypeOfSymbol(symbol);
@@ -6210,6 +6231,7 @@ module ts {
             }
             checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
             checkCollisionWithCapturedThisVariable(node, node.name);
+            checkCollistionWithRequireExportsInGeneratedCode(node, node.name);
             checkExportsOnMergedDeclarations(node);
             var enumSymbol = getSymbolOfNode(node);
             var enumType = getDeclaredTypeOfSymbol(enumSymbol);
@@ -6283,6 +6305,7 @@ module ts {
         function checkModuleDeclaration(node: ModuleDeclaration) {
             if (fullTypeCheck) {
                 checkCollisionWithCapturedThisVariable(node, node.name);
+                checkCollistionWithRequireExportsInGeneratedCode(node, node.name);
                 checkExportsOnMergedDeclarations(node);
                 var symbol = getSymbolOfNode(node);
                 if (symbol.flags & SymbolFlags.ValueModule && symbol.declarations.length > 1 && !isInAmbientContext(node)) {
@@ -6317,6 +6340,7 @@ module ts {
 
         function checkImportDeclaration(node: ImportDeclaration) {
             checkCollisionWithCapturedThisVariable(node, node.name);
+            checkCollistionWithRequireExportsInGeneratedCode(node, node.name);
             var symbol = getSymbolOfNode(node);
             var target: Symbol;
 

src/compiler/diagnosticInformationMap.generated.ts

@@ -253,6 +253,7 @@ module ts {
         Import_name_cannot_be_0: { code: 2438, category: DiagnosticCategory.Error, key: "Import name cannot be '{0}'" },
         Import_declaration_in_an_ambient_external_module_declaration_cannot_reference_external_module_through_relative_external_module_name: { code: 2439, category: DiagnosticCategory.Error, key: "Import declaration in an ambient external module declaration cannot reference external module through relative external module name." },
         Import_declaration_conflicts_with_local_declaration_of_0: { code: 2440, category: DiagnosticCategory.Error, key: "Import declaration conflicts with local declaration of '{0}'" },
+        Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_an_external_module: { code: 2441, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of an external module." },
         Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
         Type_parameter_0_of_exported_class_has_or_is_using_name_1_from_private_module_2: { code: 4001, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using name '{1}' from private module '{2}'." },
         Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 4002, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using private name '{1}'." },

src/compiler/diagnosticMessages.json

@@ -1004,6 +1004,11 @@
         "category": "Error",
         "code": 2440
     },
+    "Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of an external module.": {
+        "category": "Error",
+        "code": 2441
+    },
+
 
     "Import declaration '{0}' is using private name '{1}'.": {
         "category": "Error",

tests/baselines/reference/collisionExportsRequireAndAlias.errors.txt

@@ -0,0 +1,20 @@
+==== tests/cases/compiler/collisionExportsRequireAndAlias_file2.ts (2 errors) ====
+    import require = require('collisionExportsRequireAndAlias_file1'); // Error
+           ~~~~~~~
+!!! Duplicate identifier 'require'. Compiler reserves name 'require' in top level scope of an external module.
+    import exports = require('collisionExportsRequireAndAlias_file3333'); // Error
+           ~~~~~~~
+!!! Duplicate identifier 'exports'. Compiler reserves name 'exports' in top level scope of an external module.
+    export function foo() {
+        require.bar();
+    }
+    export function foo2() {
+        exports.bar2();
+    }
+==== tests/cases/compiler/collisionExportsRequireAndAlias_file1.ts (0 errors) ====
+    export function bar() {
+    }
+    
+==== tests/cases/compiler/collisionExportsRequireAndAlias_file3333.ts (0 errors) ====
+    export function bar2() {
+    }

tests/baselines/reference/collisionExportsRequireAndAlias.js

@@ -1,23 +1,42 @@
-//// [collisionExportsRequireAndAlias.ts]
-// TODO: re-enable, fails when run in the browser with full compiler suite, but not when run alone
-
-////@module: amd
-//// @Filename: collisionExportsRequireAndAlias_file1.ts
-//export function bar() {
-//}
-
-//// @Filename: collisionExportsRequireAndAlias_file3333.ts
-//export function bar2() {
-//}
-//// @Filename: collisionExportsRequireAndAlias_file2.ts
-//import require = require('collisionExportsRequireAndAlias_file1'); // Error
-//import exports = require('collisionExportsRequireAndAlias_file3333'); // Error
-//export function foo() {
-//    require.bar();
-//}
-//export function foo2() {
-//    exports.bar2();
-//}
+//// [tests/cases/compiler/collisionExportsRequireAndAlias.ts] ////
 
-//// [collisionExportsRequireAndAlias.js]
-// TODO: re-enable, fails when run in the browser with full compiler suite, but not when run alone
+//// [collisionExportsRequireAndAlias_file1.ts]
+export function bar() {
+}
+
+//// [collisionExportsRequireAndAlias_file3333.ts]
+export function bar2() {
+}
+//// [collisionExportsRequireAndAlias_file2.ts]
+import require = require('collisionExportsRequireAndAlias_file1'); // Error
+import exports = require('collisionExportsRequireAndAlias_file3333'); // Error
+export function foo() {
+    require.bar();
+}
+export function foo2() {
+    exports.bar2();
+}
+
+//// [collisionExportsRequireAndAlias_file1.js]
+define(["require", "exports"], function (require, exports) {
+    function bar() {
+    }
+    exports.bar = bar;
+});
+//// [collisionExportsRequireAndAlias_file3333.js]
+define(["require", "exports"], function (require, exports) {
+    function bar2() {
+    }
+    exports.bar2 = bar2;
+});
+//// [collisionExportsRequireAndAlias_file2.js]
+define(["require", "exports", 'collisionExportsRequireAndAlias_file1', 'collisionExportsRequireAndAlias_file3333'], function (require, exports, require, exports) {
+    function foo() {
+        require.bar();
+    }
+    exports.foo = foo;
+    function foo2() {
+        exports.bar2();
+    }
+    exports.foo2 = foo2;
+});

tests/baselines/reference/collisionExportsRequireAndAlias.types

@@ -0,0 +1,50 @@
+//// [tests/cases/compiler/collisionExportsRequireAndAmbientClass.ts] ////
+
+//// [collisionExportsRequireAndAmbientClass_externalmodule.ts]
+export declare class require {
+}
+export declare class exports {
+}
+declare module m1 {
+    class require {
+    }
+    class exports {
+    }
+}
+module m2 {
+    export declare class require {
+    }
+    export declare class exports {
+    }
+}
+
+//// [collisionExportsRequireAndAmbientClass_globalFile.ts]
+declare class require {
+}
+declare class exports {
+}
+declare module m3 {
+    class require {
+    }
+    class exports {
+    }
+}
+module m4 {
+    export declare class require {
+    }
+    export declare class exports {
+    }
+    var a = 10;
+}
+
+//// [collisionExportsRequireAndAmbientClass_externalmodule.js]
+define(["require", "exports"], function (require, exports) {
+    var m2;
+    (function (m2) {
+    })(m2 || (m2 = {}));
+});
+//// [collisionExportsRequireAndAmbientClass_globalFile.js]
+var m4;
+(function (m4) {
+    var a = 10;
+})(m4 || (m4 = {}));

tests/baselines/reference/collisionExportsRequireAndAmbientClass.js

@@ -0,0 +1,57 @@
+=== tests/cases/compiler/collisionExportsRequireAndAmbientClass_externalmodule.ts ===
+export declare class require {
+>require : require
+}
+export declare class exports {
+>exports : exports
+}
+declare module m1 {
+>m1 : typeof m1
+
+    class require {
+>require : require
+    }
+    class exports {
+>exports : exports
+    }
+}
+module m2 {
+>m2 : typeof m2
+
+    export declare class require {
+>require : require
+    }
+    export declare class exports {
+>exports : exports
+    }
+}
+
+=== tests/cases/compiler/collisionExportsRequireAndAmbientClass_globalFile.ts ===
+declare class require {
+>require : require
+}
+declare class exports {
+>exports : exports
+}
+declare module m3 {
+>m3 : typeof m3
+
+    class require {
+>require : require
+    }
+    class exports {
+>exports : exports
+    }
+}
+module m4 {
+>m4 : typeof m4
+
+    export declare class require {
+>require : require
+    }
+    export declare class exports {
+>exports : exports
+    }
+    var a = 10;
+>a : number
+}

tests/baselines/reference/collisionExportsRequireAndAmbientClass.types

@@ -0,0 +1,72 @@
+//// [tests/cases/compiler/collisionExportsRequireAndAmbientEnum.ts] ////
+
+//// [collisionExportsRequireAndAmbientEnum_externalmodule.ts]
+export declare enum require {
+    _thisVal1,
+    _thisVal2,
+}
+export declare enum exports {
+    _thisVal1,
+    _thisVal2,
+}
+declare module m1 {
+    enum require {
+        _thisVal1,
+        _thisVal2,
+    }
+    enum exports {
+        _thisVal1,
+        _thisVal2,
+    }
+}
+module m2 {
+    export declare enum require { 
+        _thisVal1,
+        _thisVal2,
+    }
+    export declare enum exports {
+        _thisVal1,
+        _thisVal2,
+    }
+}
+
+//// [collisionExportsRequireAndAmbientEnum_globalFile.ts]
+declare enum require {
+    _thisVal1,
+    _thisVal2,
+}
+declare enum exports {
+    _thisVal1,
+    _thisVal2,
+}
+declare module m3 {
+    enum require {
+        _thisVal1,
+        _thisVal2,
+    }
+    enum exports {
+        _thisVal1,
+        _thisVal2,
+    }
+}
+module m4 {
+    export declare enum require {
+        _thisVal1,
+        _thisVal2,
+    }
+    export declare enum exports {
+        _thisVal1,
+        _thisVal2,
+    }
+}
+
+//// [collisionExportsRequireAndAmbientEnum_externalmodule.js]
+define(["require", "exports"], function (require, exports) {
+    var m2;
+    (function (m2) {
+    })(m2 || (m2 = {}));
+});
+//// [collisionExportsRequireAndAmbientEnum_globalFile.js]
+var m4;
+(function (m4) {
+})(m4 || (m4 = {}));