Fix file rename crashes when dealing with solution configuration files#4067
Fix file rename crashes when dealing with solution configuration files#4067DanielRosenwasser wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a nil-pointer panic during workspace/willRenameFiles handling when a “solution-style” ancestor tsconfig.json project exists without a built Program (e.g., it only references child composite projects). This makes file renames safe in multi-project/solution configurations in the language server.
Changes:
- Guard
LanguageService.GetEditsForFileRenameagainst a nilProgramto prevent crashes. - Add a fourslash regression test that exercises renaming within a referenced composite project while an ancestor solution project has a nil program.
Show a summary per file
| File | Description |
|---|---|
| internal/ls/file_rename.go | Adds a nil-check for program before computing rename edits, preventing a panic. |
| internal/fourslash/tests/getEditsForFileRenameSolutionNoCrash_test.go | Adds regression coverage for solution-style tsconfig rename behavior to ensure no crash and correct import update. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
|
|
||
| func (l *LanguageService) GetEditsForFileRename(ctx context.Context, oldURI lsproto.DocumentUri, newURI lsproto.DocumentUri) []lsproto.TextDocumentEditOrCreateFileOrRenameFileOrDeleteFile { | ||
| program := l.GetProgram() | ||
| if program == nil { |
There was a problem hiding this comment.
I think this is ok, but for correctness, we probably also need to make sure that we receive the project's command line here so that we can properly rename things in tsconfigs even when the program is nil.
There was a problem hiding this comment.
I assumed that a program could only be nil if we ended up with no files - in which case, there should be nothing to update, right?
There was a problem hiding this comment.
We could still have to update paths in its tsconfig I think, which is what we try to do a few lines below here in l.updateTsconfigFiles.
There was a problem hiding this comment.
I'm having a hard time wrapping my head around when a tsconfig.json may not have an associated Program, but the owning project will have a CommandLine set.
…n case `program` is `nil`.
Fixes #4066.