check ambient modules before reusing old state

aozgaa · aozgaa · commit 6547c1816f23 · 2017-04-27T20:27:48.000-07:00
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -3201,9 +3201,13 @@
         "category": "Message",
         "code": 6182
     },
-    "Reusing module resolutions originating in '{0}' since this file was not modified.": {
+    "Reusing resolution of module '{0}' to file '{1}' from old program.": {
         "category": "Message",
-        "code": 6183
+        "code": 618
+    },
+    "Reusing module resolutions originating in '{0}' since resolutions are unchanged from old program.": {
+        "category": "Message",
+        "code": 6184
     },
     "Variable '{0}' implicitly has an '{1}' type.": {
         "category": "Error",
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -480,6 +480,7 @@ namespace ts {
         interface OldProgramState {
             program: Program | undefined;
             file: SourceFile;
+            /** The collection of paths modified *since* the old program. */
             modifiedFilePaths: Path[];
         }
 
@@ -497,101 +498,96 @@ namespace ts {
                 return resolveModuleNamesWorker(moduleNames, containingFile);
             }
 
-            const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile);
-            if (oldSourceFile === file) {
-                // `file` is unchanged from the old program, so we can reuse old module resolutions.
-                if (isTraceEnabled(options, host)) {
-                    trace(host, Diagnostics.Reusing_module_resolutions_originating_in_0_since_this_file_was_not_modified, file.path);
-                }
-                const oldSourceFileResult: ResolvedModuleFull[] = [];
-                for (const moduleName of moduleNames) {
-                    const resolvedModule = oldSourceFile.resolvedModules.get(moduleName);
-                    oldSourceFileResult.push(resolvedModule);
-                }
-                return oldSourceFileResult;
-            }
-
-            // at this point we know that either
+            // at this point we know at least one of the following hold:
             // - file has local declarations for ambient modules
-            // OR
             // - old program state is available
-            // OR
-            // - both of items above
-            // With this it is possible that we can tell how some module names from the initial list will be resolved
-            // without doing actual resolution (in particular if some name was resolved to ambient module).
-            // Such names should be excluded from the list of module names that will be provided to `resolveModuleNamesWorker`
-            // since we don't want to resolve them again.
-
-            // this is a list of modules for which we cannot predict resolution so they should be actually resolved
+            // With this information, we can infer some module resolutions without performing resolution.
+
+            /** An ordered list of module names for which we cannot recover the resolution. */
             let unknownModuleNames: string[];
-            // this is a list of combined results assembles from predicted and resolved results.
-            // Order in this list matches the order in the original list of module names `moduleNames` which is important
-            // so later we can split results to resolutions of modules and resolutions of module augmentations.
+            /**
+             * The indexing of elements in this list matches that of `moduleNames`.
+             *
+             * Before combining results, result[i] is in one of the following states:
+             * * undefined: needs to be recomputed,
+             * * predictedToResolveToAmbientModuleMarker: known to be an ambient module.
+             * Needs to be reset to undefined before returning,
+             * * ResolvedModuleFull instance: can be reused.
+             */
             let result: ResolvedModuleFull[];
-            // a transient placeholder that is used to mark predicted resolution in the result list
+            /** A transient placeholder used to mark predicted resolution in the result list. */
             const predictedToResolveToAmbientModuleMarker: ResolvedModuleFull = <any>{};
 
+            const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile);
+
             for (let i = 0; i < moduleNames.length; i++) {
                 const moduleName = moduleNames[i];
-                // module name is known to be resolved to ambient module if
-                // - module name is contained in the list of ambient modules that are locally declared in the file
-                // - in the old program module name was resolved to ambient module whose declaration is in non-modified file
+                // TODO: if we want to reuse resolutions more aggressively, refine this to check for whether the
+                // text of the corresponding modulenames has changed.
+                if (file === oldSourceFile) {
+                    const oldResolvedModule = oldSourceFile && oldSourceFile.resolvedModules.get(moduleName);
+                    if (oldResolvedModule) {
+                        if (isTraceEnabled(options, host)) {
+                            trace(host, Diagnostics.Reusing_resolution_of_module_0_to_file_1_from_old_program, moduleName, containingFile);
+                        }
+                        (result || (result = new Array(moduleNames.length)))[i] = oldResolvedModule;
+                        continue;
+                    }
+                }
+                // We know moduleName resolves to an ambient module provided that moduleName:
+                // - is in the list of ambient modules locally declared in the current source file.
+                // - resolved to an ambient module in the old program whose declaration is in an unmodified file
                 //   (so the same module declaration will land in the new program)
-                let isKnownToResolveToAmbientModule = false;
+                let resolvesToAmbientModuleInNonModifiedFile = false;
                 if (contains(file.ambientModuleNames, moduleName)) {
-                    isKnownToResolveToAmbientModule = true;
+                    resolvesToAmbientModuleInNonModifiedFile = true;
                     if (isTraceEnabled(options, host)) {
                         trace(host, Diagnostics.Module_0_was_resolved_as_locally_declared_ambient_module_in_file_1, moduleName, containingFile);
                     }
                 }
                 else {
-                    isKnownToResolveToAmbientModule = checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName, oldProgramState);
-                }
-
-                if (isKnownToResolveToAmbientModule) {
-                    if (!unknownModuleNames) {
-                        // found a first module name for which result can be prediced
-                        // this means that this module name should not be passed to `resolveModuleNamesWorker`.
-                        // We'll use a separate list for module names that are definitely unknown.
-                        result = new Array(moduleNames.length);
-                        // copy all module names that appear before the current one in the list
-                        // since they are known to be unknown
-                        unknownModuleNames = moduleNames.slice(0, i);
-                    }
-                    // Mark predicted resolution in the result list.
-                    result[i] = predictedToResolveToAmbientModuleMarker;
+                    resolvesToAmbientModuleInNonModifiedFile = moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName, oldProgramState);
                 }
