Fix emit for object rest on a module export (microsoft#32699)

* Fix emit for object rest on a module export

* Add tests for exports of empty object/array binding patterns

* Add delay for exec to ensure diff tool has enough time to start

Author: rbuckton

Gulpfile.js

@@ -436,10 +436,10 @@ task("runtests-parallel").flags = {
     "   --shardId": "1-based ID of this shard (default: 1)",
 };
 
-task("diff", () => exec(getDiffTool(), [refBaseline, localBaseline], { ignoreExitCode: true }));
+task("diff", () => exec(getDiffTool(), [refBaseline, localBaseline], { ignoreExitCode: true, waitForExit: false }));
 task("diff").description = "Diffs the compiler baselines using the diff tool specified by the 'DIFF' environment variable";
 
-task("diff-rwc", () => exec(getDiffTool(), [refRwcBaseline, localRwcBaseline], { ignoreExitCode: true }));
+task("diff-rwc", () => exec(getDiffTool(), [refRwcBaseline, localRwcBaseline], { ignoreExitCode: true, waitForExit: false }));
 task("diff-rwc").description = "Diffs the RWC baselines using the diff tool specified by the 'DIFF' environment variable";
 
 /**

scripts/build/utils.js

@@ -25,37 +25,45 @@ const isWindows = /^win/.test(process.platform);
  * @property {boolean} [ignoreExitCode]
  * @property {import("prex").CancellationToken} [cancelToken]
  * @property {boolean} [hidePrompt]
+ * @property {boolean} [waitForExit=true]
  */
 function exec(cmd, args, options = {}) {
     return /**@type {Promise<{exitCode: number}>}*/(new Promise((resolve, reject) => {
-        const { ignoreExitCode, cancelToken = CancellationToken.none } = options;
+        const { ignoreExitCode, cancelToken = CancellationToken.none, waitForExit = true } = options;
         cancelToken.throwIfCancellationRequested();
 
         // TODO (weswig): Update child_process types to add windowsVerbatimArguments to the type definition
         const subshellFlag = isWindows ? "/c" : "-c";
         const command = isWindows ? [possiblyQuote(cmd), ...args] : [`${cmd} ${args.join(" ")}`];
 
         if (!options.hidePrompt) log(`> ${chalk.green(cmd)} ${args.join(" ")}`);
-        const proc = spawn(isWindows ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: "inherit", windowsVerbatimArguments: true });
+        const proc = spawn(isWindows ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: waitForExit ? "inherit" : "ignore", windowsVerbatimArguments: true });
         const registration = cancelToken.register(() => {
             log(`${chalk.red("killing")} '${chalk.green(cmd)} ${args.join(" ")}'...`);
             proc.kill("SIGINT");
             proc.kill("SIGTERM");
             reject(new CancelError());
         });
-        proc.on("exit", exitCode => {
-            registration.unregister();
-            if (exitCode === 0 || ignoreExitCode) {
-                resolve({ exitCode });
-            }
-            else {
-                reject(new Error(`Process exited with code: ${exitCode}`));
-            }
-        });
-        proc.on("error", error => {
-            registration.unregister();
-            reject(error);
-        });
+        if (waitForExit) {
+            proc.on("exit", exitCode => {
+                registration.unregister();
+                if (exitCode === 0 || ignoreExitCode) {
+                    resolve({ exitCode });
+                }
+                else {
+                    reject(new Error(`Process exited with code: ${exitCode}`));
+                }
+            });
+            proc.on("error", error => {
+                registration.unregister();
+                reject(error);
+            });
+        }
+        else {
+            proc.unref();
+            // wait a short period in order to allow the process to start successfully before Node exits.
+            setTimeout(() => resolve({ exitCode: undefined }), 100);
+        }
     }));
 }
 exports.exec = exec;

src/compiler/transformers/es2018.ts

