Fix three snapshots bugs (microsoft#1633)

Author: andrewbranch

internal/project/configfilechanges_test.go

@@ -153,4 +153,34 @@ func TestConfigFileChanges(t *testing.T) {
 		defer release()
 		assert.Equal(t, len(snapshot.ProjectCollection.Projects()), 1)
 	})
+
+	t.Run("should update project when missing extended config is created", func(t *testing.T) {
+		t.Parallel()
+		// Start with a project whose tsconfig extends a base config that doesn't exist yet
+		missingBaseFiles := map[string]any{}
+		for k, v := range files {
+			if k == "/tsconfig.base.json" {
+				continue
+			}
+			missingBaseFiles[k] = v
+		}
+
+		session, utils := projecttestutil.Setup(missingBaseFiles)
+		session.DidOpenFile(context.Background(), "file:///src/index.ts", 1, missingBaseFiles["/src/index.ts"].(string), lsproto.LanguageKindTypeScript)
+
+		// Create the previously-missing base config file that is extended by /src/tsconfig.json
+		err := utils.FS().WriteFile("/tsconfig.base.json", `{"compilerOptions": {"strict": true}}`, false /*writeByteOrderMark*/)
+		assert.NilError(t, err)
+		session.DidChangeWatchedFiles(context.Background(), []*lsproto.FileEvent{
+			{
+				Uri:  lsproto.DocumentUri("file:///tsconfig.base.json"),
+				Type: lsproto.FileChangeTypeCreated,
+			},
+		})
+
+		// Accessing the language service should trigger project update
+		ls, err := session.GetLanguageService(context.Background(), lsproto.DocumentUri("file:///src/index.ts"))
+		assert.NilError(t, err)
+		assert.Equal(t, ls.GetProgram().Options().Strict, core.TSTrue)
+	})
 }

internal/project/extendedconfigcache.go

@@ -23,10 +23,14 @@ type extendedConfigCacheEntry struct {
 func (c *extendedConfigCache) Acquire(fh FileHandle, path tspath.Path, parse func() *tsoptions.ExtendedConfigCacheEntry) *tsoptions.ExtendedConfigCacheEntry {
 	entry, loaded := c.loadOrStoreNewLockedEntry(path)
 	defer entry.mu.Unlock()
-	if !loaded || entry.hash != fh.Hash() {
+	var hash xxh3.Uint128
+	if fh != nil {
+		hash = fh.Hash()
+	}
+	if !loaded || entry.hash != hash {
 		// Reparse the config if the hash has changed, or parse for the first time.
 		entry.entry = parse()
-		entry.hash = fh.Hash()
+		entry.hash = hash
 	}
 	return entry.entry
 }

internal/project/project_test.go

@@ -130,3 +130,48 @@ func TestProjectProgramUpdateKind(t *testing.T) {
 		assert.Equal(t, configured.ProgramUpdateKind, project.ProgramUpdateKindSameFileNames)
 	})
 }
+
+func TestProject(t *testing.T) {
+	t.Parallel()
+	if !bundled.Embedded {
+		t.Skip("bundled files are not embedded")
+	}
+
+	t.Run("commandLineWithTypingsFiles is reset on CommandLine change", func(t *testing.T) {
+		t.Parallel()
+		files := map[string]any{
+			"/user/username/projects/project1/app.js":       ``,
+			"/user/username/projects/project1/package.json": `{"name":"p1","dependencies":{"jquery":"^3.1.0"}}`,
+			"/user/username/projects/project2/app.js":       ``,
+		}
+
+		session, utils := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
+			PackageToFile: map[string]string{
+				// Provide typings content to be installed for jquery so ATA actually installs something
+				"jquery": `declare const $: { x: number }`,
+			},
+		})
+
+		// 1) Open an inferred project file that triggers ATA
+		uri1 := lsproto.DocumentUri("file:///user/username/projects/project1/app.js")
+		session.DidOpenFile(context.Background(), uri1, 1, files["/user/username/projects/project1/app.js"].(string), lsproto.LanguageKindJavaScript)
+
+		// 2) Wait for ATA/background tasks to finish, then get a language service for the first file
+		session.WaitForBackgroundTasks()
+		// Sanity check: ensure ATA performed at least one install
+		npmCalls := utils.NpmExecutor().NpmInstallCalls()
+		assert.Assert(t, len(npmCalls) > 0, "expected at least one npm install call from ATA")
+		_, err := session.GetLanguageService(context.Background(), uri1)
+		assert.NilError(t, err)
+
+		// 3) Open another inferred project file
+		uri2 := lsproto.DocumentUri("file:///user/username/projects/project2/app.js")
+		session.DidOpenFile(context.Background(), uri2, 1, ``, lsproto.LanguageKindJavaScript)
+
+		// 4) Get a language service for the second file
+		//    If commandLineWithTypingsFiles was not reset, the new program command line
+		//    won't include the newly opened file and this will fail.
+		_, err = session.GetLanguageService(context.Background(), uri2)
+		assert.NilError(t, err)
+	})
+}

internal/project/projectcollectionbuilder.go

