Tests for file watching behaviour and make sure that virtual file system with watch behaves same way

Author: sheetalkamat

.gitignore

@@ -16,6 +16,7 @@ tests/baselines/rwc/*
 tests/baselines/reference/projectOutput/*
 tests/baselines/local/projectOutput/*
 tests/baselines/reference/testresults.tap
+tests/baselines/symlinks/*
 tests/services/baselines/prototyping/local/*
 tests/services/browser/typescriptServices.js
 src/harness/*.js

src/harness/harnessLanguageService.ts

@@ -398,6 +398,7 @@ class SessionServerHost implements ts.server.ServerHost {
         "watchedFiles",
         "watchedDirectories",
         ts.createGetCanonicalFileName(this.useCaseSensitiveFileNames),
+        this,
     );
 
     constructor(private host: NativeLanguageServiceHost) {

src/harness/watchUtils.ts

@@ -1,5 +1,4 @@
 import {
-    addRange,
     arrayFrom,
     compareStringsCaseSensitive,
     contains,
@@ -10,6 +9,7 @@ import {
     GetCanonicalFileName,
     MultiMap,
     PollingInterval,
+    System,
 } from "./_namespaces/ts";
 
 export interface TestFileWatcher {
@@ -25,7 +25,7 @@ export interface TestFsWatcher<DirCallback> {
 export interface Watches<Data> {
     add(path: string, data: Data): void;
     remove(path: string, data: Data): void;
-    forEach(path: string, cb: (data: Data) => void): void;
+    forEach(path: string, cb: (data: Data, path: string) => void): void;
     serialize(baseline: string[]): void;
 }
 
@@ -44,6 +44,7 @@ export function createWatchUtils<PollingWatcherData, FsWatcherData>(
     pollingWatchesName: string,
     fsWatchesName: string,
     getCanonicalFileName: GetCanonicalFileName,
+    system: Required<Pick<System, "realpath">>,
 ): WatchUtils<PollingWatcherData, FsWatcherData> {
     const pollingWatches = initializeWatches<PollingWatcherData>(pollingWatchesName);
     const fsWatches = initializeWatches<FsWatcherData>(fsWatchesName);
@@ -64,6 +65,8 @@ export function createWatchUtils<PollingWatcherData, FsWatcherData>(
         const actuals = createMultiMap<string, Data>();
         let serialized: Map<string, Data[]> | undefined;
         let canonicalPathsToStrings: Map<string, Set<string>> | undefined;
+        let realToLinked: MultiMap<string, string> | undefined;
+        let pathToReal: Map<string, string> | undefined;
         return {
             add,
             remove,
@@ -73,40 +76,69 @@ export function createWatchUtils<PollingWatcherData, FsWatcherData>(
 
         function add(path: string, data: Data) {
             actuals.add(path, data);
-            if (actuals.get(path)!.length === 1) {
-                const canonicalPath = getCanonicalFileName(path);
-                if (canonicalPath !== path) {
-                    (canonicalPathsToStrings ??= new Map()).set(
-                        canonicalPath,
-                        (canonicalPathsToStrings?.get(canonicalPath) ?? new Set()).add(path),
-                    );
-                }
+            if (actuals.get(path)!.length !== 1) return;
+            const canonicalPath = getCanonicalFileName(path);
+            if (canonicalPath !== path) {
+                (canonicalPathsToStrings ??= new Map()).set(
+                    canonicalPath,
+                    (canonicalPathsToStrings?.get(canonicalPath) ?? new Set()).add(path),
+                );
+            }
+            const real = system.realpath(path);
+            (pathToReal ??= new Map()).set(path, real);
+            if (real === path) return;
+            const canonicalReal = getCanonicalFileName(real);
+            if (getCanonicalFileName(path) !== canonicalReal) {
+                (realToLinked ??= createMultiMap()).add(canonicalReal, path);
             }
         }
 
         function remove(path: string, data: Data) {
             actuals.remove(path, data);
-            if (!actuals.has(path)) {
-                const canonicalPath = getCanonicalFileName(path);
-                if (canonicalPath !== path) {
-                    const existing = canonicalPathsToStrings!.get(canonicalPath);
-                    if (existing!.size === 1) canonicalPathsToStrings!.delete(canonicalPath);
-                    else existing!.delete(path);
-                }
+            if (actuals.has(path)) return;
+            const canonicalPath = getCanonicalFileName(path);
+            if (canonicalPath !== path) {
+                const existing = canonicalPathsToStrings!.get(canonicalPath);
+                if (existing!.size === 1) canonicalPathsToStrings!.delete(canonicalPath);
+                else existing!.delete(path);
+            }
+            const real = pathToReal?.get(path)!;
+            pathToReal!.delete(path);
+            if (real === path) return;
+            const canonicalReal = getCanonicalFileName(real);
+            if (getCanonicalFileName(path) !== canonicalReal) {
+                realToLinked!.remove(canonicalReal, path);
             }
         }
 
-        function forEach(path: string, cb: (data: Data) => void) {
-            let allData: Data[] | undefined;
-            allData = addRange(allData, actuals.get(path));
+        function getAllData(path: string) {
+            let allData: Map<string, Data[]> | undefined;
+            addData(path);
             const canonicalPath = getCanonicalFileName(path);
-            if (canonicalPath !== path) allData = addRange(allData, actuals.get(canonicalPath));
+            if (canonicalPath !== path) addData(canonicalPath);
             canonicalPathsToStrings?.get(canonicalPath)?.forEach(canonicalSamePath => {
                 if (canonicalSamePath !== path && canonicalSamePath !== canonicalPath) {
-                    allData = addRange(allData, actuals.get(canonicalSamePath));
+                    addData(canonicalSamePath);
                 }
             });
-            allData?.forEach(cb);
+            return allData;
+            function addData(path: string) {
+                const data = actuals.get(path);
+                if (data) (allData ??= new Map()).set(path, data);
+            }
+        }
+
+        function forEach(path: string, cb: (data: Data, path: string) => void) {
+            const real = system.realpath(path);
+            const canonicalPath = getCanonicalFileName(path);
+            const canonicalReal = getCanonicalFileName(real);
+            let allData = canonicalPath === canonicalReal ? getAllData(path) : getAllData(real);
+            realToLinked?.get(canonicalReal)?.forEach(linked => {
+                if (allData?.has(linked)) return;
+                const data = actuals.get(linked);
+                if (data) (allData ??= new Map()).set(linked, data);
+            });
+            allData?.forEach((data, path) => data.forEach(d => cb(d, path)));
         }
 
         function serialize(baseline: string[]) {

src/testRunner/tests.ts

@@ -65,6 +65,7 @@ import "./unittests/services/preProcessFile";
 import "./unittests/services/textChanges";
 import "./unittests/services/transpile";
 import "./unittests/services/utilities";
+import "./unittests/sys/symlinkWatching";
 import "./unittests/tsbuild/amdModulesWithOut";
 import "./unittests/tsbuild/clean";
 import "./unittests/tsbuild/commandLine";

src/testRunner/unittests/helpers/tscWatch.ts

@@ -45,7 +45,6 @@ export interface TscWatchCompileChange<T extends ts.BuilderProgram = ts.EmitAndS
         watchOrSolution: WatchOrSolution<T>,
     ) => void;
     // TODO:: sheetal: Needing these fields are technically issues that need to be fixed later
-    symlinksNotReflected?: readonly string[];
     skipStructureCheck?: true;
 }
 export interface TscWatchCheckOptions {
@@ -220,7 +219,7 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
     });
 
     if (edits) {
-        for (const { caption, edit, timeouts, symlinksNotReflected, skipStructureCheck } of edits) {
+        for (const { caption, edit, timeouts, skipStructureCheck } of edits) {
             applyEdit(sys, baseline, edit, caption);
             timeouts(sys, programs, watchOrSolution);
             programs = watchBaseline({
@@ -233,7 +232,6 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
                 caption,
                 resolutionCache: !skipStructureCheck ? (watchOrSolution as ts.WatchOfConfigFile<T> | undefined)?.getResolutionCache?.() : undefined,
                 useSourceOfProjectReferenceRedirect,
-                symlinksNotReflected,
             });
         }
     }
@@ -254,7 +252,6 @@ export interface WatchBaseline extends BaselineBase, TscWatchCheckOptions {
     caption?: string;
     resolutionCache?: ts.ResolutionCache;
     useSourceOfProjectReferenceRedirect?: () => boolean;
-    symlinksNotReflected?: readonly string[];
 }
 export function watchBaseline({
     baseline,
@@ -266,7 +263,6 @@ export function watchBaseline({
     caption,
     resolutionCache,
     useSourceOfProjectReferenceRedirect,
-    symlinksNotReflected,
 }: WatchBaseline) {
     if (baselineSourceMap) generateSourceMapBaselineFiles(sys);
     const programs = getPrograms();
@@ -279,7 +275,13 @@ export function watchBaseline({
     // Verify program structure and resolution cache when incremental edit with tsc --watch (without build mode)
     if (resolutionCache && programs.length) {
         ts.Debug.assert(programs.length === 1);
-        verifyProgramStructureAndResolutionCache(caption!, sys, programs[0][0], resolutionCache, useSourceOfProjectReferenceRedirect, symlinksNotReflected);
+        verifyProgramStructureAndResolutionCache(
+            caption!,
+            sys,
+            programs[0][0],
+            resolutionCache,
+            useSourceOfProjectReferenceRedirect,
+        );
     }
     return programs;
 }
@@ -289,23 +291,12 @@ function verifyProgramStructureAndResolutionCache(
     program: ts.Program,
     resolutionCache: ts.ResolutionCache,
     useSourceOfProjectReferenceRedirect?: () => boolean,
-    symlinksNotReflected?: readonly string[],
 ) {
     const options = program.getCompilerOptions();
     const compilerHost = ts.createCompilerHostWorker(options, /*setParentNodes*/ undefined, sys);
     compilerHost.trace = ts.noop;
     compilerHost.writeFile = ts.notImplemented;
     compilerHost.useSourceOfProjectReferenceRedirect = useSourceOfProjectReferenceRedirect;
