Skip to content

Commit e3f4979

Browse files
rbucktonrbuckton
authored
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
1 parent 3b54ffc commit e3f4979

38 files changed

Lines changed: 374 additions & 28 deletions

File tree

‎Gulpfile.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,10 +436,10 @@ task("runtests-parallel").flags = {
436436
" --shardId": "1-based ID of this shard (default: 1)",
437437
};
438438

439-
task("diff", () => exec(getDiffTool(), [refBaseline, localBaseline], { ignoreExitCode: true }));
439+
task("diff", () => exec(getDiffTool(), [refBaseline, localBaseline], { ignoreExitCode: true, waitForExit: false }));
440440
task("diff").description = "Diffs the compiler baselines using the diff tool specified by the 'DIFF' environment variable";
441441

442-
task("diff-rwc", () => exec(getDiffTool(), [refRwcBaseline, localRwcBaseline], { ignoreExitCode: true }));
442+
task("diff-rwc", () => exec(getDiffTool(), [refRwcBaseline, localRwcBaseline], { ignoreExitCode: true, waitForExit: false }));
443443
task("diff-rwc").description = "Diffs the RWC baselines using the diff tool specified by the 'DIFF' environment variable";
444444

445445
/**

‎scripts/build/utils.js‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,37 +25,45 @@ const isWindows = /^win/.test(process.platform);
2525
* @property {boolean} [ignoreExitCode]
2626
* @property {import("prex").CancellationToken} [cancelToken]
2727
* @property {boolean} [hidePrompt]
28+
* @property {boolean} [waitForExit=true]
2829
*/
2930
function exec(cmd, args, options = {}) {
3031
return /**@type {Promise<{exitCode: number}>}*/(new Promise((resolve, reject) => {
31-
const { ignoreExitCode, cancelToken = CancellationToken.none } = options;
32+
const { ignoreExitCode, cancelToken = CancellationToken.none, waitForExit = true } = options;
3233
cancelToken.throwIfCancellationRequested();
3334

3435
// TODO (weswig): Update child_process types to add windowsVerbatimArguments to the type definition
3536
const subshellFlag = isWindows ? "/c" : "-c";
3637
const command = isWindows ? [possiblyQuote(cmd), ...args] : [`${cmd} ${args.join(" ")}`];
3738

3839
if (!options.hidePrompt) log(`> ${chalk.green(cmd)} ${args.join(" ")}`);
39-
const proc = spawn(isWindows ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: "inherit", windowsVerbatimArguments: true });
40+
const proc = spawn(isWindows ? "cmd" : "/bin/sh", [subshellFlag, ...command], { stdio: waitForExit ? "inherit" : "ignore", windowsVerbatimArguments: true });
4041
const registration = cancelToken.register(() => {
4142
log(`${chalk.red("killing")} '${chalk.green(cmd)} ${args.join(" ")}'...`);
4243
proc.kill("SIGINT");
4344
proc.kill("SIGTERM");
4445
reject(new CancelError());
4546
});
46-
proc.on("exit", exitCode => {
47-
registration.unregister();
48-
if (exitCode === 0 || ignoreExitCode) {
49-
resolve({ exitCode });
50-
}
51-
else {
52-
reject(new Error(`Process exited with code: ${exitCode}`));
53-
}
54-
});
55-
proc.on("error", error => {
56-
registration.unregister();
57-
reject(error);
58-
});
47+
if (waitForExit) {
48+
proc.on("exit", exitCode => {
49+
registration.unregister();
50+
if (exitCode === 0 || ignoreExitCode) {
51+
resolve({ exitCode });
52+
}
53+
else {
54+
reject(new Error(`Process exited with code: ${exitCode}`));
55+
}
56+
});
57+
proc.on("error", error => {
58+
registration.unregister();
59+
reject(error);
60+
});
61+
}
62+
else {
63+
proc.unref();
64+
// wait a short period in order to allow the process to start successfully before Node exits.
65+
setTimeout(() => resolve({ exitCode: undefined }), 100);
66+
}
5967
}));
6068
}
6169
exports.exec = exec;

