Resolution is valid unless it is invalidated

Jakefile.js

@@ -121,7 +121,6 @@ var harnessSources = harnessCoreSources.concat([
     "transpile.ts",
     "reuseProgramStructure.ts",
     "textStorage.ts",
-    "cachingInServerLSHost.ts",
     "moduleResolution.ts",
     "tsconfigParsing.ts",
     "commandLineParsing.ts",

src/compiler/moduleNameResolver.ts

@@ -1127,21 +1127,4 @@ namespace ts {
     function toSearchResult<T>(value: T | undefined): SearchResult<T> {
         return value !== undefined ? { value } : undefined;
     }
-
-    /** Calls `callback` on `directory` and every ancestor directory it has, returning the first defined result. */
-    function forEachAncestorDirectory<T>(directory: string, callback: (directory: string) => SearchResult<T>): SearchResult<T> {
-        while (true) {
-            const result = callback(directory);
-            if (result !== undefined) {
-                return result;
-            }
-
-            const parentPath = getDirectoryPath(directory);
-            if (parentPath === directory) {
-                return undefined;
-            }
-
-            directory = parentPath;
-        }
-    }
 }

src/compiler/resolutionCache.ts

@@ -184,9 +184,9 @@ namespace ts {
 
             const seenNamesInFile = createMap<true>();
             for (const name of names) {
-                // check if this is a duplicate entry in the list
                 let resolution = resolutionsInFile.get(name);
-                if (!moduleResolutionIsValid(resolution, name)) {
+                // Resolution is valid if it is present and not invalidated
+                if (!resolution || resolution.isInvalidated) {
                     const existingResolution = resolution;
                     const resolutionInDirectory = perDirectoryResolution.get(name);
                     if (resolutionInDirectory) {
@@ -221,25 +221,6 @@ namespace ts {
 
             return resolvedModules;
 
-            function moduleResolutionIsValid(resolution: T, name: string): boolean {
-                // This is already calculated resolution in this round of synchronization
-                if (seenNamesInFile.has(name)) {
-                    return true;
-                }
-
-                if (!resolution || resolution.isInvalidated) {
-                    return false;
-                }
-
-                const result = getResult(resolution);
-                if (result) {
-                    return true;
-                }
-                // consider situation if we have no candidate locations as valid resolution.
-                // after all there is no point to invalidate it if we have no idea where to look for the module.
-                return resolution.failedLookupLocations.length === 0;
-            }
-
             function resolutionIsEqualTo(oldResolution: T, newResolution: T): boolean {
                 if (oldResolution === newResolution) {
                     return true;

src/compiler/utilities.ts

@@ -3609,6 +3609,23 @@ namespace ts {
     export function closeFileWatcherOf<T extends { watcher: FileWatcher; }>(objWithWatcher: T) {
         objWithWatcher.watcher.close();
     }
+
+    /** Calls `callback` on `directory` and every ancestor directory it has, returning the first defined result. */
+    export function forEachAncestorDirectory<T>(directory: string, callback: (directory: string) => T): T {
+        while (true) {
+            const result = callback(directory);
+            if (result !== undefined) {
+                return result;
+            }
+
+            const parentPath = getDirectoryPath(directory);
+            if (parentPath === directory) {
+                return undefined;
+            }
+
+            directory = parentPath;
+        }
+    }
 }
 
 namespace ts {

src/harness/tsconfig.json

@@ -109,7 +109,6 @@
         "./unittests/convertToBase64.ts",
         "./unittests/transpile.ts",
         "./unittests/reuseProgramStructure.ts",
-        "./unittests/cachingInServerLSHost.ts",
         "./unittests/moduleResolution.ts",
         "./unittests/tsconfigParsing.ts",
         "./unittests/commandLineParsing.ts",

src/harness/unittests/cachingInServerLSHost.ts

@@ -1,203 +0,0 @@
-/// <reference path="..\harness.ts" />
-
-namespace ts {
-    interface File {
-        name: string;
-        content: string;
-    }
-
-    function createDefaultServerHost(fileMap: Map<File>): server.ServerHost {
-        const existingDirectories = createMap<boolean>();
-        forEachKey(fileMap, name => {
-            let dir = getDirectoryPath(name);
-            let previous: string;
-            do {
-                existingDirectories.set(dir, true);
-                previous = dir;
-                dir = getDirectoryPath(dir);
-            } while (dir !== previous);
-        });
-        return {
-            args: <string[]>[],
-            newLine: "\r\n",
-            useCaseSensitiveFileNames: false,
-            write: noop,
-            readFile: path => {
-                const file = fileMap.get(path);
-                return file && file.content;
-            },
-            writeFile: notImplemented,
-            resolvePath: notImplemented,
-            fileExists: path => fileMap.has(path),
-            directoryExists: path => existingDirectories.get(path) || false,
-            createDirectory: noop,
-            getExecutingFilePath: () => "",
-            getCurrentDirectory: () => "",
-            getDirectories: () => [],
-            getEnvironmentVariable: () => "",
-            readDirectory: notImplemented,
-            exit: noop,
-            watchFile: () => ({
-                close: noop
-            }),
-            watchDirectory: () => ({
-                close: noop
-            }),
-            setTimeout,
-            clearTimeout,
-            setImmediate: typeof setImmediate !== "undefined" ? setImmediate : action => setTimeout(action, 0),
-            clearImmediate: typeof clearImmediate !== "undefined" ? clearImmediate : clearTimeout,
-            createHash: Harness.mockHash,
-        };
-    }
-
-    function createProject(rootFile: string, serverHost: server.ServerHost): { project: server.Project, rootScriptInfo: server.ScriptInfo } {
-        const svcOpts: server.ProjectServiceOptions = {
-            host: serverHost,
-            logger: projectSystem.nullLogger,
-            cancellationToken: { isCancellationRequested: () => false },
-            useSingleInferredProject: false,
-            useInferredProjectPerProjectRoot: false,
-            typingsInstaller: undefined
-        };
-        const projectService = new server.ProjectService(svcOpts);
-        const rootScriptInfo = projectService.getOrCreateScriptInfoForNormalizedPath(server.toNormalizedPath(rootFile), /*openedByClient*/ true);
-
-        const project = projectService.assignOrphanScriptInfoToInferredProject(rootScriptInfo);
-        project.setCompilerOptions({ module: ts.ModuleKind.AMD, noLib: true } );
-        return {
-            project,
-            rootScriptInfo
-        };
-    }
-
-    describe("Caching in LSHost", () => {
-        it("works using legacy resolution logic", () => {
-            const root: File = {
-                name: "c:/d/f0.ts",
-                content: `import {x} from "f1"`
-            };
-
-            const imported: File = {
-                name: "c:/f1.ts",
-                content: `foo()`
-            };
-
-            const serverHost = createDefaultServerHost(createMapFromTemplate({ [root.name]: root, [imported.name]: imported }));
-            const { project, rootScriptInfo } = createProject(root.name, serverHost);
-
-            // ensure that imported file was found
-            let diags = project.getLanguageService().getSemanticDiagnostics(imported.name);
-            assert.equal(diags.length, 1);
-
-
-            const originalFileExists = serverHost.fileExists;
-            {
-                // patch fileExists to make sure that disk is not touched
-                serverHost.fileExists = notImplemented;
-
-                const newContent = `import {x} from "f1"
-                var x: string = 1;`;
-                rootScriptInfo.editContent(0, root.content.length, newContent);
-                // trigger synchronization to make sure that import will be fetched from the cache
-                diags = project.getLanguageService().getSemanticDiagnostics(imported.name);
-                // ensure file has correct number of errors after edit
-                assert.equal(diags.length, 1);
-            }
-            {
-                let fileExistsIsCalled = false;
-                serverHost.fileExists = (fileName): boolean => {
-                    if (fileName === "lib.d.ts") {
-                        return false;
-                    }
-                    fileExistsIsCalled = true;
-                    assert.isTrue(fileName.indexOf("/f2.") !== -1);
-                    return originalFileExists.call(serverHost, fileName);
-                };
-                const newContent = `import {x} from "f2"`;
-                rootScriptInfo.editContent(0, root.content.length, newContent);
-
-                try {
-                    // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk
-                    project.getLanguageService().getSemanticDiagnostics(imported.name);
-                    assert.isTrue(false, `should not find file '${imported.name}'`);
-                }
-                catch (e) {
-                    assert.isTrue(e.message.indexOf(`Could not find file: '${imported.name}'.`) === 0);
-                }
-
-                assert.isTrue(fileExistsIsCalled);
-            }
-            {
-                let fileExistsCalled = false;
-                serverHost.fileExists = (fileName): boolean => {
-                    if (fileName === "lib.d.ts") {
-                        return false;
-                    }
-                    fileExistsCalled = true;
-                    assert.isTrue(fileName.indexOf("/f1.") !== -1);
-                    return originalFileExists.call(serverHost, fileName);
-                };
-
-                const newContent = `import {x} from "f1"`;
-                rootScriptInfo.editContent(0, root.content.length, newContent);
-                project.getLanguageService().getSemanticDiagnostics(imported.name);
-                assert.isTrue(fileExistsCalled);
-
-                // setting compiler options discards module resolution cache
-                fileExistsCalled = false;
-
-                const compilerOptions = ts.cloneCompilerOptions(project.getCompilationSettings());
-                compilerOptions.target = ts.ScriptTarget.ES5;
-                project.setCompilerOptions(compilerOptions);
-
-                project.getLanguageService().getSemanticDiagnostics(imported.name);
-                assert.isTrue(fileExistsCalled);
-            }
-        });
-
-        it("loads missing files from disk", () => {
-            const root: File = {
-                name: `c:/foo.ts`,
-                content: `import {x} from "bar"`
-            };
-
-            const imported: File = {
-                name: `c:/bar.d.ts`,
-                content: `export var y = 1`
-            };
-
-            const fileMap = createMapFromTemplate({ [root.name]: root });
-            const serverHost = createDefaultServerHost(fileMap);
-            const originalFileExists = serverHost.fileExists;
-
-            let fileExistsCalledForBar = false;
-            serverHost.fileExists = fileName => {
-                if (fileName === "lib.d.ts") {
-                    return false;
-                }
-                if (!fileExistsCalledForBar) {
-                    fileExistsCalledForBar = fileName.indexOf("/bar.") !== -1;
-                }
-
-                return originalFileExists.call(serverHost, fileName);
-            };
-
-            const { project, rootScriptInfo } = createProject(root.name, serverHost);
-
-            let diags = project.getLanguageService().getSemanticDiagnostics(root.name);
-            assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called");
-            assert.isTrue(diags.length === 1, "one diagnostic expected");
-            const messageText = diags[0].messageText;
-            assert.isTrue(isString(messageText) && messageText.indexOf("Cannot find module") === 0, "should be 'cannot find module' message");
-
-            fileMap.set(imported.name, imported);
-            fileExistsCalledForBar = false;
-            rootScriptInfo.editContent(0, root.content.length, `import {y} from "bar"`);
-
-            diags = project.getLanguageService().getSemanticDiagnostics(root.name);
-            assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called.");
-            assert.isTrue(diags.length === 0, "The import should succeed once the imported file appears on disk.");
-        });
-    });
-}

src/harness/unittests/tsserverProjectSystem.ts

@@ -249,14 +249,9 @@ namespace ts.projectSystem {
 
     function getNodeModuleDirectories(dir: string) {
         const result: string[] = [];
-        while (true) {
-            result.push(combinePaths(dir, "node_modules"));
-            const parentDir = getDirectoryPath(dir);
-            if (parentDir === dir) {
-                break;
-            }
-            dir = parentDir;
-        }
+        forEachAncestorDirectory(dir, ancestor => {
+            result.push(combinePaths(ancestor, "node_modules"));
+        });
         return result;
     }
 
@@ -343,14 +338,9 @@ namespace ts.projectSystem {
     export function getTypeRootsFromLocation(currentDirectory: string) {
         currentDirectory = normalizePath(currentDirectory);
         const result: string[] = [];
-        while (true) {
-            result.push(combinePaths(currentDirectory, "node_modules/@types"));
-            const parentDirectory = getDirectoryPath(currentDirectory);
-            if (parentDirectory === currentDirectory) {
-                break;
-            }
-            currentDirectory = parentDirectory;
-        }
+        forEachAncestorDirectory(currentDirectory, ancestor => {
+            result.push(combinePaths(ancestor, "node_modules/@types"));
+        });
         return result;
     }
 
@@ -4147,6 +4137,7 @@ namespace ts.projectSystem {
                 verifyCalledOnEachEntry,
                 verifyNoHostCalls,
                 verifyNoHostCallsExceptFileExistsOnce,
+                verifyCalledOn,
                 clear
             };
 
@@ -4179,6 +4170,12 @@ namespace ts.projectSystem {
                 }
             }
 
+            function verifyCalledOn(callback: keyof CalledMaps, name: string) {
+                const calledMap = calledMaps[callback];
+                const result = calledMap.get(name);
+                assert.isTrue(result && !!result.length, `${callback} should be called with name: ${name}: ${arrayFrom(calledMap.keys())}`);
+            }
+
             function verifyNoCall(callback: keyof CalledMaps) {
                 const calledMap = calledMaps[callback];
                 assert.equal(calledMap.size, 0, `${callback} shouldnt be called: ${arrayFrom(calledMap.keys())}`);
@@ -4218,6 +4215,161 @@ namespace ts.projectSystem {
             }
         }
 
+        it("works using legacy resolution logic", () => {
+            let rootContent = `import {x} from "f1"`;
+            const root: FileOrFolder = {
+                path: "/c/d/f0.ts",
+                content: rootContent
+            };
+
+            const imported: FileOrFolder = {
+                path: "/c/f1.ts",
+                content: `foo()`
+            };
+
+            const host = createServerHost([root, imported]);
+            const projectService = createProjectService(host);
+            projectService.setCompilerOptionsForInferredProjects({ module: ts.ModuleKind.AMD, noLib: true });
+            projectService.openClientFile(root.path);
+            checkNumberOfProjects(projectService, { inferredProjects: 1 });
+            const project = projectService.inferredProjects[0];
+            const rootScriptInfo = project.getRootScriptInfos()[0];
+            assert.equal(rootScriptInfo.fileName, root.path);
+
+            // ensure that imported file was found
+            verifyImportedDiagnostics();
+
+            const callsTrackingHost = createCallsTrackingHost(host);
+
+            // trigger synchronization to make sure that import will be fetched from the cache
+            // ensure file has correct number of errors after edit
+            editContent(`import {x} from "f1";
+                 var x: string = 1;`);
+            verifyImportedDiagnostics();
+            callsTrackingHost.verifyNoHostCalls();
+
+            // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk
+            editContent(`import {x} from "f2"`);
+            try {
+                // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk
+                verifyImportedDiagnostics();
+                assert.isTrue(false, `should not find file '${imported.path}'`);
+            }
+            catch (e) {
+                assert.isTrue(e.message.indexOf(`Could not find file: '${imported.path}'.`) === 0);
+            }
+            const f2Lookups = getLocationsForModuleLookup("f2");
+            callsTrackingHost.verifyCalledOnEachEntryNTimes("fileExists", f2Lookups, 1);
+            const f2DirLookups = getLocationsForDirectoryLookup();
+            callsTrackingHost.verifyCalledOnEachEntry("directoryExists", f2DirLookups);
+            callsTrackingHost.verifyNoCall("getDirectories");
+            callsTrackingHost.verifyNoCall("readFile");
+            callsTrackingHost.verifyNoCall("readDirectory");
+
+            editContent(`import {x} from "f1"`);
+            verifyImportedDiagnostics();
+            const f1Lookups = f2Lookups.map(s => s.replace("f2", "f1"));
+            f1Lookups.length = f1Lookups.indexOf(imported.path) + 1;
+            const f1DirLookups = ["/c/d", "/c", typeRootFromTsserverLocation];
+            vertifyF1Lookups();
+
+            // setting compiler options discards module resolution cache
+            callsTrackingHost.clear();
+            projectService.setCompilerOptionsForInferredProjects({ module: ts.ModuleKind.AMD, noLib: true, target: ts.ScriptTarget.ES5 });
+            verifyImportedDiagnostics();
+            vertifyF1Lookups();
+
+            function vertifyF1Lookups() {
+                callsTrackingHost.verifyCalledOnEachEntryNTimes("fileExists", f1Lookups, 1);
+                callsTrackingHost.verifyCalledOnEachEntryNTimes("directoryExists", f1DirLookups, 1);
+                callsTrackingHost.verifyNoCall("getDirectories");
+                callsTrackingHost.verifyNoCall("readFile");
+                callsTrackingHost.verifyNoCall("readDirectory");
+            }
+
+            function editContent(newContent: string) {
+                callsTrackingHost.clear();
+                rootScriptInfo.editContent(0, rootContent.length, newContent);
+                rootContent = newContent;
+            }
+
+            function verifyImportedDiagnostics() {
+                const diags = project.getLanguageService().getSemanticDiagnostics(imported.path);
+                assert.equal(diags.length, 1);
+                const diag = diags[0];
+                assert.equal(diag.code, Diagnostics.Cannot_find_name_0.code);
+                assert.equal(flattenDiagnosticMessageText(diag.messageText, "\n"), "Cannot find name 'foo'.");
+            }
+
+            function getLocationsForModuleLookup(module: string) {
+                const locations: string[] = [];
+                forEachAncestorDirectory(getDirectoryPath(root.path), ancestor => {
+                    locations.push(
+                        combinePaths(ancestor, `${module}.ts`),
+                        combinePaths(ancestor, `${module}.tsx`),
+                        combinePaths(ancestor, `${module}.d.ts`)
+                    );
+                });
+                forEachAncestorDirectory(getDirectoryPath(root.path), ancestor => {
+                    locations.push(
+                        combinePaths(ancestor, `${module}.js`),
+                        combinePaths(ancestor, `${module}.jsx`)
+                    );
+                });
+                return locations;
+            }
+
+            function getLocationsForDirectoryLookup() {
+                const result = createMap<number>();
+                // Type root
+                result.set(typeRootFromTsserverLocation, 1);
+                forEachAncestorDirectory(getDirectoryPath(root.path), ancestor => {
+                    // To resolve modules
+                    result.set(ancestor, 2);
+                    // for type roots
+                    result.set(combinePaths(ancestor, `node_modules`), 1);
+                });
+                return result;
+            }
+        });
+
+        it("loads missing files from disk", () => {
+            const root: FileOrFolder = {
+                path: "/c/foo.ts",
+                content: `import {y} from "bar"`
+            };
+
+            const imported: FileOrFolder = {
+                path: "/c/bar.d.ts",
+                content: `export var y = 1`
+            };
+
+            const host = createServerHost([root]);
+            const projectService = createProjectService(host);
+            projectService.setCompilerOptionsForInferredProjects({ module: ts.ModuleKind.AMD, noLib: true });
+            const callsTrackingHost = createCallsTrackingHost(host);
+            projectService.openClientFile(root.path);
+            checkNumberOfProjects(projectService, { inferredProjects: 1 });
+            const project = projectService.inferredProjects[0];
+            const rootScriptInfo = project.getRootScriptInfos()[0];
+            assert.equal(rootScriptInfo.fileName, root.path);
+
+            let diags = project.getLanguageService().getSemanticDiagnostics(root.path);
+            assert.equal(diags.length, 1);
+            const diag = diags[0];
+            assert.equal(diag.code, Diagnostics.Cannot_find_module_0.code);
+            assert.equal(flattenDiagnosticMessageText(diag.messageText, "\n"), "Cannot find module 'bar'.");
+            callsTrackingHost.verifyCalledOn("fileExists", imported.path);
+
+
+            callsTrackingHost.clear();
+            host.reloadFS([root, imported]);
+            host.runQueuedTimeoutCallbacks();
+            diags = project.getLanguageService().getSemanticDiagnostics(root.path);
+            assert.equal(diags.length, 0);
+            callsTrackingHost.verifyCalledOn("fileExists", imported.path);
+        });
+
         it("when calling goto definition of module", () => {
             const clientFile: FileOrFolder = {
                 path: "/a/b/controllers/vessels/client.ts",
@@ -4371,18 +4523,7 @@ namespace ts.projectSystem {
                 const canonicalFile3Path = useCaseSensitiveFileNames ? file3.path : file3.path.toLocaleLowerCase();
                 const numberOfTimesWatchInvoked = getNumberOfWatchesInvokedForRecursiveWatches(watchingRecursiveDirectories, canonicalFile3Path);
                 callsTrackingHost.verifyCalledOnEachEntryNTimes("fileExists", [canonicalFile3Path], numberOfTimesWatchInvoked);
-
-                // Called for type root resolution
-                const directoryExistsCalled = createMap<number>();
-                for (let dir = frontendDir; dir !== "/"; dir = getDirectoryPath(dir)) {
-                    directoryExistsCalled.set(`${dir}/node_modules`, 2);
-                }
-                directoryExistsCalled.set(`/node_modules`, 2);
-                directoryExistsCalled.set(`${frontendDir}/types`, 2);
-                directoryExistsCalled.set(`${frontendDir}/node_modules/@types`, 2);
-                directoryExistsCalled.set(canonicalFile3Path, numberOfTimesWatchInvoked);
-                callsTrackingHost.verifyCalledOnEachEntry("directoryExists", directoryExistsCalled);
-
+                callsTrackingHost.verifyCalledOnEachEntryNTimes("directoryExists", [canonicalFile3Path], numberOfTimesWatchInvoked);
                 callsTrackingHost.verifyNoCall("getDirectories");
                 callsTrackingHost.verifyCalledOnEachEntryNTimes("readFile", [file3.path], 1);
                 callsTrackingHost.verifyNoCall("readDirectory");