Preserve comments on elided import declarations

src/compiler/emitter.ts

@@ -727,7 +727,7 @@ namespace ts {
                     case SyntaxKind.VariableStatement:
                         return emitVariableStatement(<VariableStatement>node);
                     case SyntaxKind.EmptyStatement:
-                        return emitEmptyStatement();
+                        return !(getEmitFlags(node) & EmitFlags.CommentsOnly) ? emitEmptyStatement() : undefined;
                     case SyntaxKind.ExpressionStatement:
                         return emitExpressionStatement(<ExpressionStatement>node);
                     case SyntaxKind.IfStatement:

src/compiler/factory.ts

@@ -3355,6 +3355,11 @@ namespace ts {
 
     // Utilities
 
+    /** @internal */
+    export function createCommentPreservationStatement(node: Statement): Statement {
+        return addEmitFlags(setCommentRange(setOriginalNode(createEmptyStatement(), node), node), EmitFlags.CommentsOnly);
+    }
+
     export function createForOfBindingStatement(node: ForInitializer, boundValue: Expression): Statement {
         if (isVariableDeclarationList(node)) {
             const firstDeclaration = first(node.declarations);

src/compiler/transformers/module/module.ts

@@ -850,6 +850,9 @@ namespace ts {
                     )
                 );
             }
+            else if (hasLeadingOrTrailingComments(node, currentSourceFile)) {
+                statements = append(statements, createCommentPreservationStatement(node));
+            }
 
             if (hasAssociatedEndOfDeclarationMarker(node)) {
                 // Defer exports until we encounter an EndOfDeclarationMarker node
@@ -939,6 +942,9 @@ namespace ts {
                         )
                     );
                 }
+                else if (hasLeadingOrTrailingComments(node, currentSourceFile)) {
+                    statements = append(statements, createCommentPreservationStatement(node));
+                }
             }
 
             if (hasAssociatedEndOfDeclarationMarker(node)) {

src/compiler/transformers/utilities.ts

@@ -255,4 +255,8 @@ namespace ts {
             return result;
         };
     }
+
+    export function hasLeadingOrTrailingComments(node: Node, sourceFile: SourceFile) {
+        return !!(getSyntheticLeadingComments(node) || getSyntheticTrailingComments(node) || getLeadingCommentRangesOfNode(node, sourceFile) || getTrailingCommentRanges(sourceFile.text, node.pos));
+    }
 }

src/compiler/types.ts

@@ -5028,6 +5028,7 @@ namespace ts {
         NoAsciiEscaping = 1 << 24,              // When synthesizing nodes that lack an original node or textSourceNode, we want to write the text on the node with ASCII escaping substitutions.
         /*@internal*/ TypeScriptClassWrapper = 1 << 25, // The node is an IIFE class wrapper created by the ts transform.
         /*@internal*/ NeverApplyImportHelper = 1 << 26, // Indicates the node should never be wrapped with an import star helper (because, for example, it imports tslib itself)
+        /*@internal*/ CommentsOnly = 1 << 27,   // Only for empty statements, incidcates that the statement exists for comment preservation, so the semicolon may be elided
     }
 
     export interface EmitHelper {

tests/baselines/reference/ambientExternalModuleInAnotherExternalModule.js

@@ -18,6 +18,7 @@ define(["require", "exports", "ext"], function (require, exports, ext) {
         }
         return D;
     }());
+    // Cannot resolve this ext module reference
     var x = ext;
     return D;
 });

tests/baselines/reference/ambientExternalModuleWithInternalImportDeclaration.js

@@ -23,5 +23,6 @@ var c = new A();
 define(["require", "exports", "M"], function (require, exports, A) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='ambientExternalModuleWithInternalImportDeclaration_0.ts'/>
     var c = new A();
 });

tests/baselines/reference/ambientExternalModuleWithoutInternalImportDeclaration.js

@@ -22,5 +22,6 @@ var c = new A();
 define(["require", "exports", "M"], function (require, exports, A) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='ambientExternalModuleWithoutInternalImportDeclaration_0.ts'/>
     var c = new A();
 });

tests/baselines/reference/augmentExportEquals3_1.js

@@ -31,6 +31,7 @@ let b = x.b;
 define(["require", "exports", "file1"], function (require, exports, x) {
     "use strict";
     exports.__esModule = true;
+    /// <reference path="file1.d.ts"/>
     x.b = 1;
 });
 //// [file3.js]

tests/baselines/reference/augmentExportEquals4_1.js

@@ -31,6 +31,7 @@ let b = x.b;
 define(["require", "exports", "file1"], function (require, exports, x) {
     "use strict";
     exports.__esModule = true;
+    /// <reference path="file1.d.ts"/>
     x.b = 1;
 });
 //// [file3.js]

tests/baselines/reference/collisionExportsRequireAndAlias.js

