If we are updating dts of any of the file and it affects global scope, everything needs update in signature and dts emit (microsoft#48600)

* Add test

* Renames

* More refactoring

* If we are updating dts of any of the file and it affects global scope, everything needs update in signature and dts emit
Fixes microsoft#42769

Author: sheetalkamat

src/compiler/builder.ts

@@ -434,25 +434,35 @@ namespace ts {
         return undefined;
     }
 
+    function removeDiagnosticsOfLibraryFiles(state: BuilderProgramState) {
+        if (!state.cleanedDiagnosticsOfLibFiles) {
+            state.cleanedDiagnosticsOfLibFiles = true;
+            const program = Debug.checkDefined(state.program);
+            const options = program.getCompilerOptions();
+            forEach(program.getSourceFiles(), f =>
+                program.isSourceFileDefaultLibrary(f) &&
+                !skipTypeChecking(f, options, program) &&
+                removeSemanticDiagnosticsOf(state, f.resolvedPath)
+            );
+        }
+    }
+
     /**
      *  Handles semantic diagnostics and dts emit for affectedFile and files, that are referencing modules that export entities from affected file
      *  This is because even though js emit doesnt change, dts emit / type used can change resulting in need for dts emit and js change
      */
-    function handleDtsMayChangeOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, cancellationToken: CancellationToken | undefined, computeHash: BuilderState.ComputeHash, host: BuilderProgramHost) {
+    function handleDtsMayChangeOfAffectedFile(
+        state: BuilderProgramState,
+        affectedFile: SourceFile,
+        cancellationToken: CancellationToken | undefined,
+        computeHash: BuilderState.ComputeHash,
+        host: BuilderProgramHost,
+    ) {
         removeSemanticDiagnosticsOf(state, affectedFile.resolvedPath);
 
         // If affected files is everything except default library, then nothing more to do
         if (state.allFilesExcludingDefaultLibraryFile === state.affectedFiles) {
-            if (!state.cleanedDiagnosticsOfLibFiles) {
-                state.cleanedDiagnosticsOfLibFiles = true;
-                const program = Debug.checkDefined(state.program);
-                const options = program.getCompilerOptions();
-                forEach(program.getSourceFiles(), f =>
-                    program.isSourceFileDefaultLibrary(f) &&
-                    !skipTypeChecking(f, options, program) &&
-                    removeSemanticDiagnosticsOf(state, f.resolvedPath)
-                );
-            }
+            removeDiagnosticsOfLibraryFiles(state);
             // When a change affects the global scope, all files are considered to be affected without updating their signature
             // That means when affected file is handled, its signature can be out of date
             // To avoid this, ensure that we update the signature for any affected file in this scenario.
@@ -467,20 +477,22 @@ namespace ts {
             );
             return;
         }
-        else {
-            Debug.assert(state.hasCalledUpdateShapeSignature.has(affectedFile.resolvedPath) || state.currentAffectedFilesSignatures?.has(affectedFile.resolvedPath), `Signature not updated for affected file: ${affectedFile.fileName}`);
-        }
-
-        if (!state.compilerOptions.assumeChangesOnlyAffectDirectDependencies) {
-            forEachReferencingModulesOfExportOfAffectedFile(state, affectedFile, (state, path) => handleDtsMayChangeOf(state, path, cancellationToken, computeHash, host));
-        }
+        Debug.assert(state.hasCalledUpdateShapeSignature.has(affectedFile.resolvedPath) || state.currentAffectedFilesSignatures?.has(affectedFile.resolvedPath), `Signature not updated for affected file: ${affectedFile.fileName}`);
+        if (state.compilerOptions.assumeChangesOnlyAffectDirectDependencies) return;
+        handleDtsMayChangeOfReferencingExportOfAffectedFile(state, affectedFile, cancellationToken, computeHash, host);
     }
 
     /**
      * Handle the dts may change, so they need to be added to pending emit if dts emit is enabled,
      * Also we need to make sure signature is updated for these files
      */
-    function handleDtsMayChangeOf(state: BuilderProgramState, path: Path, cancellationToken: CancellationToken | undefined, computeHash: BuilderState.ComputeHash, host: BuilderProgramHost): void {
+    function handleDtsMayChangeOf(
+        state: BuilderProgramState,
+        path: Path,
+        cancellationToken: CancellationToken | undefined,
+        computeHash: BuilderState.ComputeHash,
+        host: BuilderProgramHost
+    ): void {
         removeSemanticDiagnosticsOf(state, path);
 
         if (!state.changedFilesSet.has(path)) {
@@ -529,16 +541,61 @@ namespace ts {
         return newSignature !== oldSignature;
     }
 
+    function forEachKeyOfExportedModulesMap<T>(
+        state: BuilderProgramState,
+        filePath: Path,
+        fn: (exportedFromPath: Path) => T | undefined,
+    ): T | undefined {
+        // Go through exported modules from cache first
+        let keys = state.currentAffectedFilesExportedModulesMap!.getKeys(filePath);
+        const result = keys && forEachKey(keys, fn);
+        if (result) return result;
+
+        // If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
+        keys = state.exportedModulesMap!.getKeys(filePath);
+        return keys && forEachKey(keys, exportedFromPath =>
+            // If the cache had an updated value, skip
+            !state.currentAffectedFilesExportedModulesMap!.hasKey(exportedFromPath) &&
+            !state.currentAffectedFilesExportedModulesMap!.deletedKeys()?.has(exportedFromPath) ?
+                fn(exportedFromPath) :
+                undefined
+        );
+    }
+
+    function handleDtsMayChangeOfGlobalScope(
+        state: BuilderProgramState,
+        filePath: Path,
+        cancellationToken: CancellationToken | undefined,
+        computeHash: BuilderState.ComputeHash,
+        host: BuilderProgramHost,
+    ): boolean {
+        if (!state.fileInfos.get(filePath)?.affectsGlobalScope) return false;
+        // Every file needs to be handled
+        BuilderState.getAllFilesExcludingDefaultLibraryFile(state, state.program!, /*firstSourceFile*/ undefined)
+            .forEach(file => handleDtsMayChangeOf(
+                state,
+                file.resolvedPath,
+                cancellationToken,
+                computeHash,
+                host,
+            ));
+        removeDiagnosticsOfLibraryFiles(state);
+        return true;
+    }
+
     /**
-     * Iterate on referencing modules that export entities from affected file
+     * Iterate on referencing modules that export entities from affected file and delete diagnostics and add pending emit
      */
-    function forEachReferencingModulesOfExportOfAffectedFile(state: BuilderProgramState, affectedFile: SourceFile, fn: (state: BuilderProgramState, filePath: Path) => void) {
+    function handleDtsMayChangeOfReferencingExportOfAffectedFile(
+        state: BuilderProgramState,
+        affectedFile: SourceFile,
+        cancellationToken: CancellationToken | undefined,
+        computeHash: BuilderState.ComputeHash,
+        host: BuilderProgramHost
+    ) {
         // If there was change in signature (dts output) for the changed file,
         // then only we need to handle pending file emit
-        if (!state.exportedModulesMap || !state.changedFilesSet.has(affectedFile.resolvedPath)) {
-            return;
-        }
-
+        if (!state.exportedModulesMap || !state.changedFilesSet.has(affectedFile.resolvedPath)) return;
         if (!isChangedSignature(state, affectedFile.resolvedPath)) return;
 
         // Since isolated modules dont change js files, files affected by change in signature is itself
@@ -551,7 +608,8 @@ namespace ts {
                 const currentPath = queue.pop()!;
                 if (!seenFileNamesMap.has(currentPath)) {
                     seenFileNamesMap.set(currentPath, true);
-                    fn(state, currentPath);
+                    if (handleDtsMayChangeOfGlobalScope(state, currentPath, cancellationToken, computeHash, host)) return;
+                    handleDtsMayChangeOf(state, currentPath, cancellationToken, computeHash, host);
                     if (isChangedSignature(state, currentPath)) {
                         const currentSourceFile = Debug.checkDefined(state.program).getSourceFileByPath(currentPath)!;
                         queue.push(...BuilderState.getReferencedByPaths(state, currentSourceFile.resolvedPath));
@@ -561,65 +619,69 @@ namespace ts {
         }
 
         Debug.assert(!!state.currentAffectedFilesExportedModulesMap);
-
         const seenFileAndExportsOfFile = new Set<string>();
         // Go through exported modules from cache first
         // If exported modules has path, all files referencing file exported from are affected
-        state.currentAffectedFilesExportedModulesMap.getKeys(affectedFile.resolvedPath)?.forEach(exportedFromPath =>
-            forEachFilesReferencingPath(state, exportedFromPath, seenFileAndExportsOfFile, fn)
-        );
-
-        // If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
-        state.exportedModulesMap.getKeys(affectedFile.resolvedPath)?.forEach(exportedFromPath =>
-            // If the cache had an updated value, skip
-            !state.currentAffectedFilesExportedModulesMap!.hasKey(exportedFromPath) &&
-            !state.currentAffectedFilesExportedModulesMap!.deletedKeys()?.has(exportedFromPath) &&
-            forEachFilesReferencingPath(state, exportedFromPath, seenFileAndExportsOfFile, fn)
-        );
-    }
-
-    /**
-     * Iterate on files referencing referencedPath
-     */
-    function forEachFilesReferencingPath(state: BuilderProgramState, referencedPath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => void): void {
-        state.referencedMap!.getKeys(referencedPath)?.forEach(filePath =>
-            forEachFileAndExportsOfFile(state, filePath, seenFileAndExportsOfFile, fn)
-        );
+        forEachKeyOfExportedModulesMap(state, affectedFile.resolvedPath, exportedFromPath => {
+            if (handleDtsMayChangeOfGlobalScope(state, exportedFromPath, cancellationToken, computeHash, host)) return true;
+            const references = state.referencedMap!.getKeys(exportedFromPath);
+            return references && forEachKey(references, filePath =>
+                handleDtsMayChangeOfFileAndExportsOfFile(
+                    state,
+                    filePath,
+                    seenFileAndExportsOfFile,
+                    cancellationToken,
+                    computeHash,
+                    host,
+                )
+            );
+        });
     }
 
     /**
-     * fn on file and iterate on anything that exports this file
+     * handle dts and semantic diagnostics on file and iterate on anything that exports this file
+     * return true when all work is done and we can exit handling dts emit and semantic diagnostics
      */
-    function forEachFileAndExportsOfFile(state: BuilderProgramState, filePath: Path, seenFileAndExportsOfFile: Set<string>, fn: (state: BuilderProgramState, filePath: Path) => void): void {
-        if (!tryAddToSet(seenFileAndExportsOfFile, filePath)) {
-            return;
-        }
-
-        fn(state, filePath);
-
+    function handleDtsMayChangeOfFileAndExportsOfFile(
+        state: BuilderProgramState,
+        filePath: Path,
+        seenFileAndExportsOfFile: Set<string>,
+        cancellationToken: CancellationToken | undefined,
+        computeHash: BuilderState.ComputeHash,
+        host: BuilderProgramHost,
+    ): boolean | undefined {
+        if (!tryAddToSet(seenFileAndExportsOfFile, filePath)) return undefined;
+
+        if (handleDtsMayChangeOfGlobalScope(state, filePath, cancellationToken, computeHash, host)) return true;
+        handleDtsMayChangeOf(state, filePath, cancellationToken, computeHash, host);
         Debug.assert(!!state.currentAffectedFilesExportedModulesMap);
-        // Go through exported modules from cache first
-        // If exported modules has path, all files referencing file exported from are affected
-        state.currentAffectedFilesExportedModulesMap.getKeys(filePath)?.forEach(exportedFromPath =>
-            forEachFileAndExportsOfFile(state, exportedFromPath, seenFileAndExportsOfFile, fn)
-        );
 
-        // If exported from path is not from cache and exported modules has path, all files referencing file exported from are affected
-        state.exportedModulesMap!.getKeys(filePath)?.forEach(exportedFromPath =>
-            // If the cache had an updated value, skip
-            !state.currentAffectedFilesExportedModulesMap!.hasKey(exportedFromPath) &&
-            !state.currentAffectedFilesExportedModulesMap!.deletedKeys()?.has(exportedFromPath) &&
-            forEachFileAndExportsOfFile(state, exportedFromPath, seenFileAndExportsOfFile, fn)
+        // If exported modules has path, all files referencing file exported from are affected
+        forEachKeyOfExportedModulesMap(state, filePath, exportedFromPath =>
+            handleDtsMayChangeOfFileAndExportsOfFile(
+                state,
+                exportedFromPath,
+                seenFileAndExportsOfFile,
+                cancellationToken,
+                computeHash,
+                host,
+            )
         );
 
         // Remove diagnostics of files that import this file (without going to exports of referencing files)
         state.referencedMap!.getKeys(filePath)?.forEach(referencingFilePath =>
             !seenFileAndExportsOfFile.has(referencingFilePath) && // Not already removed diagnostic file
-            fn(state, referencingFilePath) // Dont add to seen since this is not yet done with the export removal
+            handleDtsMayChangeOf( // Dont add to seen since this is not yet done with the export removal
+                state,
+                referencingFilePath,
+                cancellationToken,
+                computeHash,
+                host,
+            )
         );
+        return undefined;
     }
 
-
     /**
      * This is called after completing operation on the next affected file.
      * The operations here are postponed to ensure that cancellation during the iteration is handled correctly

src/testRunner/unittests/tsc/incremental.ts

@@ -464,5 +464,40 @@ declare global {
                 fs.writeFileSync("/lib/lib.esnext.d.ts", libContent);
             }
         });
+
+        verifyTscWithEdits({
+            scenario: "incremental",
+            subScenario: "change to type that gets used as global through export in another file",
+            commandLineArgs: ["-p", `src/project`],
+            fs: () => loadProjectFromFiles({
+                "/src/project/tsconfig.json": JSON.stringify({ compilerOptions: { composite: true }, }),
+                "/src/project/class1.ts": `const a: MagicNumber = 1;
+console.log(a);`,
+                "/src/project/constants.ts": "export default 1;",
+                "/src/project/types.d.ts": `type MagicNumber = typeof import('./constants').default`,
+            }),
+            edits: [{
+                subScenario: "Modify imports used in global file",
+                modifyFs: fs => fs.writeFileSync("/src/project/constants.ts", "export default 2;"),
+            }],
+        });
+
+        verifyTscWithEdits({
+            scenario: "incremental",
+            subScenario: "change to type that gets used as global through export in another file through indirect import",
+            commandLineArgs: ["-p", `src/project`],
+            fs: () => loadProjectFromFiles({
+                "/src/project/tsconfig.json": JSON.stringify({ compilerOptions: { composite: true }, }),
+                "/src/project/class1.ts": `const a: MagicNumber = 1;
+console.log(a);`,
+                "/src/project/constants.ts": "export default 1;",
+                "/src/project/reexport.ts": `export { default as ConstantNumber } from "./constants"`,
+                "/src/project/types.d.ts": `type MagicNumber = typeof import('./reexport').ConstantNumber`,
+            }),
+            edits: [{
+                subScenario: "Modify imports used in global file",
+                modifyFs: fs => fs.writeFileSync("/src/project/constants.ts", "export default 2;"),
+            }],
+        });
     });
 }