Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 31, 2025

Fix crash in range formatting when requested on a line different from containing function

  • Fix nil pointer dereference in getIndentationForNodeWorker at indent.go:69
  • Add test case to reproduce the crash scenario
  • Run existing tests to ensure no regressions
  • Verify fix handles edge cases properly
  • Code review completed - no issues found
  • Security check passed
  • Refactored based on code review feedback

Summary

Fixed a nil pointer dereference that caused a panic during range formatting when the containing list had no children. The fix adds a nil check before calling getStartLineAndCharacterForNode to prevent accessing a nil node's position.

Changes Made

  1. internal/format/indent.go: Added nil check and refactored to compute listIndentsChild directly within the check, avoiding redundant nil checks
  2. internal/ls/format_test.go: Added comprehensive test cases covering multiple edge cases from the issue

All tests pass successfully.

Original prompt

This section details on the original issue you should resolve

<issue_title>Crash in range formatting when requested on a line that is different from the containing function</issue_title>
<issue_description>```
function foo() {
[|return (1 + 2);|]
}


Format the range (trigger with <kbd>Ctrl</kbd>+<kbd>K</kbd>; <kbd>Ctrl</kbd>+<kbd>F</kbd>).

panic handling request textDocument/rangeFormatting runtime error: invalid memory address or nil pointer dereference goroutine 2169 [running]:
runtime/debug.Stack()
runtime/debug/stack.go:26 +0x64
github.com/microsoft/typescript-go/internal/lsp.(*Server).recover(0x4000000f00, 0x400c378180)
github.com/microsoft/typescript-go/internal/lsp/server.go:567 +0x44
panic({0x7ff78a61ad20?, 0x7ff78af56550?})
runtime/panic.go:783 +0x120
github.com/microsoft/typescript-go/internal/ast.(*Node).Pos(...)
github.com/microsoft/typescript-go/internal/ast/ast.go:240
github.com/microsoft/typescript-go/internal/scanner.GetTokenPosOfNode(0xf?, 0x29?, 0xc0?)
github.com/microsoft/typescript-go/internal/scanner/scanner.go:2323 +0xa8
github.com/microsoft/typescript-go/internal/format.getStartLineAndCharacterForNode(0x400c2b46c0?, 0x40060cbb08)
github.com/microsoft/typescript-go/internal/format/indent.go:250 +0x24
github.com/microsoft/typescript-go/internal/format.getIndentationForNodeWorker(0x7ff78a97b790?, 0x40060cbb08?, 0xf?, 0x4006151d48, 0x7ff78a97d0e8?, 0x40060cbb08, 0x0, 0x40034ea150)
github.com/microsoft/typescript-go/internal/format/indent.go:69 +0x154
github.com/microsoft/typescript-go/internal/format.GetIndentationForNode(0x400c2b46c0, 0x4006151d48, 0x40060cbb08, 0x40034ea150)
github.com/microsoft/typescript-go/internal/format/indent.go:17 +0x6c
github.com/microsoft/typescript-go/internal/format.FormatSpan({0x7ff78a97d0e8, 0x400ac40180}, {0x12, 0x26}, 0x40060cbb08, 0x1)
github.com/microsoft/typescript-go/internal/format/api.go:69 +0xbc
github.com/microsoft/typescript-go/internal/format.FormatSelection({0x7ff78a97d0e8, 0x400ac40180}, 0x40060cbb08, 0x7ff78a970f20?, 0x26)
github.com/microsoft/typescript-go/internal/format/api.go:112 +0x50
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getFormattingEditsForRange(0x400a301470?, {0x7ff78a97d120?, 0x4009878000?}, 0x40060cbb08, 0x17?, {0x0?, 0x0?})
github.com/microsoft/typescript-go/internal/ls/format.go:95 +0x5c
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).ProvideFormatDocumentRange(0x400ac400f0, {0x7ff78a97d120, 0x4009878000}, {0x4009ba8300?, 0x4006151e78?}, 0x400ac1a000, {{0x46bcb97d?, 0x12?}, {0x634352ec?, 0x1395cb5?}})
github.com/microsoft/typescript-go/internal/ls/format.go:61 +0xf8
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleDocumentRangeFormat(0x40002a0a20?, {0x7ff78a97d120?, 0x4009878000?}, 0x4009ba8300?, 0x13?)
github.com/microsoft/typescript-go/internal/lsp/server.go:869 +0x40
github.com/microsoft/typescript-go/internal/lsp.init.func1.registerLanguageServiceDocumentRequestHandler[...].21({0x7ff78a97d120, 0x4009878000}, 0x400c378180)
github.com/microsoft/typescript-go/internal/lsp/server.go:553 +0xe0
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleRequestOrNotification(0x4000000f00, {0x7ff78a97d120, 0x4009878000}, 0x400c378180)
github.com/microsoft/typescript-go/internal/lsp/server.go:449 +0xf4
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop.func1()
github.com/microsoft/typescript-go/internal/lsp/server.go:354 +0x34
created by github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop in goroutine 10
github.com/microsoft/typescript-go/internal/lsp/server.go:374 +0x79c


## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@DanielRosenwasser</author><body>
Updated title - I think you can trigger this just by requesting formatting ranges anywhere after the first line of the function

```ts
function
/*$*/f() {
}
function f() {
  /*$*/
}
function f() {
}/*$*/
[Error - 11:14:49 AM] Request textDocument/rangeFormatting failed.
  Message: InternalError: panic handling request textDocument/rangeFormatting: runtime error: invalid memory address or nil pointer dereference
  Code: -32603 
