Revert System changes and consume fs directly

amcasey · amcasey · commit b3861c8be1d4 · 2020-08-20T18:04:38.000-07:00
diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts
@@ -201,7 +201,8 @@ namespace ts {
             name: "generateTrace",
             type: "string",
             isFilePath: true,
-            paramType: Diagnostics.FILE_OR_DIRECTORY,
+            isCommandLineOnly: true,
+            paramType: Diagnostics.DIRECTORY,
             category: Diagnostics.Advanced_Options,
             description: Diagnostics.Generates_an_event_trace_and_a_list_of_types
         },
diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts
@@ -1047,13 +1047,11 @@ namespace ts {
         args: string[];
         newLine: string;
         useCaseSensitiveFileNames: boolean;
-        write(s: string, fd?: number): void;
+        write(s: string): void;
         writeOutputIsTTY?(): boolean;
         readFile(path: string, encoding?: string): string | undefined;
         getFileSize?(path: string): number;
         writeFile(path: string, data: string, writeByteOrderMark?: boolean): void;
-        openFile(path: string, mode: "w"): number | undefined;
-        closeFile(fd: number): void;
 
         /**
          * @pollingInterval - this parameter is used in polling-based watchers and ignored in watchers that
@@ -1185,33 +1183,12 @@ namespace ts {
                 args: process.argv.slice(2),
                 newLine: _os.EOL,
                 useCaseSensitiveFileNames,
-                write(s: string, fd?: number): void {
-                    if (fd) {
-                        _fs.writeSync(fd, s);
-                    }
-                    else {
-                        process.stdout.write(s);
-                    }
+                write(s: string): void {
+                    process.stdout.write(s);
                 },
                 writeOutputIsTTY() {
                     return process.stdout.isTTY;
                 },
-                openFile: (path, mode) => {
-                    try {
-                        return _fs.openSync(path, mode);
-                    }
-                    catch {
-                        return undefined;
-                    }
-                },
-                closeFile: (fd) => {
-                    try {
-                        _fs.closeSync(fd);
-                    }
-                    catch {
-                        // ignore
-                    }
-                },
                 readFile,
                 writeFile,
                 watchFile,
diff --git a/src/compiler/tracing.ts b/src/compiler/tracing.ts
@@ -1,27 +1,83 @@
 /*@internal*/
 /** Tracing events for the compiler. */
 namespace ts.tracing {
-    type WriteFn = (data: string) => void;
+    let fs: typeof import("fs") | false | undefined;
 
-    let write: WriteFn | undefined;
+    let traceCount = 0;
+    let traceFd: number | undefined;
 
-    /** Enables (and resets) tracing events for the compiler. */
-    export function startTracing(w: WriteFn) {
-        write = w;
-        write(`[\n`);
+    let legendPath: string | undefined;
+    const legend: TraceRecord[] = [];
+
+    /** Starts tracing for the given project (unless the `fs` module is unavailable). */
+    export function startTracing(configFilePath: string | undefined, traceDir: string, isBuildMode: boolean) {
+        Debug.assert(!traceFd, "Tracing already started");
+
+        if (fs === undefined) {
+            try {
+                fs = require("fs");
+            }
+            catch {
+                fs = false;
+            }
+        }
+
+        if (!fs) {
+            return;
+        }
+
+        if (legendPath === undefined) {
+            legendPath = combinePaths(traceDir, "legend.json");
+        }
+
+        // Note that writing will fail later on if it exists and is not a directory
+        if (!fs.existsSync(traceDir)) {
+            fs.mkdirSync(traceDir, { recursive: true });
+        }
+
+        const countPart = isBuildMode ? `.${++traceCount}` : ``;
+        const tracePath = combinePaths(traceDir, `trace${countPart}.json`);
+        const typesPath = combinePaths(traceDir, `types${countPart}.json`);
+
+        legend.push({
+            configFilePath,
+            tracePath,
+            typesPath,
+        });
+
+        traceFd = fs.openSync(tracePath, "w");
+        fs.writeSync(traceFd, `[\n`);
     }
 
-    /** Disables tracing events for the compiler. */
-    export function stopTracing() {
+    /** Stops tracing for the in-progress project and dumps the type catalog (unless the `fs` module is unavailable). */
+    export function stopTracing(typeCatalog: readonly Type[]) {
+        if (!traceFd) {
+            Debug.assert(!fs, "Tracing is not in progress");
+            return;
+        }
+
+        Debug.assert(fs);
+
         // This both indicates that the trace is untruncated and conveniently
         // ensures that the last array element won't have a trailing comma.
-        write?.(`{"pid":1,"tid":1,"ph":"i","ts":${1000 * timestamp()},"name":"done","s":"g"}\n`);
-        write?.(`]\n`);
-        write = undefined;
+        fs.writeSync(traceFd, `{"pid":1,"tid":1,"ph":"i","ts":${1000 * timestamp()},"name":"done","s":"g"}\n`);
+        fs.writeSync(traceFd, `]\n`);
+
+        fs.closeSync(traceFd);
+        traceFd = undefined;
+
+        if (typeCatalog) {
+            dumpTypes(typeCatalog);
+        }
+        else {
+            // We pre-computed this path for convenience, but clear it
+            // now that the file won't be created.
+            legend[legend.length - 1].typesPath = undefined;
+        }
     }
 
     export function isTracing() {
-        return !!write;
+        return !!traceFd;
     }
 
     export const enum Phase {
@@ -33,15 +89,25 @@ namespace ts.tracing {
     }
 
     export function begin(phase: Phase, name: string, args: object) {
+        if (!traceFd) {
+            return;
+        }
+        Debug.assert(fs);
+
         performance.mark("beginTracing");
-        write?.(`{"pid":1,"tid":1,"ph":"B","cat":"${phase}","ts":${1000 * timestamp()},"name":"${name}","args":{ "ts": ${JSON.stringify(args)} }},\n`);
+        fs.writeSync(traceFd, `{"pid":1,"tid":1,"ph":"B","cat":"${phase}","ts":${1000 * timestamp()},"name":"${name}","args":{ "ts": ${JSON.stringify(args)} }},\n`);
         performance.mark("endTracing");
         performance.measure("Tracing", "beginTracing", "endTracing");
     }
 
     export function end() {
+        if (!traceFd) {
+            return;
+        }
+        Debug.assert(fs);
+
         performance.mark("beginTracing");
-        write?.(`{"pid":1,"tid":1,"ph":"E","ts":${1000 * timestamp()}},\n`);
+        fs.writeSync(traceFd, `{"pid":1,"tid":1,"ph":"E","ts":${1000 * timestamp()}},\n`);
         performance.mark("endTracing");
         performance.measure("Tracing", "beginTracing", "endTracing");
     }
@@ -53,13 +119,18 @@ namespace ts.tracing {
         };
     }
 
-    export function dumpTypes(types: readonly Type[], write: WriteFn) {
+    function dumpTypes(types: readonly Type[]) {
+        Debug.assert(fs);
+
         performance.mark("beginDumpTypes");
 
-        const numTypes = types.length;
+        const typesPath = legend[legend.length - 1].typesPath!;
+        const typesFd = fs.openSync(typesPath, "w");
+
+        // Cleverness: no line break here so that the type ID will match the line number
+        fs.writeSync(typesFd, "[");
 
-        // Cleverness: no line break hear so that the type ID will match the line number
-        write("[");
+        const numTypes = types.length;
         for (let i = 0; i < numTypes; i++) {
             const type = types[i];
             const objectFlags = (type as any).objectFlags;
@@ -127,14 +198,32 @@ namespace ts.tracing {
                 display,
             };
 
-            write(JSON.stringify(descriptor));
+            fs.writeSync(typesFd, JSON.stringify(descriptor));
             if (i < numTypes - 1) {
-                write(",\n");
+                fs.writeSync(typesFd, ",\n");
             }
         }
-        write("]\n");
+
+        fs.writeSync(typesFd, "]\n");
+
+        fs.closeSync(typesFd);
 
         performance.mark("endDumpTypes");
         performance.measure("Dump types", "beginDumpTypes", "endDumpTypes");
     }
+
+    export function dumpLegend() {
+        if (!legendPath) {
+            return;
+        }
+        Debug.assert(fs);
+
+        fs.writeFileSync(legendPath, JSON.stringify(legend));
+    }
+
+    interface TraceRecord {
+        configFilePath?: string;
+        tracePath: string;
+        typesPath?: string;
+    }
 }
diff --git a/src/executeCommandLine/executeCommandLine.ts b/src/executeCommandLine/executeCommandLine.ts
@@ -456,6 +456,7 @@ namespace ts {
         updateSolutionBuilderHost(sys, cb, buildHost);
         const builder = createSolutionBuilder(buildHost, projects, buildOptions);
         const exitStatus = buildOptions.clean ? builder.clean() : builder.build();
+        tracing.dumpLegend();
         return sys.exit(exitStatus);
     }
 
@@ -476,7 +477,7 @@ namespace ts {
         const currentDirectory = host.getCurrentDirectory();
         const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames());
         changeCompilerHostLikeToUseCache(host, fileName => toPath(fileName, currentDirectory, getCanonicalFileName));
-        enableStatisticsAndTracing(sys, options);
+        enableStatisticsAndTracing(sys, options, /*isBuildMode*/ false);
 
         const programOptions: CreateProgramOptions = {
             rootNames: fileNames,
@@ -504,7 +505,7 @@ namespace ts {
         config: ParsedCommandLine
     ) {
         const { options, fileNames, projectReferences } = config;
-        enableStatisticsAndTracing(sys, options);
+        enableStatisticsAndTracing(sys, options, /*isBuildMode*/ false);
         const host = createIncrementalCompilerHost(options, sys);
         const exitStatus = ts.performIncrementalCompilation({
             host,
@@ -541,7 +542,7 @@ namespace ts {
         host.createProgram = (rootNames, options, host, oldProgram, configFileParsingDiagnostics, projectReferences) => {
             Debug.assert(rootNames !== undefined || (options === undefined && !!oldProgram));
             if (options !== undefined) {
-                enableStatisticsAndTracing(sys, options);
+                enableStatisticsAndTracing(sys, options, /*isBuildMode*/ true);
             }
             return compileUsingBuilder(rootNames, options, host, oldProgram, configFileParsingDiagnostics, projectReferences);
         };
@@ -610,41 +611,25 @@ namespace ts {
         return system === sys && (compilerOptions.diagnostics || compilerOptions.extendedDiagnostics);
     }
 
-    let traceCount = 0;
-    let tracingFd: number | undefined;
+    function canTrace(system: System, compilerOptions: CompilerOptions) {
+        return system === sys && compilerOptions.generateTrace;
+    }
 
-    function enableStatisticsAndTracing(system: System, compilerOptions: CompilerOptions) {
+    function enableStatisticsAndTracing(system: System, compilerOptions: CompilerOptions, isBuildMode: boolean) {
         if (canReportDiagnostics(system, compilerOptions)) {
             performance.enable();
         }
 
-        Debug.assert(!tracingFd, "Tracing already started");
-        if (system === sys) {
-            const tracePath = compilerOptions.generateTrace;
-            if (tracePath) {
-                const extension = getAnyExtensionFromPath(tracePath);
-                tracingFd = sys.openFile(changeAnyExtension(tracePath, `${++traceCount}${extension}`), "w");
-                if (tracingFd) {
-                    tracing.startTracing(event => sys.write(event, tracingFd));
-                }
-            }
+        if (canTrace(system, compilerOptions)) {
+            tracing.startTracing(compilerOptions.configFilePath, compilerOptions.generateTrace!, isBuildMode);
         }
     }
 
     function reportStatistics(sys: System, program: Program) {
         const compilerOptions = program.getCompilerOptions();
 
-        if (tracingFd) {
-            tracing.stopTracing();
-            sys.closeFile(tracingFd);
-            tracingFd = undefined;
-
-            const typesPath = changeAnyExtension(compilerOptions.generateTrace!, `${traceCount}.types.json`);
-            const typesFd = sys.openFile(typesPath, "w");
-            if (typesFd) {
-                tracing.dumpTypes(program.getTypeCatalog(), type => sys.write(type, typesFd));
-                sys.closeFile(typesFd);
-            }
+        if (canTrace(sys, compilerOptions)) {
+            tracing.stopTracing(program.getTypeCatalog());
         }
 
         let statistics: Statistic[];
diff --git a/src/harness/fakesHosts.ts b/src/harness/fakesHosts.ts
@@ -37,8 +37,7 @@ namespace fakes {
             return true;
         }
 
-        public write(message: string, fd?: number) {
-            assert.isUndefined(fd);
+        public write(message: string) {
             this.output.push(message);
         }
 
@@ -61,15 +60,6 @@ namespace fakes {
             this.vfs.unlinkSync(path);
         }
 
-        public openFile(_path: string, _mode: "w"): number | undefined {
-            assert.fail("NYI");
-            return undefined;
-        }
-
-        public closeFile(_fd: number): void{
-            assert.fail("NYI");
-        }
-
         public fileExists(path: string) {
             const stats = this._getStats(path);
             return stats ? stats.isFile() : false;
diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts
@@ -724,8 +724,7 @@ namespace Harness.LanguageService {
 
         onMessage = ts.noop;
         writeMessage = ts.noop; // overridden
-        write(message: string, fd?: number): void {
-            assert.isUndefined(fd);
+        write(message: string): void {
             this.writeMessage(message);
         }
 
@@ -745,9 +744,6 @@ namespace Harness.LanguageService {
 
         writeFile = ts.noop;
 
-        openFile = ts.returnUndefined;
-        closeFile = ts.noop;
-
         resolvePath(path: string): string {
             return path;
         }
diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts
@@ -1011,21 +1011,11 @@ interface Array<T> { length: number; [n: number]: T; }`
             }
         }
 
-        openFile(_path: string, _mode: "w"): number | undefined {
-            assert.fail("NYI");
-            return undefined;
-        }
-
-        closeFile(_fd: number): void {
-            assert.fail("NYI");
-        }
-
         appendFile(path: string, content: string, options?: Partial<ReloadWatchInvokeOptions>): void {
             this.modifyFile(path, this.readFile(path) + content, options);
         }
 
-        write(message: string, fd?: number) {
-            assert.isUndefined(fd);
+        write(message: string) {
             this.output.push(message);
         }
 
diff --git a/src/testRunner/unittests/tsserver/session.ts b/src/testRunner/unittests/tsserver/session.ts
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts

Original file line number	Diff line number	Diff line change
@@ -1011,21 +1011,11 @@ interface Array<T> { length: number; [n: number]: T; }`
`1011`	`1011`	`}`
`1012`	`1012`	`}`
`1013`	`1013`
`1014`		`- openFile(_path: string, _mode: "w"): number \| undefined {`
`1015`		`- assert.fail("NYI");`
`1016`		`- return undefined;`
`1017`		`- }`
`1018`		`-`
`1019`		`- closeFile(_fd: number): void {`
`1020`		`- assert.fail("NYI");`
`1021`		`- }`
`1022`		`-`
`1023`	`1014`	`appendFile(path: string, content: string, options?: Partial<ReloadWatchInvokeOptions>): void {`
`1024`	`1015`	`this.modifyFile(path, this.readFile(path) + content, options);`
`1025`	`1016`	`}`
`1026`	`1017`
`1027`		`- write(message: string, fd?: number) {`
`1028`		`- assert.isUndefined(fd);`
	`1018`	`+ write(message: string) {`
`1029`	`1019`	`this.output.push(message);`
`1030`	`1020`	`}`
`1031`	`1021`