-                else if (unknownModuleNames) {
-                    // found unknown module name and we are already using separate list for those - add it to the list
-                    unknownModuleNames.push(moduleName);
-                }
-            }
 
-            if (!unknownModuleNames) {
-                // we've looked throught the list but have not seen any predicted resolution
-                // use default logic
-                return resolveModuleNamesWorker(moduleNames, containingFile);
+                if (resolvesToAmbientModuleInNonModifiedFile) {
+                    (result || (result = new Array(moduleNames.length)))[i] = predictedToResolveToAmbientModuleMarker;
+                }
+                else {
+                    // Resolution failed in the old program, or resolved to an ambient module for which we can't reuse the result.
+                    (unknownModuleNames || (unknownModuleNames = [])).push(moduleName);
+                }
             }
 
-            const resolutions = unknownModuleNames.length
+            const resolutions = unknownModuleNames && unknownModuleNames.length
                 ? resolveModuleNamesWorker(unknownModuleNames, containingFile)
                 : emptyArray;
 
-            // combine results of resolutions and predicted results
+            // Combine results of resolutions and predicted results
+            if (!result) {
+                // There were no unresolved/ambient resolutions.
+                Debug.assert(resolutions.length === moduleNames.length);
+                return <ResolvedModuleFull[]>resolutions;
+            }
+
             let j = 0;
             for (let i = 0; i < result.length; i++) {
-                if (result[i] === predictedToResolveToAmbientModuleMarker) {
-                    result[i] = undefined;
+                if (result[i]) {
+                    if (result[i] === predictedToResolveToAmbientModuleMarker) {
+                        result[i] = undefined;
+                    }
                 }
                 else {
                     result[i] = resolutions[j];
                     j++;
                 }
             }
             Debug.assert(j === resolutions.length);
+
             return result;
 
-            function checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean {
+            // TODO: if we change our policy of rechecking failed lookups on each program create,
+            // we should adjust the value returned here.
+            function moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean {
                 const resolutionToFile = getResolvedModule(oldProgramState.file, moduleName);
                 if (resolutionToFile) {
                     // module used to be resolved to file - ignore it
@@ -706,7 +702,7 @@ namespace ts {
 
             modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path);
             // try to verify results of module resolution
-            for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) {
+            modifiedSourceFilesLoop: for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) {
                 const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory);
                 if (resolveModuleNamesWorker) {
                     const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), getTextOfLiteral);
@@ -715,7 +711,8 @@ namespace ts {
                     // ensure that module resolution results are still correct
                     const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo);
                     if (resolutionsChanged) {
-                        return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles;
+                        oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles;
+                        continue modifiedSourceFilesLoop;
                     }
                 }
                 if (resolveTypeReferenceDirectiveNamesWorker) {
@@ -724,14 +721,19 @@ namespace ts {
                     // ensure that types resolutions are still correct
                     const resolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, resolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, typeDirectiveIsEqualTo);
                     if (resolutionsChanged) {
-                        return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles;
+                        oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles;
+                        continue modifiedSourceFilesLoop;
                     }
                 }
                 // pass the cache of module/types resolutions from the old source file
                 newSourceFile.resolvedModules = oldSourceFile.resolvedModules;
                 newSourceFile.resolvedTypeReferenceDirectiveNames = oldSourceFile.resolvedTypeReferenceDirectiveNames;
             }
 