‎src/compiler/transformers/es2018.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ namespace ts {
2222
const previousOnSubstituteNode = context.onSubstituteNode;
2323
context.onSubstituteNode = onSubstituteNode;
2424

25+
let exportedVariableStatement = false;
2526
let enabledSubstitutions: ESNextSubstitutionFlags;
2627
let enclosingFunctionFlags: FunctionFlags;
2728
let enclosingSuperContainerFlags: NodeCheckFlags = 0;
@@ -40,6 +41,7 @@ namespace ts {
4041
return node;
4142
}
4243

44+
exportedVariableStatement = false;
4345
const visited = visitEachChild(node, visitor, context);
4446
addEmitHelpers(visited, context.readEmitHelpers());
4547
return visited;
@@ -79,6 +81,8 @@ namespace ts {
7981
return visitBinaryExpression(node as BinaryExpression, noDestructuringValue);
8082
case SyntaxKind.CatchClause:
8183
return visitCatchClause(node as CatchClause);
84+
case SyntaxKind.VariableStatement:
85+
return visitVariableStatement(node as VariableStatement);
8286
case SyntaxKind.VariableDeclaration:
8387
return visitVariableDeclaration(node as VariableDeclaration);
8488
case SyntaxKind.ForOfStatement:
@@ -321,19 +325,43 @@ namespace ts {
321325
return visitEachChild(node, visitor, context);
322326
}
323327

328+
function visitVariableStatement(node: VariableStatement): VisitResult<VariableStatement> {
329+
if (hasModifier(node, ModifierFlags.Export)) {
330+
const savedExportedVariableStatement = exportedVariableStatement;
331+
exportedVariableStatement = true;
332+
const visited = visitEachChild(node, visitor, context);
333+
exportedVariableStatement = savedExportedVariableStatement;
334+
return visited;
335+
}
336+
return visitEachChild(node, visitor, context);
337+
}
338+
324339
/**
325340
* Visits a VariableDeclaration node with a binding pattern.
326341
*
327342
* @param node A VariableDeclaration node.
328343
*/
329344
function visitVariableDeclaration(node: VariableDeclaration): VisitResult<VariableDeclaration> {
345+
if (exportedVariableStatement) {
346+
const savedExportedVariableStatement = exportedVariableStatement;
347+
exportedVariableStatement = false;
348+
const visited = visitVariableDeclarationWorker(node, /*exportedVariableStatement*/ true);
349+
exportedVariableStatement = savedExportedVariableStatement;
350+
return visited;
351+
}
352+
return visitVariableDeclarationWorker(node, /*exportedVariableStatement*/ false);
353+
}
354+
355+
function visitVariableDeclarationWorker(node: VariableDeclaration, exportedVariableStatement: boolean): VisitResult<VariableDeclaration> {
330356
// If we are here it is because the name contains a binding pattern with a rest somewhere in it.
331357
if (isBindingPattern(node.name) && node.name.transformFlags & TransformFlags.ContainsObjectRestOrSpread) {
332358
return flattenDestructuringBinding(
333359
node,
334360
visitor,
335361
context,
336-
FlattenLevel.ObjectRest
362+
FlattenLevel.ObjectRest,
363+
/*rval*/ undefined,
364+
exportedVariableStatement
337365
);
338366
}
339367
return visitEachChild(node, visitor, context);

‎src/harness/harness.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,7 @@ namespace Harness {
726726
includeBuiltFile?: string;
727727
baselineFile?: string;
728728
libFiles?: string;
729+
noTypesAndSymbols?: boolean;
729730
}
730731

731732
// Additional options not already in ts.optionDeclarations
@@ -742,6 +743,7 @@ namespace Harness {
742743
{ name: "currentDirectory", type: "string" },
743744
{ name: "symlink", type: "string" },
744745
{ name: "link", type: "string" },
746+
{ name: "noTypesAndSymbols", type: "boolean" },
745747
// Emitted js baseline will print full paths for every output file
746748
{ name: "fullEmitPaths", type: "boolean" }
747749
];

‎src/testRunner/compilerRunner.ts‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,18 @@ class CompilerBaselineRunner extends RunnerBase {
5858
}
5959

6060
public checkTestCodeOutput(fileName: string, test?: CompilerFileBasedTest) {
61-
if (test && test.configurations) {
61+
if (test && ts.some(test.configurations)) {
6262
test.configurations.forEach(configuration => {
6363
describe(`${this.testSuiteName} tests for ${fileName}${configuration ? ` (${Harness.getFileBasedTestConfigurationDescription(configuration)})` : ``}`, () => {
6464
this.runSuite(fileName, test, configuration);
6565
});
6666
});
6767
}
68-
describe(`${this.testSuiteName} tests for ${fileName}`, () => {
69-
this.runSuite(fileName, test);
70-
});
68+
else {
69+
describe(`${this.testSuiteName} tests for ${fileName}`, () => {
70+
this.runSuite(fileName, test);
71+
});
72+
}
7173
}
7274

7375
private runSuite(fileName: string, test?: CompilerFileBasedTest, configuration?: Harness.FileBasedTestConfiguration) {
@@ -112,6 +114,7 @@ class CompilerBaselineRunner extends RunnerBase {
112114
class CompilerTest {
113115
private fileName: string;
114116
private justName: string;
117+
private configuredName: string;
115118
private lastUnit: Harness.TestCaseParser.TestUnitData;
116119
private harnessSettings: Harness.TestCaseParser.CompilerSettings;
117120
private hasNonDtsFiles: boolean;
@@ -126,6 +129,25 @@ class CompilerTest {
126129
constructor(fileName: string, testCaseContent?: Harness.TestCaseParser.TestCaseContent, configurationOverrides?: Harness.TestCaseParser.CompilerSettings) {
127130
this.fileName = fileName;
128131
this.justName = vpath.basename(fileName);
132+
this.configuredName = this.justName;
133+
if (configurationOverrides) {
134+
let configuredName = "";
135+
const keys = Object
136+
.keys(configurationOverrides)
137+
.map(k => k.toLowerCase())
138+
.sort();
139+
for (const key of keys) {
140+
if (configuredName) {
141+
configuredName += ",";
142+
}
143+
configuredName += `${key}=${configurationOverrides[key].toLowerCase()}`;
144+
}
145+
if (configuredName) {
146+
const extname = vpath.extname(this.justName);
147+
const basename = vpath.basename(this.justName, extname, /*ignoreCase*/ true);
148+
this.configuredName = `${basename}(${configuredName})${extname}`;
149+
}
150+
}
129151

130152
const rootDir = fileName.indexOf("conformance") === -1 ? "tests/cases/compiler/" : ts.getDirectoryPath(fileName) + "/";
131153

@@ -205,15 +227,15 @@ class CompilerTest {
205227
public verifyDiagnostics() {
206228
// check errors
207229
Harness.Compiler.doErrorBaseline(
208-
this.justName,
230+
this.configuredName,
209231
this.tsConfigFiles.concat(this.toBeCompiled, this.otherFiles),
210232
this.result.diagnostics,
211233
!!this.options.pretty);
212234
}
213235

214236
public verifyModuleResolution() {
215237
if (this.options.traceResolution) {
216-
Harness.Baseline.runBaseline(this.justName.replace(/\.tsx?$/, ".trace.json"),
238+
Harness.Baseline.runBaseline(this.configuredName.replace(/\.tsx?$/, ".trace.json"),
217239
JSON.stringify(this.result.traces.map(utils.sanitizeTraceResolutionLogEntry), undefined, 4));
218240
}
219241
}
@@ -225,14 +247,14 @@ class CompilerTest {
225247
// Because of the noEmitOnError option no files are created. We need to return null because baselining isn't required.
226248
? null // tslint:disable-line no-null-keyword
227249
: record;
228-
Harness.Baseline.runBaseline(this.justName.replace(/\.tsx?$/, ".sourcemap.txt"), baseline);
250+
Harness.Baseline.runBaseline(this.configuredName.replace(/\.tsx?$/, ".sourcemap.txt"), baseline);
229251
}
230252
}
231253

232254
public verifyJavaScriptOutput() {
233255
if (this.hasNonDtsFiles) {
234256
Harness.Compiler.doJsEmitBaseline(
235-
this.justName,
257+
this.configuredName,
236258
this.fileName,
237259
this.options,
238260
this.result,
@@ -245,7 +267,7 @@ class CompilerTest {
245267

246268
public verifySourceMapOutput() {
247269
Harness.Compiler.doSourcemapBaseline(
248-
this.justName,
270+
this.configuredName,
249271
this.options,
250272
this.result,
251273
this.harnessSettings);
@@ -256,8 +278,15 @@ class CompilerTest {
256278
return;
257279
}
258280

281+
const noTypesAndSymbols =
282+
this.harnessSettings.noTypesAndSymbols &&
283+
this.harnessSettings.noTypesAndSymbols.toLowerCase() === "true";
284+
if (noTypesAndSymbols) {
285+
return;
286+
}
287+
259288
Harness.Compiler.doTypeAndSymbolBaseline(
260-
this.justName,
289+
this.configuredName,
261290
this.result.program!,
262291
this.toBeCompiled.concat(this.otherFiles).filter(file => !!this.result.program!.getSourceFile(file.unitName)),
263292
/*opts*/ undefined,

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//// [exportEmptyArrayBindingPattern.ts]
2+
export const [] = [];
3+
4+
//// [exportEmptyArrayBindingPattern.js]
5+
define(["require", "exports"], function (require, exports) {
6+
"use strict";
7+
var _a;
8+
Object.defineProperty(exports, "__esModule", { value: true });
9+
exports._b = _a = [];
10+
});

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//// [exportEmptyArrayBindingPattern.ts]
2+
export const [] = [];
3+
4+
//// [exportEmptyArrayBindingPattern.js]
5+
define(["require", "exports"], function (require, exports) {
6+
"use strict";
7+
var _a;
8+
Object.defineProperty(exports, "__esModule", { value: true });
9+
_a = [];
10+
});

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//// [exportEmptyArrayBindingPattern.ts]
2+
export const [] = [];
3+
4+
//// [exportEmptyArrayBindingPattern.js]
5+
"use strict";
6+
var _a;
7+
Object.defineProperty(exports, "__esModule", { value: true });
8+
exports._b = _a = [];

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//// [exportEmptyArrayBindingPattern.ts]
2+
export const [] = [];
3+
4+
//// [exportEmptyArrayBindingPattern.js]
5+
"use strict";
6+
var _a;
7+
Object.defineProperty(exports, "__esModule", { value: true });
8+
_a = [];

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
//// [exportEmptyArrayBindingPattern.ts]
2+
export const [] = [];
3+
4+
//// [exportEmptyArrayBindingPattern.js]
5+
var _a;
6+
export var _b = _a = [];

0 commit comments

Comments
 (0)