-    const readFile = compilerHost.readFile;
-    compilerHost.readFile = fileName => {
-        const text = readFile.call(compilerHost, fileName);
-        if (!ts.contains(symlinksNotReflected, fileName)) return text;
-        // Handle symlinks that dont reflect the watch change
-        ts.Debug.assert(sys.toPath(sys.realpath(fileName)) !== sys.toPath(fileName));
-        const file = program.getSourceFile(fileName)!;
-        ts.Debug.assert(file.text !== text);
-        return file.text;
-    };
     verifyProgramStructure(
         ts.createProgram({
             rootNames: program.getRootFileNames(),

src/testRunner/unittests/helpers/virtualFileSystemWithWatch.ts

@@ -378,7 +378,7 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
         this.environmentVariables = environmentVariables;
         currentDirectory = currentDirectory || "/";
         this.getCanonicalFileName = createGetCanonicalFileName(!!useCaseSensitiveFileNames);
-        this.watchUtils = createWatchUtils("PolledWatches", "FsWatches", s => this.getCanonicalFileName(s));
+        this.watchUtils = createWatchUtils("PolledWatches", "FsWatches", s => this.getCanonicalFileName(s), this);
         this.toPath = s => toPath(s, currentDirectory, this.getCanonicalFileName);
         this.executingFilePath = this.getHostSpecificPath(executingFilePath || getExecutingFilePathFromLibFile());
         this.currentDirectory = this.getHostSpecificPath(currentDirectory);
@@ -634,7 +634,7 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
         const inodeWatching = this.inodeWatching;
         if (options?.skipInodeCheckOnCreate) this.inodeWatching = false;
         this.invokeFileAndFsWatches(fileOrDirectory.fullPath, FileWatcherEventKind.Created, fileOrDirectory.modifiedTime, options?.useTildeAsSuffixInRenameEventFileName);
-        this.invokeFileAndFsWatches(folder.fullPath, FileWatcherEventKind.Changed, fileOrDirectory.modifiedTime, options?.useTildeAsSuffixInRenameEventFileName);
+        this.invokeFileAndFsWatches(folder.fullPath, FileWatcherEventKind.Changed, folder.modifiedTime, options?.useTildeAsSuffixInRenameEventFileName);
         this.inodeWatching = inodeWatching;
     }
 
@@ -658,14 +658,14 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
 
     deleteFile(filePath: string) {
         const path = this.toFullPath(filePath);
-        const currentEntry = this.fs.get(path) as FsFile;
+        const currentEntry = this.fs.get(path);
         Debug.assert(isFsFile(currentEntry));
         this.removeFileOrFolder(currentEntry);
     }
 
     deleteFolder(folderPath: string, recursive?: boolean) {
         const path = this.toFullPath(folderPath);
-        const currentEntry = this.fs.get(path) as FsFolder;
+        const currentEntry = this.fs.get(path);
         Debug.assert(isFsFolder(currentEntry));
         if (recursive && currentEntry.entries.length) {
             const subEntries = currentEntry.entries.slice();
@@ -688,7 +688,7 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
         );
     }
 
-    private fsWatchWorker(
+    fsWatchWorker(
         fileOrDirectory: string,
         recursive: boolean,
         cb: FsWatchCallback,
@@ -710,7 +710,7 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
     }
 
     invokeFileWatcher(fileFullPath: string, eventKind: FileWatcherEventKind, modifiedTime: Date | undefined) {
-        this.watchUtils.pollingWatches.forEach(fileFullPath, ({ cb }) => cb(fileFullPath, eventKind, modifiedTime));
+        this.watchUtils.pollingWatches.forEach(fileFullPath, ({ cb }, fullPath) => cb(fullPath, eventKind, modifiedTime));
     }
 
     private fsWatchCallback(watches: Watches<TestFsWatcher>, fullPath: string, eventName: "rename" | "change", modifiedTime: Date | undefined, entryFullPath: string | undefined, useTildeSuffix: boolean | undefined) {
@@ -822,6 +822,10 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
         return this.getRealFsEntry(isFsFolder, path, fsEntry);
     }
 
+    private getRealFileOrFolder(s: string): FsFile | FsFolder | undefined {
+        return this.getRealFsEntry((entry): entry is FsFile | FsFolder => !!entry && !isFsSymLink(entry), this.toFullPath(s));
+    }
+
     fileSystemEntryExists(s: string, entryKind: FileSystemEntryKind) {
         return entryKind === FileSystemEntryKind.File ? this.fileExists(s) : this.directoryExists(s);
     }
@@ -832,14 +836,11 @@ export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost,
     }
 
     getModifiedTime(s: string) {
-        const path = this.toFullPath(s);
-        const fsEntry = this.fs.get(path);
-        return (fsEntry && fsEntry.modifiedTime)!; // TODO: GH#18217
+        return this.getRealFileOrFolder(s)?.modifiedTime;
     }
 
     setModifiedTime(s: string, date: Date) {
-        const path = this.toFullPath(s);
-        const fsEntry = this.fs.get(path);
+        const fsEntry = this.getRealFileOrFolder(s);
         if (fsEntry) {
             fsEntry.modifiedTime = date;
             this.invokeFileAndFsWatches(fsEntry.fullPath, FileWatcherEventKind.Changed, fsEntry.modifiedTime);