Update to respond to PR feedback

Author: sheetalkamat

src/compiler/core.ts

@@ -2988,7 +2988,7 @@ namespace ts {
 
     /** Remove the *first* occurrence of `item` from the array. */
     export function unorderedRemoveItem<T>(array: T[], item: T) {
-        unorderedRemoveFirstItemWhere(array, element => element === item);
+        return unorderedRemoveFirstItemWhere(array, element => element === item);
     }
 
     /** Remove the *first* element satisfying `predicate`. */

src/compiler/resolutionCache.ts

@@ -10,7 +10,7 @@ namespace ts {
 
         invalidateResolutionOfFile(filePath: Path): void;
         removeResolutionsOfFile(filePath: Path): void;
-        setFilesWithInvalidatedNonRelativeUnresolvedImports(filesWithUnresolvedImports: Map<any>): void;
+        setFilesWithInvalidatedNonRelativeUnresolvedImports(filesWithUnresolvedImports: Map<ReadonlyArray<string>>): void;
         createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution;
 
         startCachingPerDirectoryResolution(): void;
@@ -75,7 +75,7 @@ namespace ts {
     export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootDirForResolution: string, logChangesWhenResolvingModule: boolean): ResolutionCache {
         let filesWithChangedSetOfUnresolvedImports: Path[] | undefined;
         let filesWithInvalidatedResolutions: Map<true> | undefined;
-        let filesWithInvalidatedNonRelativeUnresolvedImports: Map<any> | undefined;
+        let filesWithInvalidatedNonRelativeUnresolvedImports: Map<ReadonlyArray<string>> | undefined;
         let allFilesHaveInvalidatedResolution = false;
 
         const getCurrentDirectory = memoize(() => resolutionHost.getCurrentDirectory());
@@ -168,6 +168,16 @@ namespace ts {
             return collected;
         }
 
+        function isFileWithInvalidatedNonRelativeUnresolvedImports(path: Path) {
+            if (!filesWithInvalidatedNonRelativeUnresolvedImports) {
+                return false;
+            }
+
+            // Invalidated if file has unresolved imports
+            const value = filesWithInvalidatedNonRelativeUnresolvedImports.get(path);
+            return value && !!value.length;
+        }
+
         function createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution {
             if (allFilesHaveInvalidatedResolution || forceAllFilesAsInvalidated) {
                 // Any file asked would have invalidated resolution
@@ -177,7 +187,7 @@ namespace ts {
             const collected = filesWithInvalidatedResolutions;
             filesWithInvalidatedResolutions = undefined;
             return path => (collected && collected.has(path)) ||
-                (filesWithInvalidatedNonRelativeUnresolvedImports && filesWithInvalidatedNonRelativeUnresolvedImports.has(path));
+                isFileWithInvalidatedNonRelativeUnresolvedImports(path);
         }
 
         function clearPerDirectoryResolutions() {
@@ -242,7 +252,7 @@ namespace ts {
 
             const resolvedModules: R[] = [];
             const compilerOptions = resolutionHost.getCompilationSettings();
-            const hasInvalidatedNonRelativeUnresolvedImport = logChanges && filesWithInvalidatedNonRelativeUnresolvedImports && filesWithInvalidatedNonRelativeUnresolvedImports.has(path);
+            const hasInvalidatedNonRelativeUnresolvedImport = logChanges && isFileWithInvalidatedNonRelativeUnresolvedImports(path);
             const seenNamesInFile = createMap<true>();
             for (const name of names) {
                 let resolution = resolutionsInFile.get(name);
@@ -584,7 +594,7 @@ namespace ts {
             );
         }
 
-        function setFilesWithInvalidatedNonRelativeUnresolvedImports(filesMap: Map<any>) {
+        function setFilesWithInvalidatedNonRelativeUnresolvedImports(filesMap: Map<ReadonlyArray<string>>) {
             Debug.assert(filesWithInvalidatedNonRelativeUnresolvedImports === filesMap || filesWithInvalidatedNonRelativeUnresolvedImports === undefined);
             filesWithInvalidatedNonRelativeUnresolvedImports = filesMap;
         }

src/server/project.ts

@@ -96,15 +96,10 @@ namespace ts.server {
          */
         cachedUnresolvedImportsPerFile = createMap<ReadonlyArray<string>>();
 
-        /**
-         * This is the set that has entry to true if file doesnt contain any unresolved import
-         */
-        private filesWithNoUnresolvedImports = createMap<true>();
-
         /*@internal*/
         lastCachedUnresolvedImportsList: SortedReadonlyArray<string>;
         /*@internal*/
-        hasMoreOrLessScriptInfos = false;
+        private hasMoreOrLessFiles = false;
 
         private lastFileExceededProgramSize: string | undefined;
 
@@ -136,10 +131,10 @@ namespace ts.server {
          */
         private lastReportedVersion = 0;
         /**
-         * Current project structure version.
+         * Current project's program version. (incremented everytime new program is created that is not complete reuse from the old one)
          * This property is changed in 'updateGraph' based on the set of files in program
          */
-        private projectStructureVersion = 0;
+        private projectProgramVersion = 0;
         /**
          * Current version of the project state. It is changed when:
          * - new root file was added/removed
@@ -566,7 +561,6 @@ namespace ts.server {
             this.resolutionCache.clear();
             this.resolutionCache = undefined;
             this.cachedUnresolvedImportsPerFile = undefined;
-            this.filesWithNoUnresolvedImports = undefined;
             this.directoryStructureHost = undefined;
 
             // Clean up file watchers waiting for missing files
@@ -727,7 +721,6 @@ namespace ts.server {
             else {
                 this.resolutionCache.invalidateResolutionOfFile(info.path);
             }
-            this.filesWithNoUnresolvedImports.delete(info.path);
             this.cachedUnresolvedImportsPerFile.delete(info.path);
 
             if (detachFromProject) {
@@ -749,19 +742,11 @@ namespace ts.server {
         }
 
         /* @internal */
-        private extractUnresolvedImportsFromSourceFile(file: SourceFile, result: string[] | undefined, ambientModules: string[]): string[] | undefined {
-            // No unresolve imports in this file
-            if (this.filesWithNoUnresolvedImports.has(file.path)) {
-                return result;
-            }
-
+        private extractUnresolvedImportsFromSourceFile(file: SourceFile, ambientModules: string[]): ReadonlyArray<string> {
             const cached = this.cachedUnresolvedImportsPerFile.get(file.path);
             if (cached) {
-                // found cached result - use it and return
-                for (const f of cached) {
-                    (result || (result = [])).push(f);
-                }
-                return result;
+                // found cached result, return
+                return cached;
             }
             let unresolvedImports: string[] | undefined;
             if (file.resolvedModules) {
@@ -779,40 +764,39 @@ namespace ts.server {
                             trimmed = trimmed.substr(0, i);
                         }
                         (unresolvedImports || (unresolvedImports = [])).push(trimmed);
-                        (result || (result = [])).push(trimmed);
                     }
                 });
             }
-            if (unresolvedImports) {
-                this.cachedUnresolvedImportsPerFile.set(file.path, unresolvedImports);
-            }
-            else {
-                this.filesWithNoUnresolvedImports.set(file.path, true);
-            }
-            return result;
+
+            this.cachedUnresolvedImportsPerFile.set(file.path, unresolvedImports || emptyArray);
+            return unresolvedImports || emptyArray;
 
             function isAmbientlyDeclaredModule(name: string) {
                 return ambientModules.some(m => m === name);
             }
         }
 
+        /* @internal */
+        setHasMoreOrLessFiles() {
+            this.hasMoreOrLessFiles = true;
+        }
+
         /**
          * Updates set of files that contribute to this project
          * @returns: true if set of files in the project stays the same and false - otherwise.
          */
         updateGraph(): boolean {
             this.resolutionCache.startRecordingFilesWithChangedResolutions();
 
-            const hasChanges = this.updateGraphWorker();
-            const hasMoreOrLessScriptInfos = this.hasMoreOrLessScriptInfos;
-            this.hasMoreOrLessScriptInfos = false;
+            const hasNewProgram = this.updateGraphWorker();
+            const hasMoreOrLessFiles = this.hasMoreOrLessFiles;
+            this.hasMoreOrLessFiles = false;
 
             const changedFiles: ReadonlyArray<Path> = this.resolutionCache.finishRecordingFilesWithChangedResolutions() || emptyArray;
 
             for (const file of changedFiles) {
                 // delete cached information for changed files
                 this.cachedUnresolvedImportsPerFile.delete(file);
-                this.filesWithNoUnresolvedImports.delete(file);
             }
 
             // update builder only if language service is enabled
@@ -824,25 +808,28 @@ namespace ts.server {
                 // 3. new files were added/removed, but compilation settings stays the same - collect unresolved imports for all new/modified files
                 // (can reuse cached imports for files that were not changed)
                 // 4. compilation settings were changed in the way that might affect module resolution - drop all caches and collect all data from the scratch
-                if (hasChanges || changedFiles.length) {
+                if (hasNewProgram || changedFiles.length) {
                     let result: string[] | undefined;
                     const ambientModules = this.program.getTypeChecker().getAmbientModules().map(mod => stripQuotes(mod.getName()));
                     for (const sourceFile of this.program.getSourceFiles()) {
-                        result = this.extractUnresolvedImportsFromSourceFile(sourceFile, result, ambientModules);
+                        const unResolved = this.extractUnresolvedImportsFromSourceFile(sourceFile, ambientModules);
+                        if (unResolved !== emptyArray) {
+                            (result || (result = [])).push(...unResolved);
+                        }
                     }
                     this.lastCachedUnresolvedImportsList = result ? toDeduplicatedSortedArray(result) : emptyArray;
                 }
 
-                this.projectService.typingsCache.enqueueInstallTypingsForProject(this, this.lastCachedUnresolvedImportsList, hasMoreOrLessScriptInfos);
+                this.projectService.typingsCache.enqueueInstallTypingsForProject(this, this.lastCachedUnresolvedImportsList, hasMoreOrLessFiles);
             }
             else {
                 this.lastCachedUnresolvedImportsList = undefined;
             }
 
-            if (hasChanges) {
-                this.projectStructureVersion++;
+            if (hasNewProgram) {
+                this.projectProgramVersion++;
             }
-            return !hasChanges;
+            return !hasNewProgram;
         }
 
         /*@internal*/
@@ -878,9 +865,9 @@ namespace ts.server {
             // bump up the version if
             // - oldProgram is not set - this is a first time updateGraph is called
             // - newProgram is different from the old program and structure of the old program was not reused.
-            const hasChanges = this.program && (!oldProgram || (this.program !== oldProgram && !(oldProgram.structureIsReused & StructureIsReused.Completely)));
+            const hasNewProgram = this.program && (!oldProgram || (this.program !== oldProgram && !(oldProgram.structureIsReused & StructureIsReused.Completely)));
             this.hasChangedAutomaticTypeDirectiveNames = false;
-            if (hasChanges) {
+            if (hasNewProgram) {
                 if (oldProgram) {
                     for (const f of oldProgram.getSourceFiles()) {
                         if (this.program.getSourceFileByPath(f.path)) {
@@ -918,8 +905,8 @@ namespace ts.server {
                 removed => this.detachScriptInfoFromProject(removed)
             );
             const elapsed = timestamp() - start;
-            this.writeLog(`Finishing updateGraphWorker: Project: ${this.getProjectName()} Version: ${this.getProjectVersion()} structureChanged: ${hasChanges} Elapsed: ${elapsed}ms`);
-            return hasChanges;
+            this.writeLog(`Finishing updateGraphWorker: Project: ${this.getProjectName()} Version: ${this.getProjectVersion()} structureChanged: ${hasNewProgram} Elapsed: ${elapsed}ms`);
+            return hasNewProgram;
         }
 
         private detachScriptInfoFromProject(uncheckedFileName: string) {
@@ -993,7 +980,6 @@ namespace ts.server {
                 if (changesAffectModuleResolution(oldOptions, compilerOptions)) {
                     // reset cached unresolved imports if changes in compiler options affected module resolution
                     this.cachedUnresolvedImportsPerFile.clear();
-                    this.filesWithNoUnresolvedImports.clear();
                     this.lastCachedUnresolvedImportsList = undefined;
                     this.resolutionCache.clear();
                 }
@@ -1007,7 +993,7 @@ namespace ts.server {
 
             const info: protocol.ProjectVersionInfo = {
                 projectName: this.getProjectName(),
-                version: this.projectStructureVersion,
+                version: this.projectProgramVersion,
                 isInferred: this.projectKind === ProjectKind.Inferred,
                 options: this.getCompilationSettings(),
                 languageServiceDisabled: !this.languageServiceEnabled,
@@ -1018,7 +1004,7 @@ namespace ts.server {
             // check if requested version is the same that we have reported last time
             if (this.lastReportedFileNames && lastKnownVersion === this.lastReportedVersion) {
                 // if current structure version is the same - return info without any changes
-                if (this.projectStructureVersion === this.lastReportedVersion && !updatedFileNames) {
+                if (this.projectProgramVersion === this.lastReportedVersion && !updatedFileNames) {
                     return { info, projectErrors: this.getGlobalProjectErrors() };
                 }
                 // compute and return the difference
@@ -1041,7 +1027,7 @@ namespace ts.server {
                     }
                 });
                 this.lastReportedFileNames = currentFiles;
-                this.lastReportedVersion = this.projectStructureVersion;
+                this.lastReportedVersion = this.projectProgramVersion;
                 return { info, changes: { added, removed, updated }, projectErrors: this.getGlobalProjectErrors() };
             }
             else {
@@ -1050,7 +1036,7 @@ namespace ts.server {
                 const externalFiles = this.getExternalFiles().map(f => toNormalizedPath(f));
                 const allFiles = projectFileNames.concat(externalFiles);
                 this.lastReportedFileNames = arrayToSet(allFiles);
-                this.lastReportedVersion = this.projectStructureVersion;
+                this.lastReportedVersion = this.projectProgramVersion;
                 return { info, files: allFiles, projectErrors: this.getGlobalProjectErrors() };
             }
         }

src/server/scriptInfo.ts

@@ -304,7 +304,7 @@ namespace ts.server {
             const isNew = !this.isAttached(project);
             if (isNew) {
                 this.containingProjects.push(project);
-                project.hasMoreOrLessScriptInfos = true;
+                project.setHasMoreOrLessFiles();
                 if (!project.getCompilerOptions().preserveSymlinks) {
                     this.ensureRealPath();
                 }
@@ -329,23 +329,23 @@ namespace ts.server {
                     return;
                 case 1:
                     if (this.containingProjects[0] === project) {
-                        project.hasMoreOrLessScriptInfos = true;
+                        project.setHasMoreOrLessFiles();
                         this.containingProjects.pop();
                     }
                     break;
                 case 2:
                     if (this.containingProjects[0] === project) {
-                        project.hasMoreOrLessScriptInfos = true;
+                        project.setHasMoreOrLessFiles();
                         this.containingProjects[0] = this.containingProjects.pop();
                     }
                     else if (this.containingProjects[1] === project) {
-                        project.hasMoreOrLessScriptInfos = true;
+                        project.setHasMoreOrLessFiles();
                         this.containingProjects.pop();
                     }
                     break;
                 default:
                     if (unorderedRemoveItem(this.containingProjects, project)) {
-                        project.hasMoreOrLessScriptInfos = true;
+                        project.setHasMoreOrLessFiles();
                     }
                     break;
             }

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -7643,10 +7643,6 @@ declare namespace ts.server {
         private externalFiles;
         private missingFilesMap;
         private plugins;
-        /**
-         * This is the set that has entry to true if file doesnt contain any unresolved import
-         */
-        private filesWithNoUnresolvedImports;
         private lastFileExceededProgramSize;
         protected languageService: LanguageService;
         languageServiceEnabled: boolean;
@@ -7666,10 +7662,10 @@ declare namespace ts.server {
          */
         private lastReportedVersion;
         /**
-         * Current project structure version.
+         * Current project's program version. (incremented everytime new program is created that is not complete reuse from the old one)
          * This property is changed in 'updateGraph' based on the set of files in program
          */
-        private projectStructureVersion;
+        private projectProgramVersion;
         /**
          * Current version of the project state. It is changed when:
          * - new root file was added/removed