@@ -37,6 +37,7 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "collisionExportsRequireAndAlias_file1", "collisionExportsRequireAndAlias_file3333"], function (require, exports, require, exports) {
     "use strict";
     exports.__esModule = true;
+    // Error
     function foo() {
         require.bar();
     }

tests/baselines/reference/commentsExternalModules.js

@@ -129,6 +129,8 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "commentsExternalModules_0"], function (require, exports, extMod) {
     "use strict";
     Object.defineProperty(exports, "__esModule", { value: true });
+    /**This is on import declaration*/
+    // trailing comment1
     extMod.m1.fooExport();
     var newVar = new extMod.m1.m2.c();
     extMod.m4.fooExport();

tests/baselines/reference/commentsExternalModules2.js

@@ -129,6 +129,8 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "commentsExternalModules2_0"], function (require, exports, extMod) {
     "use strict";
     Object.defineProperty(exports, "__esModule", { value: true });
+    /**This is on import declaration*/
+    // trailing comment 1
     extMod.m1.fooExport();
     exports.newVar = new extMod.m1.m2.c();
     extMod.m4.fooExport();

tests/baselines/reference/commentsOnImportInAMDPreserved.js

@@ -0,0 +1,10 @@
+//// [commentsOnImportInAMDPreserved.ts]
+// foo
+import 'foo';
+
+//// [commentsOnImportInAMDPreserved.js]
+define(["require", "exports", "foo"], function (require, exports) {
+    "use strict";
+    exports.__esModule = true;
+    // foo
+});

tests/baselines/reference/commentsOnImportInAMDPreserved.symbols

@@ -0,0 +1,4 @@
+=== tests/cases/compiler/commentsOnImportInAMDPreserved.ts ===
+// foo
+No type information for this code.import 'foo';
+No type information for this code.

tests/baselines/reference/commentsOnImportInAMDPreserved.types

@@ -0,0 +1,4 @@
+=== tests/cases/compiler/commentsOnImportInAMDPreserved.ts ===
+// foo
+No type information for this code.import 'foo';
+No type information for this code.

tests/baselines/reference/constDeclarations-access5.js

@@ -56,6 +56,7 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "constDeclarations_access_1"], function (require, exports, m) {
     "use strict";
     Object.defineProperty(exports, "__esModule", { value: true });
+    ///<reference path='constDeclarations_access_1.ts'/>
     // Errors
     m.x = 1;
     m.x += 2;

tests/baselines/reference/copyrightWithoutNewLine1.js

@@ -12,6 +12,9 @@ greeter.start();
 define(["require", "exports", "./greeter"], function (require, exports, model) {
     "use strict";
     exports.__esModule = true;
+    /*****************************
+    * (c) Copyright - Important
+    ****************************/
     var el = document.getElementById('content');
     var greeter = new model.Greeter(el);
     /** things */

tests/baselines/reference/exportAssignmentOfGenericType1.js

@@ -40,6 +40,7 @@ var __extends = (this && this.__extends) || (function () {
 define(["require", "exports", "exportAssignmentOfGenericType1_0"], function (require, exports, q) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='exportAssignmentOfGenericType1_0.ts'/>
     var M = /** @class */ (function (_super) {
         __extends(M, _super);
         function M() {

tests/baselines/reference/exportEqualCallable.js

@@ -22,5 +22,6 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "exportEqualCallable_0"], function (require, exports, connect) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='exportEqualCallable_0.ts'/>
     connect();
 });

tests/baselines/reference/exportEqualErrorType.js

@@ -31,5 +31,6 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "exportEqualErrorType_0"], function (require, exports, connect) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='exportEqualErrorType_0.ts'/>
     connect().use(connect.static('foo')); // Error  1      The property 'static' does not exist on value of type ''.
 });

tests/baselines/reference/externalModuleAssignToVar.js

@@ -61,6 +61,7 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "externalModuleAssignToVar_core_require", "externalModuleAssignToVar_core_require2", "externalModuleAssignToVar_ext"], function (require, exports, ext, ext2, ext3) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='externalModuleAssignToVar_core_require.ts'/>
     var y1 = ext;
     y1 = ext; // ok
     var y2 = ext2;

tests/baselines/reference/instanceOfInExternalModules.js

@@ -26,6 +26,7 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "instanceOfInExternalModules_require"], function (require, exports, Bar) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='instanceOfInExternalModules_require.ts'/>
     function IsFoo(value) {
         return value instanceof Bar.Foo;
     }

tests/baselines/reference/memberAccessMustUseModuleInstances.js

@@ -32,5 +32,6 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "memberAccessMustUseModuleInstances_0"], function (require, exports, WinJS) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='memberAccessMustUseModuleInstances_0.ts'/>
     WinJS.Promise.timeout(10);
 });

tests/baselines/reference/moduleAliasAsFunctionArgument.js

@@ -23,6 +23,7 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "moduleAliasAsFunctionArgument_0"], function (require, exports, a) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='moduleAliasAsFunctionArgument_0.ts'/>
     function fn(arg) {
     }
     a.x; // OK

tests/baselines/reference/moduleAugmentationsImports3.js

