From 8fcfc8685c99b0c2b79b205695d2e6635cd641f5 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 07ceaa135632d..6dd49bb46dab8 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 0c9f5765605a2..95df0a61731f1 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 f6ca2f45d3401..3d040f4592fdb 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 a2d25b7ced77bda983333b67cb4d34c290148f92 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"]); }); });