+            if (oldProgram.structureIsReused !== StructureIsReused.Completely) {
+                return oldProgram.structureIsReused;
+            }
+
             // update fileName -> file mapping
             for (let i = 0; i < newSourceFiles.length; i++) {
                 filesByName.set(filePaths[i], newSourceFiles[i]);
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2409,6 +2409,7 @@ namespace ts {
         /* @internal */ structureIsReused?: StructureIsReused;
     }
 
+    /* @internal */
     export const enum StructureIsReused {
         Completely,
         ModulesInUneditedFiles,
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -123,7 +123,7 @@ namespace ts {
     /* @internal */
     export function hasChangesInResolutions<T>(names: string[], newResolutions: T[], oldResolutions: Map<T>, comparer: (oldResolution: T, newResolution: T) => boolean): boolean {
         if (names.length !== newResolutions.length) {
-            return false;
+            return true;
         }
         for (let i = 0; i < names.length; i++) {
             const newResolution = newResolutions[i];
diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts
@@ -547,7 +547,8 @@ namespace ts {
                 "File 'node_modules/@types/typerefs2/package.json' does not exist.",
                 "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.",
                 "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========",
-                "Reusing module resolutions originating in 'f2.ts' since this file was not modified."
+                "Reusing resolution of module './b2' to file 'f2.ts' from old program.",
+                "Reusing resolution of module './f1' to file 'f2.ts' from old program."
             ], "should reuse module resolutions in f2 since it is unchanged");
         });
     });

Original file line number	Diff line number	Diff line change
`@@ -2409,6 +2409,7 @@ namespace ts {`
`2409`	`2409`	`/* @internal */ structureIsReused?: StructureIsReused;`
`2410`	`2410`	`}`
`2411`	`2411`
	`2412`	`+ /* @internal */`
`2412`	`2413`	`export const enum StructureIsReused {`
`2413`	`2414`	`Completely,`
`2414`	`2415`	`ModulesInUneditedFiles,`
Original file line number	Diff line number	Diff line change
`@@ -123,7 +123,7 @@ namespace ts {`
`123`	`123`	`/* @internal */`
`124`	`124`	`export function hasChangesInResolutions<T>(names: string[], newResolutions: T[], oldResolutions: Map<T>, comparer: (oldResolution: T, newResolution: T) => boolean): boolean {`
`125`	`125`	`if (names.length !== newResolutions.length) {`
`126`		`- return false;`
	`126`	`+ return true;`
`127`	`127`	`}`
`128`	`128`	`for (let i = 0; i < names.length; i++) {`
`129`	`129`	`const newResolution = newResolutions[i];`