Handle getScriptVersion correctly to ensure program structure is checked correctly (#36808)

* Fix tests when there are project references but has disableSourceOfProjectReferenceRedirect

* Handle getScriptVersion correctly to ensure program structure is checked correctly
Fixes #36748

* Harness's language service host doesnt have getProjectVersion.
This means earlier we were creating fresh program everytime we did LS operation
Now we reuse same program, so quick info depends on order of quickinfo demands

* Because same program is used, it unvails a bug that if `export=` is evaluated before finding references, it cant find all definitions from the merge

* Update src/server/project.ts

Co-Authored-By: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>

* Make clearSourceMapperCache required

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>

src/compiler/program.ts

@@ -553,7 +553,7 @@ namespace ts {
         program: Program | undefined,
         rootFileNames: string[],
         newOptions: CompilerOptions,
-        getSourceVersion: (path: Path) => string | undefined,
+        getSourceVersion: (path: Path, fileName: string) => string | undefined,
         fileExists: (fileName: string) => boolean,
         hasInvalidatedResolution: HasInvalidatedResolution,
         hasChangedAutomaticTypeDirectiveNames: boolean,
@@ -606,7 +606,7 @@ namespace ts {
         }
 
         function sourceFileVersionUptoDate(sourceFile: SourceFile) {
-            return sourceFile.version === getSourceVersion(sourceFile.resolvedPath);
+            return sourceFile.version === getSourceVersion(sourceFile.resolvedPath, sourceFile.fileName);
         }
 
         function projectReferenceUptoDate(oldRef: ProjectReference, newRef: ProjectReference, index: number) {

src/harness/client.ts

@@ -808,6 +808,10 @@ namespace ts.server {
             return notImplemented();
         }
 
+        clearSourceMapperCache(): never {
+            return notImplemented();
+        }
+
         dispose(): void {
             throw new Error("dispose is not available through the server layer.");
         }

src/harness/harnessLanguageService.ts

@@ -598,6 +598,9 @@ namespace Harness.LanguageService {
         getSourceMapper(): never {
             return ts.notImplemented();
         }
+        clearSourceMapperCache(): never {
+            return ts.notImplemented();
+        }
         dispose(): void { this.shim.dispose({}); }
     }
 

src/server/editorServices.ts

@@ -873,9 +873,11 @@ namespace ts.server {
             this.delayEnsureProjectForOpenFiles();
         }
 
-        private delayUpdateProjectGraphs(projects: readonly Project[]) {
+        private delayUpdateProjectGraphs(projects: readonly Project[], clearSourceMapperCache: boolean) {
             if (projects.length) {
                 for (const project of projects) {
+                    // Even if program doesnt change, clear the source mapper cache
+                    if (clearSourceMapperCache) project.clearSourceMapperCache();
                     this.delayUpdateProjectGraph(project);
                 }
                 this.delayEnsureProjectForOpenFiles();
@@ -1033,7 +1035,7 @@ namespace ts.server {
                     // file has been changed which might affect the set of referenced files in projects that include
                     // this file and set of inferred projects
                     info.delayReloadNonMixedContentFile();
-                    this.delayUpdateProjectGraphs(info.containingProjects);
+                    this.delayUpdateProjectGraphs(info.containingProjects, /*clearSourceMapperCache*/ false);
                     this.handleSourceMapProjects(info);
                 }
             }
@@ -1066,7 +1068,7 @@ namespace ts.server {
         private delayUpdateProjectsOfScriptInfoPath(path: Path) {
             const info = this.getScriptInfoForPath(path);
             if (info) {
-                this.delayUpdateProjectGraphs(info.containingProjects);
+                this.delayUpdateProjectGraphs(info.containingProjects, /*clearSourceMapperCache*/ true);
             }
         }
 
@@ -1082,7 +1084,7 @@ namespace ts.server {
                 info.detachAllProjects();
 
                 // update projects to make sure that set of referenced files is correct
-                this.delayUpdateProjectGraphs(containingProjects);
+                this.delayUpdateProjectGraphs(containingProjects, /*clearSourceMapperCache*/ false);
                 this.handleSourceMapProjects(info);
                 info.closeSourceMapFileWatcher();
                 // need to recalculate source map from declaration file
@@ -2537,7 +2539,7 @@ namespace ts.server {
                     const declarationInfo = this.getScriptInfoForPath(declarationInfoPath);
                     if (declarationInfo && declarationInfo.sourceMapFilePath && !isString(declarationInfo.sourceMapFilePath)) {
                         // Update declaration and source projects
-                        this.delayUpdateProjectGraphs(declarationInfo.containingProjects);
+                        this.delayUpdateProjectGraphs(declarationInfo.containingProjects, /*clearSourceMapperCache*/ true);
                         this.delayUpdateSourceInfoProjects(declarationInfo.sourceMapFilePath.sourceInfos);
                         declarationInfo.closeSourceMapFileWatcher();
                     }

src/server/project.ts

@@ -384,7 +384,9 @@ namespace ts.server {
         }
 
         getScriptVersion(filename: string) {
-            const info = this.getOrCreateScriptInfoAndAttachToProject(filename);
+            // Don't attach to the project if version is asked
+
+            const info = this.projectService.getOrCreateScriptInfoNotOpenedByClient(filename, this.currentDirectory, this.directoryStructureHost);
             return (info && info.getLatestVersion())!; // TODO: GH#18217
         }
 
@@ -558,6 +560,11 @@ namespace ts.server {
             return this.getLanguageService().getSourceMapper();
         }
 
+        /** @internal */
+        clearSourceMapperCache() {
+            this.languageService.clearSourceMapperCache();
+        }
+
         /*@internal*/
         getDocumentPositionMapper(generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined {
             return this.projectService.getDocumentPositionMapper(this, generatedFileName, sourceFileName);
@@ -1224,7 +1231,10 @@ namespace ts.server {
                 watcher: this.projectService.watchFactory.watchFile(
                     this.projectService.host,
                     generatedFile,
-                    () => this.projectService.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(this),
+                    () => {
+                        this.clearSourceMapperCache();
+                        this.projectService.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(this);
+                    },
                     PollingInterval.High,
                     this.projectService.getWatchOptions(this),
                     WatchType.MissingGeneratedFile,

src/server/scriptInfo.ts

@@ -557,6 +557,8 @@ namespace ts.server {
         }
 
         getLatestVersion() {
+            // Ensure we have updated snapshot to give back latest version
+            this.textStorage.getSnapshot();
             return this.textStorage.getVersion();
         }
 

src/services/services.ts

@@ -980,11 +980,6 @@ namespace ts {
             return names;
         }
 
-        public getVersion(path: Path): string {
-            const file = this.getHostFileInformation(path);
-            return (file && file.version)!; // TODO: GH#18217
-        }
-
         public getScriptSnapshot(path: Path): IScriptSnapshot {
             const file = this.getHostFileInformation(path);
             return (file && file.scriptSnapshot)!; // TODO: GH#18217
@@ -1228,7 +1223,7 @@ namespace ts {
             const projectReferences = hostCache.getProjectReferences();
 
             // If the program is already up-to-date, we can reuse it
-            if (isProgramUptoDate(program, rootFileNames, hostCache.compilationSettings(), path => hostCache!.getVersion(path), fileExists, hasInvalidatedResolution, !!host.hasChangedAutomaticTypeDirectiveNames, projectReferences)) {
+            if (isProgramUptoDate(program, rootFileNames, hostCache.compilationSettings(), (_path, fileName) => host.getScriptVersion(fileName), fileExists, hasInvalidatedResolution, !!host.hasChangedAutomaticTypeDirectiveNames, projectReferences)) {
                 return;
             }
 
@@ -2227,6 +2222,7 @@ namespace ts {
             getEditsForRefactor,
             toLineColumnOffset: sourceMapper.toLineColumnOffset,
             getSourceMapper: () => sourceMapper,
+            clearSourceMapperCache: () => sourceMapper.clearCache(),
             prepareCallHierarchy,
             provideCallHierarchyIncomingCalls,
             provideCallHierarchyOutgoingCalls

src/services/types.ts

@@ -382,6 +382,8 @@ namespace ts {
         toLineColumnOffset?(fileName: string, position: number): LineAndCharacter;
         /** @internal */
         getSourceMapper(): SourceMapper;
+        /** @internal */
+        clearSourceMapperCache(): void;
 
         getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: readonly number[], formatOptions: FormatCodeSettings, preferences: UserPreferences): readonly CodeFixAction[];
         getCombinedCodeFix(scope: CombinedCodeFixScope, fixId: {}, formatOptions: FormatCodeSettings, preferences: UserPreferences): CombinedCodeActions;

src/testRunner/unittests/services/languageService.ts

@@ -80,5 +80,61 @@ export function Component(x: Config): any;`
                 }
             );
         });
+
+        describe("detects program upto date correctly", () => {
+            function verifyProgramUptoDate(useProjectVersion: boolean) {
+                let projectVersion = "1";
+                const files = createMap<{ version: string, text: string; }>();
+                files.set("/project/root.ts", { version: "1", text: `import { foo } from "./other"` });
+                files.set("/project/other.ts", { version: "1", text: `export function foo() { }` });
+                files.set("/lib/lib.d.ts", { version: "1", text: projectSystem.libFile.content });
+                const host: LanguageServiceHost = {
+                    useCaseSensitiveFileNames: returnTrue,
+                    getCompilationSettings: getDefaultCompilerOptions,
+                    fileExists: path => files.has(path),
+                    getProjectVersion: !useProjectVersion ? undefined : () => projectVersion,
+                    getScriptFileNames: () => ["/project/root.ts"],
+                    getScriptVersion: path => files.get(path)?.version || "",
+                    getScriptSnapshot: path => {
+                        const text = files.get(path)?.text;
+                        return text ? ScriptSnapshot.fromString(text) : undefined;
+                    },
+                    getCurrentDirectory: () => "/project",
+                    getDefaultLibFileName: () => "/lib/lib.d.ts"
+                };
+                const ls = ts.createLanguageService(host);
+                const program1 = ls.getProgram()!;
+                const program2 = ls.getProgram()!;
+                assert.strictEqual(program1, program2);
+                verifyProgramFiles(program1);
+
+                // Change other
+                projectVersion = "2";
+                files.set("/project/other.ts", { version: "2", text: `export function foo() { } export function bar() { }` });
+                const program3 = ls.getProgram()!;
+                assert.notStrictEqual(program2, program3);
+                verifyProgramFiles(program3);
+
+                // change root
+                projectVersion = "3";
+                files.set("/project/root.ts", { version: "2", text: `import { foo, bar } from "./other"` });
+                const program4 = ls.getProgram()!;
+                assert.notStrictEqual(program3, program4);
+                verifyProgramFiles(program4);
+
+                function verifyProgramFiles(program: Program) {
+                    assert.deepEqual(
+                        program.getSourceFiles().map(f => f.fileName),
+                        ["/lib/lib.d.ts", "/project/other.ts", "/project/root.ts"]
+                    );
+                }
+            }
+            it("when host implements getProjectVersion", () => {
+                verifyProgramUptoDate(/*useProjectVersion*/ true);
+            });
+            it("when host does not implement getProjectVersion", () => {
+                verifyProgramUptoDate(/*useProjectVersion*/ false);
+            });
+        });
     });
 }

src/testRunner/unittests/tsserver/projectReferences.ts

@@ -470,7 +470,7 @@ fn5();
             interface VerifierAndWithRefs {
                 withRefs: boolean;
                 disableSourceOfProjectReferenceRedirect?: true;
-                verifier: (withRefs: boolean) => readonly DocumentPositionMapperVerifier[];
+                verifier: (withRefs: boolean, disableSourceOfProjectReferenceRedirect?: true) => readonly DocumentPositionMapperVerifier[];
             }
 
             function openFiles(verifiers: readonly DocumentPositionMapperVerifier[]) {
@@ -502,7 +502,7 @@ fn5();
                     onHostCreate(host);
                 }
                 const session = createSession(host);
-                const verifiers = verifier(withRefs && !disableSourceOfProjectReferenceRedirect);
+                const verifiers = verifier(withRefs && !disableSourceOfProjectReferenceRedirect, disableSourceOfProjectReferenceRedirect);
                 openFilesForSession([...openFiles(verifiers), randomFile], session);
                 return { host, session, verifiers };
             }
@@ -724,13 +724,14 @@ fn5();
                     scenarioName,
                     verifier,
                     withRefs,
+                    disableSourceOfProjectReferenceRedirect,
                     change,
                     afterChangeActionKey
                 }: VerifyScenarioWithChanges,
                 timeoutBeforeAction: boolean,
             ) {
                 it(scenarioName, () => {
-                    const { host, session, verifiers } = openTsFile({ verifier, withRefs });
+                    const { host, session, verifiers } = openTsFile({ verifier, withRefs, disableSourceOfProjectReferenceRedirect });
 
                     // Create DocumentPositionMapper
                     firstAction(session, verifiers);
@@ -790,6 +791,7 @@ fn5();
                 scenarioName,
                 verifier,
                 withRefs,
+                disableSourceOfProjectReferenceRedirect,
                 fileLocation,
                 fileNotPresentKey,
                 fileCreatedKey,
@@ -801,6 +803,7 @@ fn5();
                         const { host, session, verifiers } = openTsFile({
                             verifier,
                             withRefs,
+                            disableSourceOfProjectReferenceRedirect,
                             onHostCreate: host => host.deleteFile(fileLocation)
                         });
                         checkProject(session, verifiers, noDts);
@@ -813,6 +816,7 @@ fn5();
                         const { host, session, verifiers } = openTsFile({
                             verifier,
                             withRefs,
+                            disableSourceOfProjectReferenceRedirect,
                             onHostCreate: host => {
                                 fileContents = host.readFile(fileLocation);
                                 host.deleteFile(fileLocation);
@@ -825,7 +829,7 @@ fn5();
                     });
 
                     it("when file is deleted after actions on the projects", () => {
-                        const { host, session, verifiers } = openTsFile({ verifier, withRefs });
+                        const { host, session, verifiers } = openTsFile({ verifier, disableSourceOfProjectReferenceRedirect, withRefs });
                         firstAction(session, verifiers);
 
                         // The dependency file is deleted when orphan files are collected
@@ -967,31 +971,35 @@ ${dependencyTs.content}`);
 
             interface VerifyScenario {
                 mainScenario: string;
-                verifier: (withRefs: boolean) => readonly DocumentPositionMapperVerifier[];
+                verifier: (withRefs: boolean, disableSourceOfProjectReferenceRedirect?: true) => readonly DocumentPositionMapperVerifier[];
             }
             function verifyScenario(scenario: VerifyScenario) {
-                describe("when main tsconfig doesnt have project reference", () => {
-                    verifyScenarioWorker(scenario, /*withRefs*/ false);
-                });
-                describe("when main tsconfig has project reference", () => {
-                    verifyScenarioWorker(scenario, /*withRefs*/ true);
-                });
-                describe("when main tsconfig has but has disableSourceOfProjectReferenceRedirect", () => {
-                    verifyScenarioWorker(scenario, /*withRefs*/ true);
+                describe(scenario.mainScenario, () => {
+                    describe("when main tsconfig doesnt have project reference", () => {
+                        verifyScenarioWorker(scenario, /*withRefs*/ false);
+                    });
+                    describe("when main tsconfig has project reference", () => {
+                        verifyScenarioWorker(scenario, /*withRefs*/ true);
+                    });
+                    describe("when main tsconfig has disableSourceOfProjectReferenceRedirect along with project reference", () => {
+                        verifyScenarioWorker(scenario, /*withRefs*/ true, /*disableSourceOfProjectReferenceRedirect*/ true);
+                    });
                 });
             }
 
             describe("from project that uses dependency", () => {
                 verifyScenario({
                     mainScenario: "can go to definition correctly",
-                    verifier: withRefs => [
+                    verifier: (withRefs, disableSourceOfProjectReferenceRedirect) => [
                         {
                             ...goToDefFromMainTsProjectInfoVerifier(withRefs),
                             main: () => ({
                                 action: goToDefFromMainTs,
                                 closedInfos: withRefs ?
                                     [dependencyTs.path, dependencyConfig.path, libFile.path] :
-                                    [dependencyTs.path, libFile.path, dtsPath, dtsMapLocation],
+                                    disableSourceOfProjectReferenceRedirect ?
+                                        [dependencyTs.path, libFile.path, dtsPath, dtsMapLocation, dependencyConfig.path] :
+                                        [dependencyTs.path, libFile.path, dtsPath, dtsMapLocation],
                                 otherWatchedFiles: [mainConfig.path],
                                 expectsDts: !withRefs, // Dts script info present only if no project reference
                                 expectsMap: !withRefs // Map script info present only if no project reference
@@ -1097,7 +1105,7 @@ ${dependencyTs.content}`);
             describe("when opening depedency and usage project", () => {
                 verifyScenario({
                     mainScenario: "goto Definition in usage and rename locations from defining project",
-                    verifier: withRefs => [
+                    verifier: (withRefs, disableSourceOfProjectReferenceRedirect) => [
                         {
                             ...goToDefFromMainTsProjectInfoVerifier(withRefs),
                             main: () => ({
@@ -1105,9 +1113,11 @@ ${dependencyTs.content}`);
                                 // DependencyTs is open, so omit it from closed infos
                                 closedInfos: withRefs ?
                                     [dependencyConfig.path, libFile.path] :
-                                    [libFile.path, dtsPath, dtsMapLocation],
-                                otherWatchedFiles: withRefs ?
-                                    [mainConfig.path] : // Its in closed info
+                                    disableSourceOfProjectReferenceRedirect ?
+                                        [libFile.path, dtsPath, dtsMapLocation, dependencyConfig.path] :
+                                        [libFile.path, dtsPath, dtsMapLocation],
+                                otherWatchedFiles: withRefs || disableSourceOfProjectReferenceRedirect ?
+                                    [mainConfig.path] : // dependencyConfig is in closed info
                                     [mainConfig.path, dependencyConfig.path],
                                 expectsDts: !withRefs, // Dts script info present only if no project reference
                                 expectsMap: !withRefs // Map script info present only if no project reference
@@ -1179,9 +1189,11 @@ ${dependencyTs.content}`);
                                 // DependencyTs is open, so omit it from closed infos
                                 closedInfos: withRefs ?
                                     [dependencyConfig.path, libFile.path, dtsLocation, dtsMapLocation] :
-                                    [libFile.path, dtsPath, dtsMapLocation],
-                                otherWatchedFiles: withRefs ?
-                                    [mainConfig.path] : // Its in closed info
+                                    disableSourceOfProjectReferenceRedirect ?
+                                        [libFile.path, dtsPath, dtsMapLocation, dependencyConfig.path] :
+                                        [libFile.path, dtsPath, dtsMapLocation],
+                                otherWatchedFiles: withRefs || disableSourceOfProjectReferenceRedirect ?
+                                    [mainConfig.path] : // dependencyConfig is in closed info
                                     [mainConfig.path, dependencyConfig.path],
                                 expectsDts: true,
                                 expectsMap: true,

tests/cases/fourslash/bestCommonTypeObjectLiterals.ts

@@ -0,0 +1,37 @@
+/// <reference path='fourslash.ts'/>
+
+////var a = { name: 'bob', age: 18 };
+////var b = { name: 'jim', age: 20 };
+////var /*1*/c = [a, b];
+
+////var a1 = { name: 'bob', age: 18 };
+////var b1 = { name: 'jim', age: 20, dob: new Date() };
+////var /*2*/c1 = [a1, b1];
+
+////var a2 = { name: 'bob', age: 18, address: 'springfield' };
+////var b2 = { name: 'jim', age: 20, dob: new Date() };
+////var /*3*/c2 = [a2, b2];
+
+////interface I {
+////    name: string;
+////    age: number;
+////}
+
+////var i: I;
+////var /*4*/c3 = [i, a];
+
+verify.quickInfos({
+    1: "var c: {\n    name: string;\n    age: number;\n}[]",
+    2: "var c1: {\n    name: string;\n    age: number;\n}[]",
+    3:
+        `var c2: ({
+    name: string;
+    age: number;
+    address: string;
+} | {
+    name: string;
+    age: number;
+    dob: Date;
+})[]`,
+    4: "var c3: {\n    name: string;\n    age: number;\n}[]"
+});

tests/cases/fourslash/bestCommonTypeObjectLiterals1.ts

@@ -20,6 +20,9 @@
 ////var i: I;
 ////var /*4*/c3 = [i, a];
 
+verify.quickInfos({
+    4: "var c3: I[]"
+});
 verify.quickInfos({
     1: "var c: {\n    name: string;\n    age: number;\n}[]",
     2: "var c1: {\n    name: string;\n    age: number;\n}[]",

tests/cases/fourslash/findAllRefs_importType_js.ts

@@ -18,6 +18,11 @@ verify.noErrors();
 // TODO: GH#24025
 
 const [rModuleDef, rModule, r0Def, r0, r1Def, r1, r2Def, r2, r3Def, r3, r4Def, r4, r5] = test.ranges();
+verify.referenceGroups([r3, r4], [
+    { definition: 'module "/a"', ranges: [r4, rModule] },
+    { definition: "(local class) C", ranges: [r0] },
+    { definition: "(alias) (local class) export=\nimport export=", ranges: [r3] },
+]);
 verify.referenceGroups(rModule, [{ definition: 'module "/a"', ranges: [r3, r4, rModule] }]);
 verify.referenceGroups(r0, [
     { definition: "(local class) C", ranges: [r0] },
@@ -33,6 +38,6 @@ verify.referenceGroups(r2, [
 ]);
 verify.referenceGroups([r3, r4], [
     { definition: 'module "/a"', ranges: [r4, rModule] },
-    { definition: "(local class) C", ranges: [r0] },
-    { definition: "(alias) (local class) export=\nimport export=", ranges: [r3] },
+    //{ definition: "(local class) C", ranges: [r0] },
+    //{ definition: "(alias) (local class) export=\nimport export=", ranges: [r3] },
 ]);