From a27b29f025c1cd37b3c971f851dcc53b5fbdf70f Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 9 Nov 2018 12:23:54 -0800 Subject: [PATCH 1/5] Remove lib file errors when all files are to be emitted. Fixes #26389 --- src/compiler/builder.ts | 26 ++++- src/testRunner/unittests/tscWatchMode.ts | 119 +++++++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/compiler/builder.ts b/src/compiler/builder.ts index 2de77fa5545..b5f0339c054 100644 --- a/src/compiler/builder.ts +++ b/src/compiler/builder.ts @@ -38,6 +38,10 @@ namespace ts { * Already seen affected files */ seenAffectedFiles: Map | undefined; + /** + * program corresponding to this state + */ + cleanedDiagnosticsOfLibFiles?: boolean; /** * True if the semantic diagnostics were copied from the old state */ @@ -83,6 +87,8 @@ namespace ts { // Update changed files and copy semantic diagnostics if we can const referencedMap = state.referencedMap; const oldReferencedMap = useOldState ? oldState!.referencedMap : undefined; + const copyDeclarationFileDiagnostics = canCopySemanticDiagnostics && !compilerOptions.skipLibCheck === !oldState!.program.getCompilerOptions().skipLibCheck; + const copyLibFileDiagnostics = copyDeclarationFileDiagnostics && !compilerOptions.skipDefaultLibCheck === !oldState!.program.getCompilerOptions().skipDefaultLibCheck; state.fileInfos.forEach((info, sourceFilePath) => { let oldInfo: Readonly | undefined; let newReferences: BuilderState.ReferencedSet | undefined; @@ -101,6 +107,11 @@ namespace ts { state.changedFilesSet.set(sourceFilePath, true); } else if (canCopySemanticDiagnostics) { + const sourceFile = state.program.getSourceFileByPath(sourceFilePath as Path)!; + + if (sourceFile.isDeclarationFile && !copyDeclarationFileDiagnostics) { return; } + if (sourceFile.hasNoDefaultLib && !copyLibFileDiagnostics) { return; } + // Unchanged file copy diagnostics const diagnostics = oldState!.semanticDiagnosticsPerFile!.get(sourceFilePath); if (diagnostics) { @@ -193,6 +204,19 @@ namespace ts { return; } + // Clean lib file diagnostics if its all files excluding default files to emit + if (state.allFilesExcludingDefaultLibraryFile === state.affectedFiles && !state.cleanedDiagnosticsOfLibFiles) { + state.cleanedDiagnosticsOfLibFiles = true; + const options = state.program.getCompilerOptions(); + if (forEach(state.program.getSourceFiles(), f => + !contains(state.allFilesExcludingDefaultLibraryFile, f) && + !skipTypeChecking(f, options) && + removeSemanticDiagnosticsOf(state, f.path) + )) { + return; + } + } + // If there was change in signature for the changed file, // then delete the semantic diagnostics for files that are affected by using exports of this module @@ -268,7 +292,7 @@ namespace ts { */ function removeSemanticDiagnosticsOf(state: BuilderProgramState, path: Path) { if (!state.semanticDiagnosticsFromOldState) { - return false; + return true; } state.semanticDiagnosticsFromOldState.delete(path); state.semanticDiagnosticsPerFile!.delete(path); diff --git a/src/testRunner/unittests/tscWatchMode.ts b/src/testRunner/unittests/tscWatchMode.ts index f77b4bce0eb..1e65e7b9e99 100644 --- a/src/testRunner/unittests/tscWatchMode.ts +++ b/src/testRunner/unittests/tscWatchMode.ts @@ -1564,6 +1564,125 @@ export class Data2 { verifyTransitiveExports([libFile, app, lib2Public, lib2Data, lib2Data2, lib1Public, lib1ToolsPublic, lib1ToolsInterface]); }); }); + + describe("updates errors in lib file when non module file changes", () => { + const currentDirectory = "/user/username/projects/myproject"; + const field = "fullscreen"; + const aFile: File = { + path: `${currentDirectory}/a.ts`, + content: `interface Document { + ${field}: boolean; +}` + }; + const libFileWithDocument: File = { + path: libFile.path, + content: `${libFile.content} +interface Document { + readonly ${field}: boolean; +}` + }; + + function getDiagnostic(program: Program, file: File) { + return getDiagnosticOfFileFromProgram(program, file.path, file.content.indexOf(field), field.length, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, field); + } + + const files = [aFile, libFileWithDocument]; + + function verifyLibErrors(options: CompilerOptions) { + const host = createWatchedSystem(files, { currentDirectory }); + const watch = createWatchOfFilesAndCompilerOptions([aFile.path], host, options); + checkProgramActualFiles(watch(), [aFile.path, libFile.path]); + checkOutputErrorsInitial(host, getErrors()); + + host.writeFile(aFile.path, "var x = 10;"); + host.runQueuedTimeoutCallbacks(); + checkProgramActualFiles(watch(), [aFile.path, libFile.path]); + checkOutputErrorsIncremental(host, emptyArray); + + host.writeFile(aFile.path, aFile.content); + host.runQueuedTimeoutCallbacks(); + checkProgramActualFiles(watch(), [aFile.path, libFile.path]); + checkOutputErrorsIncremental(host, getErrors()); + + function getErrors() { + return [ + ...(options.skipLibCheck || options.skipDefaultLibCheck ? [] : [getDiagnostic(watch(), libFileWithDocument)]), + getDiagnostic(watch(), aFile) + ]; + } + } + + it("with default options", () => { + verifyLibErrors({}); + }); + it("with skipLibCheck", () => { + verifyLibErrors({ skipLibCheck: true }); + }); + it("with skipDefaultLibCheck", () => { + verifyLibErrors({ skipDefaultLibCheck: true }); + }); + }); + + it("when skipLibCheck and skipDefaultLibCheck changes", () => { + const currentDirectory = "/user/username/projects/myproject"; + const field = "fullscreen"; + const aFile: File = { + path: `${currentDirectory}/a.ts`, + content: `interface Document { + ${field}: boolean; +}` + }; + const bFile: File = { + path: `${currentDirectory}/b.d.ts`, + content: `interface Document { + ${field}: boolean; +}` + }; + const libFileWithDocument: File = { + path: libFile.path, + content: `${libFile.content} +interface Document { + readonly ${field}: boolean; +}` + }; + const configFile: File = { + path: `${currentDirectory}/tsconfig.json`, + content: "{}" + }; + + const files = [aFile, bFile, configFile, libFileWithDocument]; + + const host = createWatchedSystem(files, { currentDirectory }); + const watch = createWatchOfConfigFile("tsconfig.json", host); + verifyProgramFiles(); + checkOutputErrorsInitial(host, [ + getDiagnostic(libFileWithDocument), + getDiagnostic(aFile), + getDiagnostic(bFile) + ]); + + verifyConfigChange({ skipLibCheck: true }, [aFile]); + verifyConfigChange({ skipDefaultLibCheck: true }, [aFile, bFile]); + verifyConfigChange({}, [libFileWithDocument, aFile, bFile]); + verifyConfigChange({ skipDefaultLibCheck: true }, [aFile, bFile]); + verifyConfigChange({ skipLibCheck: true }, [aFile]); + verifyConfigChange({}, [libFileWithDocument, aFile, bFile]); + + function verifyConfigChange(compilerOptions: CompilerOptions, errorInFiles: ReadonlyArray) { + host.writeFile(configFile.path, JSON.stringify({ compilerOptions })); + host.runQueuedTimeoutCallbacks(); + verifyProgramFiles(); + checkOutputErrorsIncremental(host, errorInFiles.map(getDiagnostic)); + } + + function getDiagnostic(file: File) { + return getDiagnosticOfFileFromProgram(watch(), file.path, file.content.indexOf(field), field.length, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, field); + } + + function verifyProgramFiles() { + checkProgramActualFiles(watch(), [aFile.path, bFile.path, libFile.path]); + } + }); }); describe("tsc-watch emit with outFile or out setting", () => { From fdafbd6e958cb06268122a4592a10223069b0a32 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 9 Nov 2018 14:43:32 -0800 Subject: [PATCH 2/5] Report identical modifiers needed error when checking the file instead of reporting it as part of another file's type check --- src/compiler/checker.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4f04c6f8e82..b78490b4c94 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -25110,6 +25110,11 @@ namespace ts { checkParameterInitializer(node); } } + if (symbol.declarations.length > 1) { + if (some(symbol.declarations, d => d !== node && !areDeclarationFlagsIdentical(d, node))) { + error(node.name, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, declarationNameToString(node.name)); + } + } } else { // Node is a secondary declaration, check that type is identical to primary declaration and check that @@ -25125,7 +25130,6 @@ namespace ts { checkTypeAssignableToAndOptionallyElaborate(checkExpressionCached(node.initializer), declarationType, node, node.initializer, /*headMessage*/ undefined); } if (!areDeclarationFlagsIdentical(node, symbol.valueDeclaration)) { - error(getNameOfDeclaration(symbol.valueDeclaration), Diagnostics.All_declarations_of_0_must_have_identical_modifiers, declarationNameToString(node.name)); error(node.name, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, declarationNameToString(node.name)); } } From 43c447867bf4dc657f8dad6dde8990dffd9b479d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 9 Nov 2018 16:18:47 -0800 Subject: [PATCH 3/5] PR feedback --- src/compiler/builder.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/compiler/builder.ts b/src/compiler/builder.ts index b5f0339c054..b2ab858cbd4 100644 --- a/src/compiler/builder.ts +++ b/src/compiler/builder.ts @@ -39,7 +39,7 @@ namespace ts { */ seenAffectedFiles: Map | undefined; /** - * program corresponding to this state + * whether this program has cleaned semantic diagnostics cache for lib files */ cleanedDiagnosticsOfLibFiles?: boolean; /** @@ -68,9 +68,11 @@ namespace ts { state.semanticDiagnosticsPerFile = createMap>(); } state.changedFilesSet = createMap(); + const useOldState = BuilderState.canReuseOldState(state.referencedMap, oldState); + const oldCompilerOptions = useOldState ? oldState!.program.getCompilerOptions() : undefined; const canCopySemanticDiagnostics = useOldState && oldState!.semanticDiagnosticsPerFile && !!state.semanticDiagnosticsPerFile && - !compilerOptionsAffectSemanticDiagnostics(compilerOptions, oldState!.program.getCompilerOptions()); + !compilerOptionsAffectSemanticDiagnostics(compilerOptions, oldCompilerOptions!); if (useOldState) { // Verify the sanity of old state if (!oldState!.currentChangedFilePath) { @@ -87,8 +89,8 @@ namespace ts { // Update changed files and copy semantic diagnostics if we can const referencedMap = state.referencedMap; const oldReferencedMap = useOldState ? oldState!.referencedMap : undefined; - const copyDeclarationFileDiagnostics = canCopySemanticDiagnostics && !compilerOptions.skipLibCheck === !oldState!.program.getCompilerOptions().skipLibCheck; - const copyLibFileDiagnostics = copyDeclarationFileDiagnostics && !compilerOptions.skipDefaultLibCheck === !oldState!.program.getCompilerOptions().skipDefaultLibCheck; + const copyDeclarationFileDiagnostics = canCopySemanticDiagnostics && !compilerOptions.skipLibCheck === !oldCompilerOptions!.skipLibCheck; + const copyLibFileDiagnostics = copyDeclarationFileDiagnostics && !compilerOptions.skipDefaultLibCheck === !oldCompilerOptions!.skipDefaultLibCheck; state.fileInfos.forEach((info, sourceFilePath) => { let oldInfo: Readonly | undefined; let newReferences: BuilderState.ReferencedSet | undefined; @@ -209,7 +211,7 @@ namespace ts { state.cleanedDiagnosticsOfLibFiles = true; const options = state.program.getCompilerOptions(); if (forEach(state.program.getSourceFiles(), f => - !contains(state.allFilesExcludingDefaultLibraryFile, f) && + state.program.isSourceFileDefaultLibrary(f) && !skipTypeChecking(f, options) && removeSemanticDiagnosticsOf(state, f.path) )) { From 941d97c45a37d0ac6a2b6ba8a6806933b14df856 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 9 Nov 2018 16:41:18 -0800 Subject: [PATCH 4/5] Handle global augmentation in the module --- src/compiler/builderState.ts | 20 +++++- src/testRunner/unittests/tscWatchMode.ts | 85 +++++++++++++++--------- 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/src/compiler/builderState.ts b/src/compiler/builderState.ts index 7cd67d99f72..7c7bebde9f9 100644 --- a/src/compiler/builderState.ts +++ b/src/compiler/builderState.ts @@ -368,7 +368,7 @@ namespace ts.BuilderState { } // If this is non module emit, or its a global file, it depends on all the source files - if (!state.referencedMap || (!isExternalModule(sourceFile) && !containsOnlyAmbientModules(sourceFile))) { + if (!state.referencedMap || isFileAffectingGlobalScope(sourceFile)) { return getAllFileNames(state, programOfThisState); } @@ -430,6 +430,22 @@ namespace ts.BuilderState { return true; } + /** + * Return true if file contains anything that augments to global scope we need to build them as if + * they are global files as well as module + */ + function containsGlobalScopeAugmentation(sourceFile: SourceFile) { + return some(sourceFile.moduleAugmentations, augmentation => isGlobalScopeAugmentation(augmentation.parent as ModuleDeclaration)); + } + + /** + * Return true if the file will invalidate all files because it affectes global scope + */ + function isFileAffectingGlobalScope(sourceFile: SourceFile) { + return containsGlobalScopeAugmentation(sourceFile) || + !isExternalModule(sourceFile) && !containsOnlyAmbientModules(sourceFile); + } + /** * Gets all files of the program excluding the default library file */ @@ -473,7 +489,7 @@ namespace ts.BuilderState { * When program emits modular code, gets the files affected by the sourceFile whose shape has changed */ function getFilesAffectedByUpdatedShapeWhenModuleEmit(state: BuilderState, programOfThisState: Program, sourceFileWithUpdatedShape: SourceFile, cacheToUpdateSignature: Map, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash | undefined, exportedModulesMapCache: ComputingExportedModulesMap | undefined) { - if (!isExternalModule(sourceFileWithUpdatedShape) && !containsOnlyAmbientModules(sourceFileWithUpdatedShape)) { + if (isFileAffectingGlobalScope(sourceFileWithUpdatedShape)) { return getAllFilesExcludingDefaultLibraryFile(state, programOfThisState, sourceFileWithUpdatedShape); } diff --git a/src/testRunner/unittests/tscWatchMode.ts b/src/testRunner/unittests/tscWatchMode.ts index 1e65e7b9e99..65f64b9da78 100644 --- a/src/testRunner/unittests/tscWatchMode.ts +++ b/src/testRunner/unittests/tscWatchMode.ts @@ -1565,15 +1565,13 @@ export class Data2 { }); }); - describe("updates errors in lib file when non module file changes", () => { + describe("updates errors in lib file", () => { const currentDirectory = "/user/username/projects/myproject"; const field = "fullscreen"; - const aFile: File = { - path: `${currentDirectory}/a.ts`, - content: `interface Document { + const fieldWithoutReadonly = `interface Document { ${field}: boolean; -}` - }; +}`; + const libFileWithDocument: File = { path: libFile.path, content: `${libFile.content} @@ -1586,40 +1584,63 @@ interface Document { return getDiagnosticOfFileFromProgram(program, file.path, file.content.indexOf(field), field.length, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, field); } - const files = [aFile, libFileWithDocument]; + function verifyLibFileErrorsWith(aFile: File) { + const files = [aFile, libFileWithDocument]; - function verifyLibErrors(options: CompilerOptions) { - const host = createWatchedSystem(files, { currentDirectory }); - const watch = createWatchOfFilesAndCompilerOptions([aFile.path], host, options); - checkProgramActualFiles(watch(), [aFile.path, libFile.path]); - checkOutputErrorsInitial(host, getErrors()); + function verifyLibErrors(options: CompilerOptions) { + const host = createWatchedSystem(files, { currentDirectory }); + const watch = createWatchOfFilesAndCompilerOptions([aFile.path], host, options); + checkProgramActualFiles(watch(), [aFile.path, libFile.path]); + checkOutputErrorsInitial(host, getErrors()); - host.writeFile(aFile.path, "var x = 10;"); - host.runQueuedTimeoutCallbacks(); - checkProgramActualFiles(watch(), [aFile.path, libFile.path]); - checkOutputErrorsIncremental(host, emptyArray); + host.writeFile(aFile.path, aFile.content.replace(fieldWithoutReadonly, "var x: string;")); + host.runQueuedTimeoutCallbacks(); + checkProgramActualFiles(watch(), [aFile.path, libFile.path]); + checkOutputErrorsIncremental(host, emptyArray); - host.writeFile(aFile.path, aFile.content); - host.runQueuedTimeoutCallbacks(); - checkProgramActualFiles(watch(), [aFile.path, libFile.path]); - checkOutputErrorsIncremental(host, getErrors()); + host.writeFile(aFile.path, aFile.content); + host.runQueuedTimeoutCallbacks(); + checkProgramActualFiles(watch(), [aFile.path, libFile.path]); + checkOutputErrorsIncremental(host, getErrors()); - function getErrors() { - return [ - ...(options.skipLibCheck || options.skipDefaultLibCheck ? [] : [getDiagnostic(watch(), libFileWithDocument)]), - getDiagnostic(watch(), aFile) - ]; + function getErrors() { + return [ + ...(options.skipLibCheck || options.skipDefaultLibCheck ? [] : [getDiagnostic(watch(), libFileWithDocument)]), + getDiagnostic(watch(), aFile) + ]; + } } + + it("with default options", () => { + verifyLibErrors({}); + }); + it("with skipLibCheck", () => { + verifyLibErrors({ skipLibCheck: true }); + }); + it("with skipDefaultLibCheck", () => { + verifyLibErrors({ skipDefaultLibCheck: true }); + }); } - it("with default options", () => { - verifyLibErrors({}); + describe("when non module file changes", () => { + const aFile: File = { + path: `${currentDirectory}/a.ts`, + content: `${fieldWithoutReadonly} +var y: number;` + }; + verifyLibFileErrorsWith(aFile); }); - it("with skipLibCheck", () => { - verifyLibErrors({ skipLibCheck: true }); - }); - it("with skipDefaultLibCheck", () => { - verifyLibErrors({ skipDefaultLibCheck: true }); + + describe("when module file with global definitions changes", () => { + const aFile: File = { + path: `${currentDirectory}/a.ts`, + content: `export {} +declare global { +${fieldWithoutReadonly} +var y: number; +}` + }; + verifyLibFileErrorsWith(aFile); }); }); From 1b8bfc832a7c4eaef305481fd9f2110c7c96647d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 9 Nov 2018 17:13:39 -0800 Subject: [PATCH 5/5] Check modifiers on variable like declarations only --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b78490b4c94..ea596c2ac2c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -25111,7 +25111,7 @@ namespace ts { } } if (symbol.declarations.length > 1) { - if (some(symbol.declarations, d => d !== node && !areDeclarationFlagsIdentical(d, node))) { + if (some(symbol.declarations, d => d !== node && isVariableLike(d) && !areDeclarationFlagsIdentical(d, node))) { error(node.name, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, declarationNameToString(node.name)); } }