[api] Fix panic in handleGetCompletionsAtPosition#4522
Conversation
allowing setupLanguageService for inferred projects
There was a problem hiding this comment.
Pull request overview
Fixes a panic in the API session’s completion handler path when operating on inferred (non-tsconfig) projects by avoiding a configured-project-only accessor.
Changes:
- Update
Session.setupLanguageServiceto key the language service withProject.ID()instead ofProject.ConfigFilePath()(avoids panic on inferred projects). - Add a regression test that requests completions for a loose file that resolves to an inferred project.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/api/session.go | Avoids calling ConfigFilePath() (panics on inferred projects) by using ID() when constructing a language service. |
| internal/api/session_completion_test.go | Adds a test covering completions on an inferred project to prevent regression. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| return nil, fmt.Errorf("%w: project %s not found", ErrClientError, projectName) | ||
| } | ||
| return ls.NewLanguageService(proj.ConfigFilePath(), program, sd.snapshot, activeFile), nil | ||
| return ls.NewLanguageService(proj.ID(), program, sd.snapshot, activeFile), nil |
There was a problem hiding this comment.
Sort of confused, every caller of NewLanguageService passes in a config file path, and yes ID is that, but that seems like only by happenstance?
There was a problem hiding this comment.
Yeah, this is currently the only way to get at the fake /dev/null/inferredproject path for inferred projects. I think this actually makes sense, because the language service only uses this string as a unique identifier (ultimately keying into project-specific buckets from the auto-import registry). I actually think the other usages of NewLanguageService could use .ID() and the parameter name could be renamed.
| return nil, fmt.Errorf("%w: project %s not found", ErrClientError, projectName) | ||
| } | ||
| return ls.NewLanguageService(proj.ConfigFilePath(), program, sd.snapshot, activeFile), nil | ||
| return ls.NewLanguageService(proj.ID(), program, sd.snapshot, activeFile), nil |
There was a problem hiding this comment.
Yeah, this is currently the only way to get at the fake /dev/null/inferredproject path for inferred projects. I think this actually makes sense, because the language service only uses this string as a unique identifier (ultimately keying into project-specific buckets from the auto-import registry). I actually think the other usages of NewLanguageService could use .ID() and the parameter name could be renamed.
andrewbranch
changed the title
Fix panic in handleGetCompletionsAtPosition by allowing setupLanguageService for inferred projects