@@ -69,6 +69,7 @@ define("b", ["require", "exports"], function (require, exports) {
 define("e", ["require", "exports", "a"], function (require, exports, a_1) {
     "use strict";
     exports.__esModule = true;
+    /// <reference path="c.d.ts"/>
     a_1.A.prototype.getCls = function () { return undefined; };
 });
 define("main", ["require", "exports", "D", "e"], function (require, exports) {

tests/baselines/reference/pathMappingBasedModuleResolution3_classic.js

@@ -34,6 +34,7 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "./file3", "file4"], function (require, exports, file3_1, file4_1) {
     "use strict";
     exports.__esModule = true;
+    // found with fallback
     exports.x = file3_1.x + file4_1.y;
 });
 //// [file1.js]

tests/baselines/reference/pathMappingBasedModuleResolution4_classic.js

@@ -32,6 +32,7 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "./file3", "file4"], function (require, exports, file3_1, file4_1) {
     "use strict";
     exports.__esModule = true;
+    // found with fallback
     exports.x = file3_1.x + file4_1.y;
 });
 //// [file1.js]

tests/baselines/reference/privacyTopLevelAmbientExternalModuleImportWithoutExport.js

@@ -76,6 +76,9 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "m", "m2", "privacyTopLevelAmbientExternalModuleImportWithoutExport_require"], function (require, exports, im_private_mi_private, im_private_mu_private, im_private_mi_public) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='privacyTopLevelAmbientExternalModuleImportWithoutExport_require2.ts'/>
+    ///<reference path='privacyTopLevelAmbientExternalModuleImportWithoutExport_require3.ts'/>
+    // Privacy errors - importing private elements
     // Usage of privacy error imports
     var privateUse_im_private_mi_private = new im_private_mi_private.c_private();
     exports.publicUse_im_private_mi_private = new im_private_mi_private.c_private();

tests/baselines/reference/project/declarationsImportedInPrivate/amd/useModule.js

@@ -1,6 +1,7 @@
 define(["require", "exports", "private_m4"], function (require, exports, private_m4) {
     "use strict";
     exports.__esModule = true;
+    // only used privately no need to emit
     var usePrivate_m4_m1;
     (function (usePrivate_m4_m1) {
         var x3 = private_m4.x;

tests/baselines/reference/project/declarationsIndirectImportShouldResultInError/amd/useModule.js

@@ -1,6 +1,7 @@
 define(["require", "exports", "m5"], function (require, exports, m5) {
     "use strict";
     exports.__esModule = true;
+    // Do not emit unused import
     exports.d = m5.foo2();
     exports.x = m5.foo2;
     function n() {

tests/baselines/reference/project/declarationsMultipleTimesImport/amd/useModule.js

@@ -13,6 +13,8 @@ define(["require", "exports", "m4", "m4"], function (require, exports, m4, multi
         var d3 = m4.d;
         var f3 = m4.foo();
     })(m1 = exports.m1 || (exports.m1 = {}));
+    // Do not emit multiple used import statements
+    // Emit used
     exports.useMultiImport_m4_x4 = multiImport_m4.x;
     exports.useMultiImport_m4_d4 = multiImport_m4.d;
     exports.useMultiImport_m4_f4 = multiImport_m4.foo();

tests/baselines/reference/project/declarationsMultipleTimesMultipleImport/amd/useModule.js

@@ -13,5 +13,6 @@ define(["require", "exports", "m4", "m5"], function (require, exports, m4, m5) {
         var d3 = m4.d;
         var f3 = m4.foo();
     })(m1 = exports.m1 || (exports.m1 = {}));
+    // Do not emit unused import
     exports.d = m5.foo2();
 });

tests/baselines/reference/requireEmitSemicolon.js

@@ -37,6 +37,8 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "requireEmitSemicolon_0"], function (require, exports, P) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path='requireEmitSemicolon_0.ts'/>
+    // bug was we were not emitting a ; here and causing runtime failures in node
     var Database;
     (function (Database) {
         var DB = /** @class */ (function () {

tests/baselines/reference/tsxElementResolution17.js

@@ -31,5 +31,7 @@ import s2 = require('elements2');
 define(["require", "exports", "elements1"], function (require, exports, s1) {
     "use strict";
     exports.__esModule = true;
+    ///<reference path="file.tsx" />
+    // Should keep s1 and elide s2
     <s1.MyElement />;
 });

tests/baselines/reference/tsxElementResolution19.js

@@ -34,5 +34,6 @@ define(["require", "exports"], function (require, exports) {
 define(["require", "exports", "react", "./file1"], function (require, exports, React, file1_1) {
     "use strict";
     exports.__esModule = true;
+    // Should not elide React import
     React.createElement(file1_1.MyClass, null);
 });

tests/baselines/reference/tsxPreserveEmit1.js

@@ -36,6 +36,7 @@ module M {
 define(["require", "exports", "react", "react-router"], function (require, exports, React, ReactRouter) {
     "use strict";
     Object.defineProperty(exports, "__esModule", { value: true });
+    // Should emit 'react-router' in the AMD dependency list
     var Route = ReactRouter.Route;
     var routes1 = <Route />;
     var M;

tests/cases/compiler/commentsOnImportInAMDPreserved.ts

@@ -0,0 +1,3 @@
+// @module: amd
+// foo
+import 'foo';