@@ -515,6 +515,7 @@ func (b *projectCollectionBuilder) findOrCreateDefaultConfiguredProjectWorker(
 			configFilePath := b.toPath(node.configFileName)
 			config := b.configFileRegistryBuilder.findOrAcquireConfigForOpenFile(node.configFileName, configFilePath, path, node.loadKind, node.logger.Fork("Acquiring config for open file"))
 			if config == nil {
+				node.logger.Log("Config file for project does not already exist")
 				return false, false
 			}
 			configs.Store(configFilePath, config)
@@ -535,6 +536,11 @@ func (b *projectCollectionBuilder) findOrCreateDefaultConfiguredProjectWorker(
 			}
 
 			project := b.findOrCreateProject(node.configFileName, configFilePath, node.loadKind, node.logger)
+			if project == nil {
+				node.logger.Log("Project does not already exist")
+				return false, false
+			}
+
 			if node.loadKind == projectLoadKindCreate {
 				// Ensure project is up to date before checking for file inclusion
 				b.updateProgram(project, node.logger)
@@ -720,6 +726,7 @@ func (b *projectCollectionBuilder) updateInferredProjectRoots(rootFileNames []st
 					logger.Log(fmt.Sprintf("Updating inferred project config with %d root files", len(rootFileNames)))
 				}
 				p.CommandLine = newCommandLine
+				p.commandLineWithTypingsFiles = nil
 				p.dirty = true
 				p.dirtyFilePath = ""
 			},
@@ -753,7 +760,10 @@ func (b *projectCollectionBuilder) updateProgram(entry dirty.Value[*Project], lo
 					filesChanged = true
 					return
 				}
-				entry.Change(func(p *Project) { p.CommandLine = commandLine })
+				entry.Change(func(p *Project) {
+					p.CommandLine = commandLine
+					p.commandLineWithTypingsFiles = nil
+				})
 			}
 		}
 		if !updateProgram {

internal/project/projectcollectionbuilder_test.go

@@ -407,6 +407,38 @@ func TestProjectCollectionBuilder(t *testing.T) {
 		assert.Assert(t, snapshot.ConfigFileRegistry.GetConfig("/home/src/projects/project/tsconfig.json") == nil)
 		assert.Assert(t, snapshot.ConfigFileRegistry.GetConfig("/home/src/projects/project/tsconfig.node.json") == nil)
 	})
+
+	t.Run("#1630", func(t *testing.T) {
+		t.Parallel()
+		files := map[string]any{
+			"/project/lib/tsconfig.json": `{
+				"files": ["a.ts"]
+			}`,
+			"/project/lib/a.ts": `export const a = 1;`,
+			"/project/lib/b.ts": `export const b = 1;`,
+			"/project/tsconfig.json": `{
+				"files": [],
+				"references": [{ "path": "./lib" }],
+				"compilerOptions": {
+					"disableReferencedProjectLoad": true
+				}
+			}`,
+			"/project/index.ts": ``,
+		}
+
+		session, _ := projecttestutil.Setup(files)
+
+		// opening b.ts puts /project/lib/tsconfig.json in the config file registry and creates the project,
+		// but the project is ultimately not a match
+		session.DidOpenFile(context.Background(), "file:///project/lib/b.ts", 1, files["/project/lib/b.ts"].(string), lsproto.LanguageKindTypeScript)
+		// opening an unrelated file triggers cleanup of /project/lib/tsconfig.json since no open file is part of that project,
+		// but will keep the config file in the registry since lib/b.ts is still open
+		session.DidOpenFile(context.Background(), "untitled:Untitled-1", 1, "", lsproto.LanguageKindTypeScript)
+		// Opening index.ts searches /project/tsconfig.json and then checks /project/lib/tsconfig.json without opening it.
+		// No early return on config file existence means we try to find an already open project, which returns nil,
+		// triggering a crash.
+		session.DidOpenFile(context.Background(), "file:///project/index.ts", 1, files["/project/index.ts"].(string), lsproto.LanguageKindTypeScript)
+	})
 }
 
 func filesForSolutionConfigFile(solutionRefs []string, compilerOptions string, ownFiles []string) map[string]any {

internal/tsoptions/tsconfigparsing.go

@@ -961,20 +961,22 @@ func getExtendedConfig(
 		cacheEntry = parse()
 	}
 
-	if cacheEntry != nil && len(cacheEntry.errors) > 0 {
+	if len(cacheEntry.errors) > 0 {
 		errors = append(errors, cacheEntry.errors...)
 	}
 
-	if sourceFile != nil {
-		result.extendedSourceFiles.Add(cacheEntry.extendedResult.SourceFile.FileName())
-		for _, extendedSourceFile := range cacheEntry.extendedResult.ExtendedSourceFiles {
-			result.extendedSourceFiles.Add(extendedSourceFile)
+	if cacheEntry.extendedResult != nil {
+		if sourceFile != nil {
+			result.extendedSourceFiles.Add(cacheEntry.extendedResult.SourceFile.FileName())
+			for _, extendedSourceFile := range cacheEntry.extendedResult.ExtendedSourceFiles {
+				result.extendedSourceFiles.Add(extendedSourceFile)
+			}
 		}
-	}
 
-	if len(cacheEntry.extendedResult.SourceFile.Diagnostics()) != 0 {
-		errors = append(errors, cacheEntry.extendedResult.SourceFile.Diagnostics()...)
-		return nil, errors
+		if len(cacheEntry.extendedResult.SourceFile.Diagnostics()) != 0 {
+			errors = append(errors, cacheEntry.extendedResult.SourceFile.Diagnostics()...)
+			return nil, errors
+		}
 	}
 	return cacheEntry.extendedConfig, errors
 }