@@ -22,6 +22,7 @@ namespace ts {
         const previousOnSubstituteNode = context.onSubstituteNode;
         context.onSubstituteNode = onSubstituteNode;
 
+        let exportedVariableStatement = false;
         let enabledSubstitutions: ESNextSubstitutionFlags;
         let enclosingFunctionFlags: FunctionFlags;
         let enclosingSuperContainerFlags: NodeCheckFlags = 0;
@@ -40,6 +41,7 @@ namespace ts {
                 return node;
             }
 
+            exportedVariableStatement = false;
             const visited = visitEachChild(node, visitor, context);
             addEmitHelpers(visited, context.readEmitHelpers());
             return visited;
@@ -79,6 +81,8 @@ namespace ts {
                     return visitBinaryExpression(node as BinaryExpression, noDestructuringValue);
                 case SyntaxKind.CatchClause:
                     return visitCatchClause(node as CatchClause);
+                case SyntaxKind.VariableStatement:
+                    return visitVariableStatement(node as VariableStatement);
                 case SyntaxKind.VariableDeclaration:
                     return visitVariableDeclaration(node as VariableDeclaration);
                 case SyntaxKind.ForOfStatement:
@@ -321,19 +325,43 @@ namespace ts {
             return visitEachChild(node, visitor, context);
         }
 
+        function visitVariableStatement(node: VariableStatement): VisitResult<VariableStatement> {
+            if (hasModifier(node, ModifierFlags.Export)) {
+                const savedExportedVariableStatement = exportedVariableStatement;
+                exportedVariableStatement = true;
+                const visited = visitEachChild(node, visitor, context);
+                exportedVariableStatement = savedExportedVariableStatement;
+                return visited;
+            }
+            return visitEachChild(node, visitor, context);
+        }
+
         /**
          * Visits a VariableDeclaration node with a binding pattern.
          *
          * @param node A VariableDeclaration node.
          */
         function visitVariableDeclaration(node: VariableDeclaration): VisitResult<VariableDeclaration> {
+            if (exportedVariableStatement) {
+                const savedExportedVariableStatement = exportedVariableStatement;
+                exportedVariableStatement = false;
+                const visited = visitVariableDeclarationWorker(node, /*exportedVariableStatement*/ true);
+                exportedVariableStatement = savedExportedVariableStatement;
+                return visited;
+            }
+            return visitVariableDeclarationWorker(node, /*exportedVariableStatement*/ false);
+        }
+
+        function visitVariableDeclarationWorker(node: VariableDeclaration, exportedVariableStatement: boolean): VisitResult<VariableDeclaration> {
             // If we are here it is because the name contains a binding pattern with a rest somewhere in it.
             if (isBindingPattern(node.name) && node.name.transformFlags & TransformFlags.ContainsObjectRestOrSpread) {
                 return flattenDestructuringBinding(
                     node,
                     visitor,
                     context,
-                    FlattenLevel.ObjectRest
+                    FlattenLevel.ObjectRest,
+                    /*rval*/ undefined,
+                    exportedVariableStatement
                 );
             }
             return visitEachChild(node, visitor, context);

src/harness/harness.ts

@@ -726,6 +726,7 @@ namespace Harness {
             includeBuiltFile?: string;
             baselineFile?: string;
             libFiles?: string;
+            noTypesAndSymbols?: boolean;
         }
 
         // Additional options not already in ts.optionDeclarations
@@ -742,6 +743,7 @@ namespace Harness {
             { name: "currentDirectory", type: "string" },
             { name: "symlink", type: "string" },
             { name: "link", type: "string" },
+            { name: "noTypesAndSymbols", type: "boolean" },
             // Emitted js baseline will print full paths for every output file
             { name: "fullEmitPaths", type: "boolean" }
         ];

src/testRunner/compilerRunner.ts

@@ -58,16 +58,18 @@ class CompilerBaselineRunner extends RunnerBase {
     }
 
     public checkTestCodeOutput(fileName: string, test?: CompilerFileBasedTest) {
-        if (test && test.configurations) {
+        if (test && ts.some(test.configurations)) {
             test.configurations.forEach(configuration => {
                 describe(`${this.testSuiteName} tests for ${fileName}${configuration ? ` (${Harness.getFileBasedTestConfigurationDescription(configuration)})` : ``}`, () => {
                     this.runSuite(fileName, test, configuration);
                 });
             });
         }
-        describe(`${this.testSuiteName} tests for ${fileName}`, () => {
-            this.runSuite(fileName, test);
-        });
+        else {
+            describe(`${this.testSuiteName} tests for ${fileName}`, () => {
+                this.runSuite(fileName, test);
+            });
+        }
     }
 
     private runSuite(fileName: string, test?: CompilerFileBasedTest, configuration?: Harness.FileBasedTestConfiguration) {
@@ -112,6 +114,7 @@ class CompilerBaselineRunner extends RunnerBase {
 class CompilerTest {
     private fileName: string;
     private justName: string;
+    private configuredName: string;
     private lastUnit: Harness.TestCaseParser.TestUnitData;
     private harnessSettings: Harness.TestCaseParser.CompilerSettings;
     private hasNonDtsFiles: boolean;
@@ -126,6 +129,25 @@ class CompilerTest {
     constructor(fileName: string, testCaseContent?: Harness.TestCaseParser.TestCaseContent, configurationOverrides?: Harness.TestCaseParser.CompilerSettings) {
         this.fileName = fileName;
         this.justName = vpath.basename(fileName);
+        this.configuredName = this.justName;
+        if (configurationOverrides) {
+            let configuredName = "";
+            const keys = Object
+                .keys(configurationOverrides)
+                .map(k => k.toLowerCase())
+                .sort();
+            for (const key of keys) {
+                if (configuredName) {
+                    configuredName += ",";
+                }
+                configuredName += `${key}=${configurationOverrides[key].toLowerCase()}`;
+            }
+            if (configuredName) {
+                const extname = vpath.extname(this.justName);
+                const basename = vpath.basename(this.justName, extname, /*ignoreCase*/ true);
+                this.configuredName = `${basename}(${configuredName})${extname}`;
+            }
+        }
 
         const rootDir = fileName.indexOf("conformance") === -1 ? "tests/cases/compiler/" : ts.getDirectoryPath(fileName) + "/";
 
@@ -205,15 +227,15 @@ class CompilerTest {
     public verifyDiagnostics() {
         // check errors
         Harness.Compiler.doErrorBaseline(
-            this.justName,
+            this.configuredName,
             this.tsConfigFiles.concat(this.toBeCompiled, this.otherFiles),
             this.result.diagnostics,
             !!this.options.pretty);
     }
 
     public verifyModuleResolution() {
         if (this.options.traceResolution) {
-            Harness.Baseline.runBaseline(this.justName.replace(/\.tsx?$/, ".trace.json"),
+            Harness.Baseline.runBaseline(this.configuredName.replace(/\.tsx?$/, ".trace.json"),
                 JSON.stringify(this.result.traces.map(utils.sanitizeTraceResolutionLogEntry), undefined, 4));
         }
     }
@@ -225,14 +247,14 @@ class CompilerTest {
                 // Because of the noEmitOnError option no files are created. We need to return null because baselining isn't required.
                 ? null // tslint:disable-line no-null-keyword
                 : record;
-            Harness.Baseline.runBaseline(this.justName.replace(/\.tsx?$/, ".sourcemap.txt"), baseline);
+            Harness.Baseline.runBaseline(this.configuredName.replace(/\.tsx?$/, ".sourcemap.txt"), baseline);
         }
     }
 
     public verifyJavaScriptOutput() {
         if (this.hasNonDtsFiles) {
             Harness.Compiler.doJsEmitBaseline(
-                this.justName,
+                this.configuredName,
                 this.fileName,
                 this.options,
                 this.result,
@@ -245,7 +267,7 @@ class CompilerTest {
 
     public verifySourceMapOutput() {
         Harness.Compiler.doSourcemapBaseline(
-            this.justName,
+            this.configuredName,
             this.options,
             this.result,
             this.harnessSettings);
@@ -256,8 +278,15 @@ class CompilerTest {
             return;
         }
 
+        const noTypesAndSymbols =
+            this.harnessSettings.noTypesAndSymbols &&
+            this.harnessSettings.noTypesAndSymbols.toLowerCase() === "true";
+        if (noTypesAndSymbols) {
+            return;
+        }
+
         Harness.Compiler.doTypeAndSymbolBaseline(
-            this.justName,
+            this.configuredName,
             this.result.program!,
             this.toBeCompiled.concat(this.otherFiles).filter(file => !!this.result.program!.getSourceFile(file.unitName)),
             /*opts*/ undefined,

tests/baselines/reference/exportEmptyArrayBindingPattern(module=amd,target=es5).js

@@ -0,0 +1,10 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+define(["require", "exports"], function (require, exports) {
+    "use strict";
+    var _a;
+    Object.defineProperty(exports, "__esModule", { value: true });
+    exports._b = _a = [];
+});

tests/baselines/reference/exportEmptyArrayBindingPattern(module=amd,target=esnext).js

@@ -0,0 +1,10 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+define(["require", "exports"], function (require, exports) {
+    "use strict";
+    var _a;
+    Object.defineProperty(exports, "__esModule", { value: true });
+    _a = [];
+});

tests/baselines/reference/exportEmptyArrayBindingPattern(module=commonjs,target=es5).js

@@ -0,0 +1,8 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+"use strict";
+var _a;
+Object.defineProperty(exports, "__esModule", { value: true });
+exports._b = _a = [];

tests/baselines/reference/exportEmptyArrayBindingPattern(module=commonjs,target=esnext).js

@@ -0,0 +1,8 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+"use strict";
+var _a;
+Object.defineProperty(exports, "__esModule", { value: true });
+_a = [];

tests/baselines/reference/exportEmptyArrayBindingPattern(module=es2015,target=es5).js

@@ -0,0 +1,6 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+var _a;
+export var _b = _a = [];

tests/baselines/reference/exportEmptyArrayBindingPattern(module=es2015,target=esnext).js

@@ -0,0 +1,5 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+export const [] = [];

tests/baselines/reference/exportEmptyArrayBindingPattern(module=esnext,target=es5).js

@@ -0,0 +1,6 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+var _a;
+export var _b = _a = [];

tests/baselines/reference/exportEmptyArrayBindingPattern(module=esnext,target=esnext).js

@@ -0,0 +1,5 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+export const [] = [];

tests/baselines/reference/exportEmptyArrayBindingPattern(module=system,target=es5).js

@@ -0,0 +1,15 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+System.register([], function (exports_1, context_1) {
+    "use strict";
+    var _a, _b;
+    var __moduleName = context_1 && context_1.id;
+    return {
+        setters: [],
+        execute: function () {
+            exports_1("_b", _b = _a = []);
+        }
+    };
+});

tests/baselines/reference/exportEmptyArrayBindingPattern(module=system,target=esnext).js

@@ -0,0 +1,15 @@
+//// [exportEmptyArrayBindingPattern.ts]
+export const [] = [];
+
+//// [exportEmptyArrayBindingPattern.js]
+System.register([], function (exports_1, context_1) {
+    "use strict";
+    var _a;
+    var __moduleName = context_1 && context_1.id;
+    return {
+        setters: [],
+        execute: function () {
+            _a = [];
+        }
+    };
+});

tests/baselines/reference/exportEmptyObjectBindingPattern(module=amd,target=es5).js

@@ -0,0 +1,10 @@
+//// [exportEmptyObjectBindingPattern.ts]
+export const {} = {};
+
+//// [exportEmptyObjectBindingPattern.js]
+define(["require", "exports"], function (require, exports) {
+    "use strict";
+    var _a;
+    Object.defineProperty(exports, "__esModule", { value: true });
+    exports._b = _a = {};
+});

tests/baselines/reference/exportEmptyObjectBindingPattern(module=amd,target=esnext).js

@@ -0,0 +1,10 @@
+//// [exportEmptyObjectBindingPattern.ts]
+export const {} = {};
+
+//// [exportEmptyObjectBindingPattern.js]
+define(["require", "exports"], function (require, exports) {
+    "use strict";
+    var _a;
+    Object.defineProperty(exports, "__esModule", { value: true });
+    _a = {};
+});