More work on PR feedback

src/compiler/types.ts

@@ -3688,7 +3688,13 @@ namespace ts {
 
     export interface ConfigFileSpecs {
         filesSpecs: ReadonlyArray<string>;
+        /**
+         * Present to report errors (user specified specs), validatedIncludeSpecs are used for file name matching
+         */
         includeSpecs: ReadonlyArray<string>;
+        /**
+         * Present to report errors (user specified specs), validatedExcludeSpecs are used for file name matching
+         */
         excludeSpecs: ReadonlyArray<string>;
         validatedIncludeSpecs: ReadonlyArray<string>;
         validatedExcludeSpecs: ReadonlyArray<string>;

src/compiler/utilities.ts

@@ -3607,6 +3607,10 @@ namespace ts {
     export function getCombinedLocalAndExportSymbolFlags(symbol: Symbol): SymbolFlags {
         return symbol.exportSymbol ? symbol.exportSymbol.flags | symbol.flags : symbol.flags;
     }
+
+    export function isRecursiveDirectoryWatch(flags: WatchDirectoryFlags) {
+        return (flags & WatchDirectoryFlags.Recursive) !== 0;
+    }
 }
 
 namespace ts {

src/server/editorServices.ts

@@ -280,7 +280,7 @@ namespace ts.server {
     /* @internal */
     export type ServerDirectoryWatcherCallback = (path: NormalizedPath) => void;
 
-    type ConfigFileExistence = {
+    interface ConfigFileExistence {
         /**
          * Cached value of existence of config file
          * It is true if there is configured project open for this file.
@@ -302,7 +302,7 @@ namespace ts.server {
          * The watcher is present only when there is no open configured project for this config file
          */
         configFileWatcher?: FileWatcher;
-    };
+    }
 
     export interface ProjectServiceOptions {
         host: ServerHost;
@@ -1606,13 +1606,15 @@ namespace ts.server {
         }
 
         /* @internal */
-        closeDirectoryWatcher(watchType: WatchType, project: Project, directory: string, watcher: FileWatcher, recursive: boolean, reason: WatcherCloseReason) {
+        closeDirectoryWatcher(watchType: WatchType, project: Project, directory: string, watcher: FileWatcher, flags: WatchDirectoryFlags, reason: WatcherCloseReason) {
+            const recursive = isRecursiveDirectoryWatch(flags);
             this.logger.info(`DirectoryWatcher ${recursive ? "recursive" : ""}:: Close: ${directory} Project: ${project.getProjectName()} WatchType: ${watchType} Reason: ${reason}`);
             watcher.close();
         }
 
         /* @internal */
-        addDirectoryWatcher(watchType: WatchType, project: Project, directory: string, cb: ServerDirectoryWatcherCallback, recursive: boolean) {
+        addDirectoryWatcher(watchType: WatchType, project: Project, directory: string, cb: ServerDirectoryWatcherCallback, flags: WatchDirectoryFlags) {
+            const recursive = isRecursiveDirectoryWatch(flags);
             this.logger.info(`DirectoryWatcher ${recursive ? "recursive" : ""}:: Added: ${directory} Project: ${project.getProjectName()} WatchType: ${watchType}`);
             return this.host.watchDirectory(directory, fileName => {
                 const path = toNormalizedPath(getNormalizedAbsolutePath(fileName, directory));

src/server/project.ts

@@ -327,10 +327,12 @@ namespace ts.server {
             this.lsHost = undefined;
 
             // Clean up file watchers waiting for missing files
-            cleanExistingMap(this.missingFilesMap, (missingFilePath, fileWatcher) => {
-                this.projectService.closeFileWatcher(WatchType.MissingFilePath, this, missingFilePath, fileWatcher, WatcherCloseReason.ProjectClose);
-            });
-            this.missingFilesMap = undefined;
+            if (this.missingFilesMap) {
+                clearMap(this.missingFilesMap, (missingFilePath, fileWatcher) => {
+                    this.projectService.closeFileWatcher(WatchType.MissingFilePath, this, missingFilePath, fileWatcher, WatcherCloseReason.ProjectClose);
+                });
+                this.missingFilesMap = undefined;
+            }
 
             // signal language service to release source files acquired from document registry
             this.languageService.dispose();
@@ -642,34 +644,37 @@ namespace ts.server {
                 const missingFilePaths = this.program.getMissingFilePaths();
                 const newMissingFilePathMap = arrayToSet(missingFilePaths);
                 // Update the missing file paths watcher
-                this.missingFilesMap = mutateExistingMapWithNewSet(
-                    this.missingFilesMap, newMissingFilePathMap,
-                    // Watch the missing files
-                    missingFilePath => {
-                        const fileWatcher = this.projectService.addFileWatcher(
-                            WatchType.MissingFilePath, this, missingFilePath,
-                            (filename, eventKind) => {
-                                if (eventKind === FileWatcherEventKind.Created && this.missingFilesMap.has(missingFilePath)) {
-                                    this.missingFilesMap.delete(missingFilePath);
-                                    this.projectService.closeFileWatcher(WatchType.MissingFilePath, this, missingFilePath, fileWatcher, WatcherCloseReason.FileCreated);
+                mutateMap(
+                    this.missingFilesMap || (this.missingFilesMap = createMap()),
+                    newMissingFilePathMap,
+                    {
+                        // Watch the missing files
+                        createNewValue: missingFilePath => {
+                            const fileWatcher = this.projectService.addFileWatcher(
+                                WatchType.MissingFilePath, this, missingFilePath,
+                                (filename, eventKind) => {
+                                    if (eventKind === FileWatcherEventKind.Created && this.missingFilesMap.has(missingFilePath)) {
+                                        this.missingFilesMap.delete(missingFilePath);
+                                        this.projectService.closeFileWatcher(WatchType.MissingFilePath, this, missingFilePath, fileWatcher, WatcherCloseReason.FileCreated);
 
-                                    if (this.projectKind === ProjectKind.Configured) {
-                                        const absoluteNormalizedPath = getNormalizedAbsolutePath(filename, getDirectoryPath(missingFilePath));
-                                        (this.lsHost.host as CachedServerHost).addOrDeleteFileOrFolder(toNormalizedPath(absoluteNormalizedPath));
+                                        if (this.projectKind === ProjectKind.Configured) {
+                                            const absoluteNormalizedPath = getNormalizedAbsolutePath(filename, getDirectoryPath(missingFilePath));
+                                            (this.lsHost.host as CachedServerHost).addOrDeleteFileOrFolder(toNormalizedPath(absoluteNormalizedPath));
+                                        }
+
+                                        // When a missing file is created, we should update the graph.
+                                        this.markAsDirty();
+                                        this.projectService.delayUpdateProjectGraphAndInferredProjectsRefresh(this);
                                     }
-
-                                    // When a missing file is created, we should update the graph.
-                                    this.markAsDirty();
-                                    this.projectService.delayUpdateProjectGraphAndInferredProjectsRefresh(this);
                                 }
-                            }
-                        );
-                        return fileWatcher;
-                    },
-                    // Files that are no longer missing (e.g. because they are no longer required)
-                    // should no longer be watched.
-                    (missingFilePath, fileWatcher) => {
-                        this.projectService.closeFileWatcher(WatchType.MissingFilePath, this, missingFilePath, fileWatcher, WatcherCloseReason.NotNeeded);
+                            );
+                            return fileWatcher;
+                        },
+                        // Files that are no longer missing (e.g. because they are no longer required)
+                        // should no longer be watched.
+                        onDeleteExistingValue: (missingFilePath, fileWatcher) => {
+                            this.projectService.closeFileWatcher(WatchType.MissingFilePath, this, missingFilePath, fileWatcher, WatcherCloseReason.NotNeeded);
+                        }
                     }
                 );
             }
@@ -970,7 +975,10 @@ namespace ts.server {
         }
     }
 
-    type WildCardDirectoryWatchers = { watcher: FileWatcher, recursive: boolean };
+    interface WildcardDirectoryWatcher {
+        watcher: FileWatcher;
+        flags: WatchDirectoryFlags;
+    }
 
     /**
      * If a file is opened, the server will look for a tsconfig (or jsconfig)
@@ -981,7 +989,7 @@ namespace ts.server {
         private typeAcquisition: TypeAcquisition;
         /* @internal */
         configFileWatcher: FileWatcher;
-        private directoriesWatchedForWildcards: Map<WildCardDirectoryWatchers> | undefined;
+        private directoriesWatchedForWildcards: Map<WildcardDirectoryWatcher> | undefined;
         private typeRootsWatchers: Map<FileWatcher> | undefined;
         readonly canonicalConfigFilePath: NormalizedPath;
 
@@ -1136,70 +1144,72 @@ namespace ts.server {
         }
 
         watchWildcards(wildcardDirectories: Map<WatchDirectoryFlags>) {
-            this.directoriesWatchedForWildcards = mutateExistingMap(
-                this.directoriesWatchedForWildcards, wildcardDirectories,
-                // Watcher is same if the recursive flags match
-                ({ recursive: existingRecursive }, flag) => {
-                    // If the recursive dont match, it needs update
-                    const recursive = (flag & WatchDirectoryFlags.Recursive) !== 0;
-                    return existingRecursive !== recursive;
-                },
-                // Create new watch and recursive info
-                (directory, flag) => {
-                    const recursive = (flag & WatchDirectoryFlags.Recursive) !== 0;
-                    return {
-                        watcher: this.projectService.addDirectoryWatcher(
-                            WatchType.WildCardDirectories, this, directory,
-                            path => this.projectService.onFileAddOrRemoveInWatchedDirectoryOfProject(this, path),
-                            recursive
-                        ),
-                        recursive
-                    };
-                },
-                // Close existing watch thats not needed any more
-                (directory, { watcher, recursive }) => this.projectService.closeDirectoryWatcher(
-                    WatchType.WildCardDirectories, this, directory, watcher, recursive, WatcherCloseReason.NotNeeded
-                ),
-                // Close existing watch that doesnt match in recursive flag
-                (directory, { watcher, recursive }) => this.projectService.closeDirectoryWatcher(
-                    WatchType.WildCardDirectories, this, directory, watcher, recursive, WatcherCloseReason.RecursiveChanged
-                )
+            mutateMap(
+                this.directoriesWatchedForWildcards || (this.directoriesWatchedForWildcards = createMap()),
+                wildcardDirectories,
+                {
+                    // Watcher is same if the recursive flags match
+                    isSameValue: ({ flags: existingFlags }, flags) => existingFlags !== flags,
+                    // Create new watch and recursive info
+                    createNewValue: (directory, flags) => {
+                        return {
+                            watcher: this.projectService.addDirectoryWatcher(
+                                WatchType.WildCardDirectories, this, directory,
+                                path => this.projectService.onFileAddOrRemoveInWatchedDirectoryOfProject(this, path),
+                                flags
+                            ),
+                            flags
+                        };
+                    },
+                    // Close existing watch thats not needed any more or doesnt match recursive flags
+                    onDeleteExistingValue: (directory, wildcardDirectoryWatcher, isNotSame) =>
+                        this.closeWildcardDirectoryWatcher(directory, wildcardDirectoryWatcher, isNotSame ? WatcherCloseReason.RecursiveChanged : WatcherCloseReason.NotNeeded),
+                }
             );
         }
 
+        private closeWildcardDirectoryWatcher(directory: string, { watcher, flags }: WildcardDirectoryWatcher, closeReason: WatcherCloseReason) {
+            this.projectService.closeDirectoryWatcher(WatchType.WildCardDirectories, this, directory, watcher, flags, closeReason);
+        }
+
         stopWatchingWildCards(reason: WatcherCloseReason) {
-            cleanExistingMap(
-                this.directoriesWatchedForWildcards,
-                (directory, { watcher, recursive }) =>
-                    this.projectService.closeDirectoryWatcher(WatchType.WildCardDirectories, this,
-                        directory, watcher, recursive, reason)
-            );
-            this.directoriesWatchedForWildcards = undefined;
+            if (this.directoriesWatchedForWildcards) {
+                clearMap(
+                    this.directoriesWatchedForWildcards,
+                    (directory, wildcardDirectoryWatcher) => this.closeWildcardDirectoryWatcher(directory, wildcardDirectoryWatcher, reason)
+                );
+                this.directoriesWatchedForWildcards = undefined;
+            }
         }
 
         watchTypeRoots() {
             const newTypeRoots = arrayToSet(this.getEffectiveTypeRoots(), dir => this.projectService.toCanonicalFileName(dir));
-            this.typeRootsWatchers = mutateExistingMapWithNewSet(
-                this.typeRootsWatchers, newTypeRoots,
-                // Create new watch
-                root => this.projectService.addDirectoryWatcher(WatchType.TypeRoot, this, root,
-                    path => this.projectService.onTypeRootFileChanged(this, path), /*recursive*/ false
-                ),
-                // Close existing watch thats not needed any more
-                (directory, watcher) => this.projectService.closeDirectoryWatcher(
-                    WatchType.TypeRoot, this, directory, watcher, /*recursive*/ false, WatcherCloseReason.NotNeeded
-                )
+            mutateMap(
+                this.typeRootsWatchers || (this.typeRootsWatchers = createMap()),
+                newTypeRoots,
+                {
+                    // Create new watch
+                    createNewValue: root => this.projectService.addDirectoryWatcher(WatchType.TypeRoot, this, root,
+                        path => this.projectService.onTypeRootFileChanged(this, path), WatchDirectoryFlags.None
+                    ),
+                    // Close existing watch thats not needed any more
+                    onDeleteExistingValue: (directory, watcher) => this.projectService.closeDirectoryWatcher(
+                        WatchType.TypeRoot, this, directory, watcher, WatchDirectoryFlags.None, WatcherCloseReason.NotNeeded
+                    )
+                }
             );
         }
 
         stopWatchingTypeRoots(reason: WatcherCloseReason) {
-            cleanExistingMap(
-                this.typeRootsWatchers,
-                (directory, watcher) =>
-                    this.projectService.closeDirectoryWatcher(WatchType.TypeRoot, this,
-                        directory, watcher, /*recursive*/ false, reason)
-            );
-            this.typeRootsWatchers = undefined;
+            if (this.typeRootsWatchers) {
+                clearMap(
+                    this.typeRootsWatchers,
+                    (directory, watcher) =>
+                        this.projectService.closeDirectoryWatcher(WatchType.TypeRoot, this,
+                            directory, watcher, WatchDirectoryFlags.None, reason)
+                );
+                this.typeRootsWatchers = undefined;
+            }
         }
 
         close() {

src/server/utilities.ts

@@ -293,76 +293,53 @@ namespace ts.server {
         }
     }
 
-    export function cleanExistingMap<T>(
-        existingMap: Map<T>,
-        onDeleteExistingValue: (key: string, existingValue: T) => void) {
-        if (existingMap) {
-            // Remove all
-            existingMap.forEach((existingValue, key) => {
-                existingMap.delete(key);
-                onDeleteExistingValue(key, existingValue);
-            });
-        }
+    /**
+     * clears already present map (!== undefined) by calling onDeleteExistingValue callback before deleting that key/value
+     */
+    export function clearMap<T>(map: Map<T>, onDeleteExistingValue: (key: string, existingValue: T) => void) {
+        // Remove all
+        map.forEach((existingValue, key) => {
+            map.delete(key);
+            onDeleteExistingValue(key, existingValue);
+        });
     }
 
-    export function mutateExistingMapWithNewSet<T>(
-        existingMap: Map<T>, newMap: Map<true>,
-        createNewValue: (key: string) => T,
-        onDeleteExistingValue: (key: string, existingValue: T) => void
-    ): Map<T> {
-        return mutateExistingMap(
-            existingMap, newMap,
-            // Same value if the value is set in the map
-            /*isSameValue*/(_existingValue, _valueInNewMap) => true,
-            /*createNewValue*/(key, _valueInNewMap) => createNewValue(key),
-            onDeleteExistingValue,
-            // Should never be called since we say yes to same values all the time
-            /*OnDeleteExistingMismatchValue*/(_key, _existingValue) => notImplemented()
-        );
+    export interface MutateMapOptions<T, U> {
+        createNewValue(key: string, valueInNewMap: U): T;
+        onDeleteExistingValue(key: string, existingValue: T, isNotSame?: boolean): void;
+
+        isSameValue?(existingValue: T, valueInNewMap: U): boolean;
     }
 
-    export function mutateExistingMap<T, U>(
-        existingMap: Map<T>, newMap: Map<U>,
-        isSameValue: (existingValue: T, valueInNewMap: U) => boolean,
-        createNewValue: (key: string, valueInNewMap: U) => T,
-        onDeleteExistingValue: (key: string, existingValue: T) => void,
-        OnDeleteExistingMismatchValue: (key: string, existingValue: T) => void
-    ): Map<T> {
+    /**
+     * Mutates the map with newMap such that keys in map will be same as newMap.
+     */
+    export function mutateMap<T, U>(map: Map<T>, newMap: ReadonlyMap<U>, options: MutateMapOptions<T, U>) {
         // If there are new values update them
         if (newMap) {
-            if (existingMap) {
-                // Needs update
-                existingMap.forEach((existingValue, key) => {
-                    const valueInNewMap = newMap.get(key);
-                    // Existing value - remove it
-                    if (valueInNewMap === undefined) {
-                        existingMap.delete(key);
-                        onDeleteExistingValue(key, existingValue);
-                    }
+            const { isSameValue, createNewValue, onDeleteExistingValue } = options;
+            // Needs update
+            map.forEach((existingValue, key) => {
+                const valueInNewMap = newMap.get(key);
+                if (valueInNewMap === undefined ||
                     // different value - remove it
-                    else if (!isSameValue(existingValue, valueInNewMap)) {
-                        existingMap.delete(key);
-                        OnDeleteExistingMismatchValue(key, existingValue);
-                    }
-                });
-            }
-            else {
-                // Create new
-                existingMap = createMap<T>();
-            }
-
-            // Add new values that are not already present
-            newMap.forEach((valueInNewMap, key) => {
-                if (!existingMap.has(key)) {
-                    // New values
-                    existingMap.set(key, createNewValue(key, valueInNewMap));
+                    (isSameValue && !isSameValue(existingValue, valueInNewMap))) {
+                    const isNotSame = valueInNewMap !== undefined;
+                    map.delete(key);
+                    onDeleteExistingValue(key, existingValue, isNotSame);
                 }
             });
 
-            return existingMap;
+            // Add new values that are not already present
+            newMap.forEach((valueInNewMap, key) => {
+                if (!map.has(key)) {
+                    // New values
+                    map.set(key, createNewValue(key, valueInNewMap));
+                }
+            });
+        }
+        else {
+            clearMap(map, options.onDeleteExistingValue);
         }
-
-        cleanExistingMap(existingMap, onDeleteExistingValue);
-        return undefined;
     }
 }