From cf540198e69551bfccdf823f1bec1669a3cc0019 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 31 Jan 2018 16:22:27 -0800 Subject: [PATCH 1/2] Always get diagnostics when emitting irrespective of whether its declaration only emit The diagnostics reporting and expression resolution caching is quite intermingled at present. Hence when we tried to get the declaration output without getting diagnostics, the resolution for functions return expression is cached but errors arent reported Symbols arent marked as referenced. So at later time when trying to get the diagnostics since the expression resolution is cached, it doesnt even go through all checks For now get diagnostics irrespective of declaration only output to avoid this issue. Fixes #21518 --- src/compiler/checker.ts | 6 ++---- src/compiler/program.ts | 2 +- src/compiler/types.ts | 2 +- src/harness/unittests/tscWatchMode.ts | 27 +++++++++++++++++++++++++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 16b2091733a74..a2b0d4b823e3f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -749,12 +749,10 @@ namespace ts { return _jsxNamespace; } - function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken, ignoreDiagnostics?: boolean) { + function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken) { // Ensure we have all the type information in place for this file so that all the // emitter questions of this resolver will return the right information. - if (!ignoreDiagnostics) { - getDiagnostics(sourceFile, cancellationToken); - } + getDiagnostics(sourceFile, cancellationToken); return emitResolver; } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 6ea50b30f6f75..e334c1ce80fb5 100755 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1179,7 +1179,7 @@ namespace ts { // This is because in the -out scenario all files need to be emitted, and therefore all // files need to be type checked. And the way to specify that all files need to be type // checked is to not pass the file to getEmitResolver. - const emitResolver = getDiagnosticsProducingTypeChecker().getEmitResolver((options.outFile || options.out) ? undefined : sourceFile, cancellationToken, emitOnlyDtsFiles); + const emitResolver = getDiagnosticsProducingTypeChecker().getEmitResolver((options.outFile || options.out) ? undefined : sourceFile, cancellationToken); performance.mark("beforeEmit"); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 481b91c9276e1..ce2c10a8dda86 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2892,7 +2892,7 @@ namespace ts { // Should not be called directly. Should only be accessed through the Program instance. /* @internal */ getDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): Diagnostic[]; /* @internal */ getGlobalDiagnostics(): Diagnostic[]; - /* @internal */ getEmitResolver(sourceFile?: SourceFile, cancellationToken?: CancellationToken, ignoreDiagnostics?: boolean): EmitResolver; + /* @internal */ getEmitResolver(sourceFile?: SourceFile, cancellationToken?: CancellationToken): EmitResolver; /* @internal */ getNodeCount(): number; /* @internal */ getIdentifierCount(): number; diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index a2b79b00dc76a..6915a6ad37d8b 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -1086,6 +1086,33 @@ namespace ts.tscWatch { // This should be 0 host.checkTimeoutQueueLengthAndRun(0); }); + + it("shouldnt report error about unused function incorrectly when file changes from global to module", () => { + const getFileContent = (asModule: boolean) => ` + function one() {} + ${asModule ? "export " : ""}function two() { + return function three() { + one(); + } + }`; + const file: FileOrFolder = { + path: "/a/b/file.ts", + content: getFileContent(/*asModule*/ false) + }; + const files = [file, libFile]; + const host = createWatchedSystem(files); + const watch = createWatchOfFilesAndCompilerOptions([file.path], host, { + noUnusedLocals: true + }); + checkProgramActualFiles(watch(), files.map(file => file.path)); + checkOutputErrors(host, [], ExpectedOutputErrorsPosition.AfterCompilationStarting); + + file.content = getFileContent(/*asModule*/ true); + host.reloadFS(files); + host.runQueuedTimeoutCallbacks(); + checkProgramActualFiles(watch(), files.map(file => file.path)); + checkOutputErrors(host, [], ExpectedOutputErrorsPosition.AfterFileChangeDetected); + }); }); describe("tsc-watch emit with outFile or out setting", () => { From 11214b9dcd572b674204371ddc4b321102774ea7 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 31 Jan 2018 17:15:54 -0800 Subject: [PATCH 2/2] Removing the test added for cancellation during affected list since thats not possible anymore as the affected files would anyways be semantically checked This is just part missed during revert of 0b79f4a --- src/harness/unittests/builder.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/harness/unittests/builder.ts b/src/harness/unittests/builder.ts index 8808a151c0494..3a275d39e868b 100644 --- a/src/harness/unittests/builder.ts +++ b/src/harness/unittests/builder.ts @@ -70,13 +70,6 @@ namespace ts { // Change e.ts and verify previously b.js as well as a.js get emitted again since previous change was consumed completely but not d.ts program = updateProgramFile(program, "/e.ts", "export function bar3() { }"); assertChanges(["/b.js", "/a.js", "/e.js"]); - - // Cancel in the middle of affected files list after b.js emit - program = updateProgramFile(program, "/b.ts", "export class b { foo2() { c + 1; } }"); - assertChanges(["/b.js", "/a.js"], 1); - // Change e.ts and verify previously b.js as well as a.js get emitted again since previous change was consumed completely but not d.ts - program = updateProgramFile(program, "/e.ts", "export function bar5() { }"); - assertChanges(["/b.js", "/a.js", "/e.js"]); }); });