From 60a2bb57d4eda331433f815ede5320f71a08f9f0 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 25 Aug 2025 11:56:31 -0700 Subject: [PATCH 1/4] Fix stale typings-augmented ParsedCommandLine cache --- internal/project/project_test.go | 43 ++++++++++++++++++++ internal/project/projectcollectionbuilder.go | 6 ++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/project/project_test.go b/internal/project/project_test.go index 33d7471180..8eba869e71 100644 --- a/internal/project/project_test.go +++ b/internal/project/project_test.go @@ -130,3 +130,46 @@ 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) { + 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) + + // 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) + }) +} diff --git a/internal/project/projectcollectionbuilder.go b/internal/project/projectcollectionbuilder.go index 9b104bd488..b6f4c8eed8 100644 --- a/internal/project/projectcollectionbuilder.go +++ b/internal/project/projectcollectionbuilder.go @@ -720,6 +720,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 +754,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 { From 86ecbd05023cb24a6d5fa63876e1946b54d9b3dc Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 25 Aug 2025 12:19:15 -0700 Subject: [PATCH 2/4] Guard against missing extended config files --- internal/project/configfilechanges_test.go | 30 ++++++++++++++++++++++ internal/project/extendedconfigcache.go | 8 ++++-- internal/tsoptions/tsconfigparsing.go | 20 ++++++++------- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/internal/project/configfilechanges_test.go b/internal/project/configfilechanges_test.go index 7b4de6dc2c..036d243963 100644 --- a/internal/project/configfilechanges_test.go +++ b/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) + }) } diff --git a/internal/project/extendedconfigcache.go b/internal/project/extendedconfigcache.go index 2eb0e20539..38654c55ae 100644 --- a/internal/project/extendedconfigcache.go +++ b/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 } diff --git a/internal/tsoptions/tsconfigparsing.go b/internal/tsoptions/tsconfigparsing.go index fcc8394ccc..d81f75880a 100644 --- a/internal/tsoptions/tsconfigparsing.go +++ b/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 } From 13d1f629365ff3117b51564bd6af0da3fb1c888e Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 25 Aug 2025 14:07:29 -0700 Subject: [PATCH 3/4] Fix #1630 --- internal/project/projectcollectionbuilder.go | 6 ++++ .../project/projectcollectionbuilder_test.go | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/internal/project/projectcollectionbuilder.go b/internal/project/projectcollectionbuilder.go index b6f4c8eed8..ad158329c3 100644 --- a/internal/project/projectcollectionbuilder.go +++ b/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) diff --git a/internal/project/projectcollectionbuilder_test.go b/internal/project/projectcollectionbuilder_test.go index cd890488dc..0f63f5aae3 100644 --- a/internal/project/projectcollectionbuilder_test.go +++ b/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 { From 835650c7784c23553f2546078687f9afbec9f60c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 25 Aug 2025 14:11:20 -0700 Subject: [PATCH 4/4] Fix lint --- internal/project/project_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/project/project_test.go b/internal/project/project_test.go index 8eba869e71..90ad9cda64 100644 --- a/internal/project/project_test.go +++ b/internal/project/project_test.go @@ -138,6 +138,7 @@ func TestProject(t *testing.T) { } 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"}}`, @@ -161,6 +162,7 @@ func TestProject(t *testing.T) { 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")