panic handling request textDocument/rangeFormatting runtime error: invalid memory address or nil pointer dereference goroutine 1294 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:26 +0x64
github.com/microsoft/typescript-go/internal/lsp.(*Server).recover(0x4000000f00, 0x40089fb8c0)
	github.com/microsoft/typescript-go/internal/lsp/server.go:570 +0x44
panic({0x7ff71019e300?, 0x7ff710add530?})
	runtime/panic.go:783 +0x120
github.com/microsoft/typescript-go/internal/ast.(*Node).Pos(...)
	github.com/microsoft/typescript-go/internal/ast/ast.go:240
github.com/microsoft/typescript-go/internal/scanner.GetTokenPosOfNode(0xe?, 0x12?, 0x20?)
	github.com/microsoft/typescript-go/internal/scanner/scanner.go:2323 +0xa8
github...

</details>

- Fixes microsoft/typescript-go#1992

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

- Added nil check before calling getStartLineAndCharacterForNode in indent.go
- Added comprehensive test cases to verify the fix
- All tests pass successfully

Co-authored-by: DanielRosenwasser <[email protected]>
Copilot AI changed the title [WIP] Fix crash in range formatting due to invalid memory access Fix nil pointer dereference in range formatting Oct 31, 2025
Copilot finished work on behalf of DanielRosenwasser October 31, 2025 18:40
Simplified the listIndentsChild computation by moving it inside the nil check,
avoiding redundant nil check and limiting listLine scope.

Co-authored-by: jakebailey <[email protected]>
Copilot AI requested a review from jakebailey October 31, 2025 18:50
Copilot finished work on behalf of jakebailey October 31, 2025 18:50
Copilot AI added a commit that referenced this pull request Nov 1, 2025
- Remove unused markers (/*1*/, /*2*/, /*3*/) from formatDocumentInPlace_test.go
- Apply fix from PR #1993 to prevent nil pointer crash in indent.go when firstListChild is nil
- Add comment about known trailing space issue (#1997)

The indent.go fix prevents crashes when formatting ranges where the list has no first child.

Co-authored-by: DanielRosenwasser <[email protected]>
@DanielRosenwasser DanielRosenwasser marked this pull request as ready for review November 7, 2025 23:21
Copilot AI review requested due to automatic review settings November 7, 2025 23:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a crash in range formatting when formatting a range inside a function body on a different line from the function declaration. The fix addresses a nil pointer dereference issue in the indentation calculation logic.

Key Changes

  • Added nil check for firstListChild before dereferencing in internal/format/indent.go to prevent panic
  • Added comprehensive test cases in internal/ls/format_test.go to verify the fix across multiple scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/format/indent.go Fixed nil pointer dereference by adding a nil check before using firstListChild in indentation calculation
internal/ls/format_test.go Added regression test TestGetFormattingEditsForRange_FunctionBody with multiple test cases to prevent future crashes

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good for now, I want to unbreak formatting already.

@jakebailey jakebailey enabled auto-merge November 7, 2025 23:45
@jakebailey jakebailey added this pull request to the merge queue Nov 8, 2025
Merged via the queue into main with commit 97756ce Nov 8, 2025
22 checks passed
@jakebailey jakebailey deleted the copilot/fix-range-formatting-crash branch November 8, 2025 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants