Fix LS bugs triggered by file moving (and probably other scenarios) (#2456)

Author: andrewbranch

internal/project/configfileregistrybuilder.go

@@ -446,13 +446,25 @@ func (c *configFileRegistryBuilder) DidChangeFiles(summary FileChangeSummary, lo
 	// Handle possible root file creation
 	if len(createdFiles) > 0 {
 		c.configs.Range(func(entry *dirty.SyncMapEntry[tspath.Path, *configFileEntry]) bool {
+			var createdOpenFile bool
 			entry.ChangeIf(
 				func(config *configFileEntry) bool {
-					if config.commandLine == nil || config.rootFilesWatch == nil || config.pendingReload != PendingReloadNone {
+					if config.pendingReload != PendingReloadNone {
 						return false
 					}
 					logger.Logf("Checking if any of %d created files match root files for config %s", len(createdFiles), entry.Key())
 					for _, fileName := range createdFiles {
+						if _, ok := config.retainingOpenFiles[c.fs.toPath(fileName)]; ok {
+							// We saw a create event for a file that's already open, and when we first opened it,
+							// we tried to see if it belonged to this config, but we may have incorrectly answered
+							// "no" because we hadn't invalidated the config's file list since the file was created.
+							// Now that we're seeing a creation event for it, we need to reload the config's file names.
+							createdOpenFile = true
+							return true
+						}
+						if config.commandLine == nil || config.rootFilesWatch == nil {
+							continue
+						}
 						if config.commandLine.PossiblyMatchesFileName(fileName) {
 							return true
 						}
@@ -467,6 +479,16 @@ func (c *configFileRegistryBuilder) DidChangeFiles(summary FileChangeSummary, lo
 					maps.Copy(affectedProjects, config.retainingProjects)
 					logger.Logf("Root files for config %s changed", entry.Key())
 					shouldInvalidateCache = hasExcessiveChanges
+					if createdOpenFile {
+						for openFilePath := range config.retainingOpenFiles {
+							if _, ok := createdFiles[openFilePath]; ok {
+								if affectedFiles == nil {
+									affectedFiles = make(map[tspath.Path]struct{})
+								}
+								affectedFiles[openFilePath] = struct{}{}
+							}
+						}
+					}
 				},
 			)
 			return !shouldInvalidateCache

internal/project/overlayfs.go

@@ -315,6 +315,7 @@ func (fs *overlayFS) processChanges(changes []FileChange) (FileChangeSummary, ma
 			}
 			result.Closed[uri] = events.closeChange.Hash
 			delete(newOverlays, path)
+			o = nil
 		}
 
 		if events.watchChanged {
@@ -364,7 +365,7 @@ func (fs *overlayFS) processChanges(changes []FileChange) (FileChangeSummary, ma
 			newOverlays[path] = o
 		}
 
-		if events.created && o == nil {
+		if events.created {
 			result.Created.Add(uri)
 		}
 

internal/project/projectcollectionbuilder.go

@@ -89,7 +89,7 @@ func (b *ProjectCollectionBuilder) Finalize(logger *logging.LogTree) (*ProjectCo
 		newProjectCollection.configuredProjects = configuredProjects
 	}
 
-	if !changed && !maps.Equal(b.fileDefaultProjects, b.base.fileDefaultProjects) {
+	if !maps.Equal(b.fileDefaultProjects, b.base.fileDefaultProjects) {
 		ensureCloned()
 		newProjectCollection.fileDefaultProjects = b.fileDefaultProjects
 	}
@@ -338,6 +338,16 @@ func logChangeFileResult(result changeFileResult, logger *logging.LogTree) {
 	}
 }
 
+func (b *ProjectCollectionBuilder) cleanupInferredProject(logger *logging.LogTree) {
+	var inferredProjectFiles []string
+	for path, overlay := range b.fs.overlays {
+		if b.findDefaultConfiguredProject(overlay.FileName(), path) == nil {
+			inferredProjectFiles = append(inferredProjectFiles, overlay.FileName())
+		}
+	}
+	b.updateInferredProjectRoots(inferredProjectFiles, logger)
+}
+
 func (b *ProjectCollectionBuilder) DidRequestFile(uri lsproto.DocumentUri, logger *logging.LogTree) {
 	startTime := time.Now()
 	fileName := uri.FileName()
@@ -349,6 +359,9 @@ func (b *ProjectCollectionBuilder) DidRequestFile(uri lsproto.DocumentUri, logge
 		if result := b.findDefaultProject(fileName, path); result != nil {
 			hasChanges = b.updateProgram(result, logger) || hasChanges
 			if result.Value() != nil {
+				if hasChanges {
+					b.cleanupInferredProject(logger)
+				}
 				return
 			}
 		}
@@ -361,15 +374,7 @@ func (b *ProjectCollectionBuilder) DidRequestFile(uri lsproto.DocumentUri, logge
 		if hasChanges {
 			// If the structure of other projects changed, we might need to move files
 			// in/out of the inferred project.
-			var inferredProjectFiles []string
-			for path, overlay := range b.fs.overlays {
-				if b.findDefaultConfiguredProject(overlay.FileName(), path) == nil {
-					inferredProjectFiles = append(inferredProjectFiles, overlay.FileName())
-				}
-			}
-			if len(inferredProjectFiles) > 0 {
-				b.updateInferredProjectRoots(inferredProjectFiles, logger)
-			}
+			b.cleanupInferredProject(logger)
 		}
 
 		if b.inferredProject.Value() != nil {

internal/project/projectlifetime_test.go

@@ -217,4 +217,131 @@ func TestProjectLifetime(t *testing.T) {
 		assert.Equal(t, len(snapshot.ProjectCollection.Projects()), 1)
 		assert.Assert(t, snapshot.ConfigFileRegistry.GetConfig(tspath.Path("/home/projects/ts/p1/tsconfig.json")) != nil)
 	})
+
+	t.Run("file move from inferred to configured via didOpen/didClose sequence", func(t *testing.T) {
+		t.Parallel()
+		// Start with tsconfig.json that includes "src" but file is at root level
+		files := map[string]any{
+			"/home/projects/TS/p1/tsconfig.json": `{
+				"compilerOptions": {
+					"noLib": true
+				},
+				"include": ["src"]
+			}`,
+			"/home/projects/TS/p1/index.ts": `export const x = 1;`,
+		}
+		session, utils := projecttestutil.Setup(files)
+
+		// Open index.ts at root level - should create inferred project since it's not under src/
+		// Creates config file registry entry, but has no files
+		indexUri := lsproto.DocumentUri("file:///home/projects/TS/p1/index.ts")
+		session.DidOpenFile(context.Background(), indexUri, 1, files["/home/projects/TS/p1/index.ts"].(string), lsproto.LanguageKindTypeScript)
+
+		// Should have one inferred project only (file is not included by tsconfig)
+		snapshot, release := session.Snapshot()
+		defer release()
+		assert.Equal(t, len(snapshot.ProjectCollection.Projects()), 1)
+		assert.Assert(t, snapshot.ProjectCollection.InferredProject() != nil)
+		assert.Assert(t, snapshot.ProjectCollection.ConfiguredProject(tspath.Path("/home/projects/ts/p1/tsconfig.json")) == nil)
+
+		// Simulate file move: create src/index.ts on disk
+		err := utils.FS().WriteFile("/home/projects/TS/p1/src/index.ts", files["/home/projects/TS/p1/index.ts"].(string), false)
+		assert.NilError(t, err)
+		err = utils.FS().Remove("/home/projects/TS/p1/index.ts")
+		assert.NilError(t, err)
+
+		// Simulate file move sequence as it would happen in an editor:
+		// 1. didOpen src/index.ts (new location)
+		// Open comes in before file create event, so the config file is not marked as needing a file name reload,
+		// so it's not turned into a configured project yet. This is probably not ideal, but it should sort itself
+		// out momentarily after the file watcher events are processed. When we try the config file, we mark it
+		// as "retained by src/index.ts" so the config entry doesn't get deleted before src/index.ts is closed.
+		// Even though we currently think src/index.ts doesn't belong to the config, the config is in its directory
+		// path, so we'll always see it as a candidate for containing src/index.ts.
+		srcIndexUri := lsproto.DocumentUri("file:///home/projects/TS/p1/src/index.ts")
+		session.DidOpenFile(context.Background(), srcIndexUri, 1, files["/home/projects/TS/p1/index.ts"].(string), lsproto.LanguageKindTypeScript)
+
+		// 2. didClose index.ts (old location)
+		session.DidCloseFile(context.Background(), indexUri)
+
+		// 3. didChangeWatchedFiles: create src/index.ts and delete index.ts
+		// The creation event for src/index.ts now hits the config file registry, and we should notice we
+		// got a creation event for a file that retained the config, triggering a filename reload.
+		session.DidChangeWatchedFiles(context.Background(), []*lsproto.FileEvent{
+			{
+				Uri:  srcIndexUri,
+				Type: lsproto.FileChangeTypeCreated,
+			},
+			{
+				Uri:  indexUri,
+				Type: lsproto.FileChangeTypeDeleted,
+			},
+		})
+
+		// Should now have one configured project only (file is now under src/)
+		_, err = session.GetLanguageService(context.Background(), srcIndexUri)
+		assert.NilError(t, err)
+		snapshot, release = session.Snapshot()
+		defer release()
+		assert.Equal(t, len(snapshot.ProjectCollection.Projects()), 1)
+		assert.Assert(t, snapshot.ProjectCollection.InferredProject() == nil)
+		assert.Assert(t, snapshot.ProjectCollection.ConfiguredProject(tspath.Path("/home/projects/ts/p1/tsconfig.json")) != nil)
+	})
+
+	t.Run("tsconfig move from subdirectory to parent via didChangeWatchedFiles", func(t *testing.T) {
+		t.Parallel()
+		// Start with tsconfig.json in src/ that includes "src" - file won't be included initially
+		files := map[string]any{
+			"/home/projects/TS/p1/src/tsconfig.json": `{
+				"compilerOptions": {
+					"noLib": true
+				},
+				"include": ["src"]
+			}`,
+			"/home/projects/TS/p1/src/index.ts": `export const x = 1;`,
+		}
+		session, utils := projecttestutil.Setup(files)
+
+		// Open src/index.ts - should create inferred project since tsconfig.json includes "src"
+		// relative to its location (src/src/ which doesn't exist)
+		indexUri := lsproto.DocumentUri("file:///home/projects/TS/p1/src/index.ts")
+		session.DidOpenFile(context.Background(), indexUri, 1, files["/home/projects/TS/p1/src/index.ts"].(string), lsproto.LanguageKindTypeScript)
+
+		// Should have one inferred project only (file is not included by tsconfig at src/tsconfig.json)
+		snapshot, release := session.Snapshot()
+		defer release()
+		assert.Equal(t, len(snapshot.ProjectCollection.Projects()), 1)
+		assert.Assert(t, snapshot.ProjectCollection.InferredProject() != nil)
+		assert.Assert(t, snapshot.ProjectCollection.ConfiguredProject(tspath.Path("/home/projects/ts/p1/src/tsconfig.json")) == nil)
+
+		// Simulate tsconfig.json move: create tsconfig.json at parent level, delete from src/
+		tsconfigContent := files["/home/projects/TS/p1/src/tsconfig.json"].(string)
+		err := utils.FS().WriteFile("/home/projects/TS/p1/tsconfig.json", tsconfigContent, false)
+		assert.NilError(t, err)
+		err = utils.FS().Remove("/home/projects/TS/p1/src/tsconfig.json")
+		assert.NilError(t, err)
+
+		// Simulate file move via didChangeWatchedFiles
+		newTsconfigUri := lsproto.DocumentUri("file:///home/projects/TS/p1/tsconfig.json")
+		oldTsconfigUri := lsproto.DocumentUri("file:///home/projects/TS/p1/src/tsconfig.json")
+		session.DidChangeWatchedFiles(context.Background(), []*lsproto.FileEvent{
+			{
+				Uri:  newTsconfigUri,
+				Type: lsproto.FileChangeTypeCreated,
+			},
+			{
+				Uri:  oldTsconfigUri,
+				Type: lsproto.FileChangeTypeDeleted,
+			},
+		})
+
+		// Should now have one configured project only (tsconfig.json now includes src/index.ts)
+		_, err = session.GetLanguageService(context.Background(), indexUri)
+		assert.NilError(t, err)
+		snapshot, release = session.Snapshot()
+		defer release()
+		assert.Equal(t, len(snapshot.ProjectCollection.Projects()), 1)
+		assert.Assert(t, snapshot.ProjectCollection.InferredProject() == nil)
+		assert.Assert(t, snapshot.ProjectCollection.ConfiguredProject(tspath.Path("/home/projects/ts/p1/tsconfig.json")) != nil)
+	})
 }