From a0d3f5e2a1efaaa3f680f3454c029d538dac5e87 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 17:03:54 +0000 Subject: [PATCH 1/5] Initial plan From 41c533d135573ec5bb88342fe8947b1c915fdd4c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 17:19:03 +0000 Subject: [PATCH 2/5] Fix panic in LSP formatting with multi-byte characters and trailing newlines Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- internal/ls/converters.go | 7 ++- internal/ls/converters_bounds_test.go | 56 +++++++++++++++++++ .../formatMultiByteTrailingNewlines.js | 9 +++ .../formatMultiByteTrailingNewlines.symbols | 7 +++ .../formatMultiByteTrailingNewlines.types | 7 +++ .../formatMultiByteTrailingNewlines.ts | 7 +++ 6 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 internal/ls/converters_bounds_test.go create mode 100644 testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js create mode 100644 testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols create mode 100644 testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types create mode 100644 testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts diff --git a/internal/ls/converters.go b/internal/ls/converters.go index 18238662f3..e01dadd72b 100644 --- a/internal/ls/converters.go +++ b/internal/ls/converters.go @@ -228,12 +228,17 @@ func (c *Converters) PositionToLineAndCharacter(script Script, position core.Tex start := lineMap.LineStarts[line] + // Ensure position doesn't exceed text length to avoid slice bounds errors + text := script.Text() + textLen := core.TextPos(len(text)) + position = min(position, textLen) + var character core.TextPos if lineMap.AsciiOnly || c.positionEncoding == lsproto.PositionEncodingKindUTF8 { character = position - start } else { // We need to rescan the text as UTF-16 to find the character offset. - for _, r := range script.Text()[start:position] { + for _, r := range text[start:position] { character += core.TextPos(utf16.RuneLen(r)) } } diff --git a/internal/ls/converters_bounds_test.go b/internal/ls/converters_bounds_test.go new file mode 100644 index 0000000000..e7743afe41 --- /dev/null +++ b/internal/ls/converters_bounds_test.go @@ -0,0 +1,56 @@ +package ls + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/core" + "github.com/microsoft/typescript-go/internal/lsp/lsproto" +) + +type mockScript struct { + fileName string + text string +} + +func (m *mockScript) FileName() string { + return m.fileName +} + +func (m *mockScript) Text() string { + return m.text +} + +func TestPositionToLineAndCharacterBoundsCheck(t *testing.T) { + t.Parallel() + + // Test case that reproduces the panic with multi-byte characters and trailing newlines + text := "\"→\" ;\n\n\n" + + lineMap := ComputeLineStarts(text) + + converters := NewConverters(lsproto.PositionEncodingKindUTF16, func(fileName string) *LineMap { + return lineMap + }) + + script := &mockScript{ + fileName: "test.ts", + text: text, + } + + // This should not panic even if position is beyond text length + textLen := len(text) + position := core.TextPos(textLen) // position at end of text + + // This should work + result := converters.PositionToLineAndCharacter(script, position) + + t.Logf("Text length: %d, Position: %d", textLen, position) + t.Logf("Result: line=%d, char=%d", result.Line, result.Character) + + // This should also not panic, even though position is beyond text length + beyondEndPosition := core.TextPos(textLen + 1) + result2 := converters.PositionToLineAndCharacter(script, beyondEndPosition) + + t.Logf("Beyond end position: %d", beyondEndPosition) + t.Logf("Result2: line=%d, char=%d", result2.Line, result2.Character) +} \ No newline at end of file diff --git a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js new file mode 100644 index 0000000000..b0b4792053 --- /dev/null +++ b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js @@ -0,0 +1,9 @@ +//// [tests/cases/compiler/formatMultiByteTrailingNewlines.ts] //// + +//// [foo.ts] +"→" ; + + + +//// [foo.js] +"→"; diff --git a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols new file mode 100644 index 0000000000..b559d630af --- /dev/null +++ b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols @@ -0,0 +1,7 @@ +//// [tests/cases/compiler/formatMultiByteTrailingNewlines.ts] //// + +=== foo.ts === + +"→" ; + + diff --git a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types new file mode 100644 index 0000000000..3cab5698e9 --- /dev/null +++ b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types @@ -0,0 +1,7 @@ +//// [tests/cases/compiler/formatMultiByteTrailingNewlines.ts] //// + +=== foo.ts === +"→" ; +>"→" : "→" + + diff --git a/testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts b/testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts new file mode 100644 index 0000000000..fb2e970279 --- /dev/null +++ b/testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts @@ -0,0 +1,7 @@ +// @target: esnext +// @module: preserve +// @moduleResolution: bundler + +// @filename: foo.ts +"→" ; + From 1956a59be4a024107538fae1373e442308228372 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 17:23:26 +0000 Subject: [PATCH 3/5] Add comprehensive test coverage for issue #1389 fix Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- internal/ls/issue1389_test.go | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 internal/ls/issue1389_test.go diff --git a/internal/ls/issue1389_test.go b/internal/ls/issue1389_test.go new file mode 100644 index 0000000000..5c25a4d25d --- /dev/null +++ b/internal/ls/issue1389_test.go @@ -0,0 +1,53 @@ +package ls + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/core" + "github.com/microsoft/typescript-go/internal/lsp/lsproto" +) + +// TestIssue1389Fix tests the specific scenario from issue #1389 +func TestIssue1389Fix(t *testing.T) { + t.Parallel() + + // This is the exact content from the issue report + content := "\"→\" ;\n\n\n" + + lineMap := ComputeLineStarts(content) + + converters := NewConverters(lsproto.PositionEncodingKindUTF16, func(fileName string) *LineMap { + return lineMap + }) + + script := &mockScript{ + fileName: "test.ts", + text: content, + } + + // Test the exact scenario that was causing the panic + textLen := len(content) + + // These should not panic + result1 := converters.PositionToLineAndCharacter(script, core.TextPos(textLen)) + t.Logf("Position at end of text (%d): line=%d, char=%d", textLen, result1.Line, result1.Character) + + result2 := converters.PositionToLineAndCharacter(script, core.TextPos(textLen+1)) + t.Logf("Position beyond end of text (%d): line=%d, char=%d", textLen+1, result2.Line, result2.Character) + + // Also test converting ranges, which is what the formatter does + textRange := core.NewTextRange(0, textLen) + lspRange := converters.ToLSPRange(script, textRange) + + t.Logf("Range conversion successful: start=(%d,%d), end=(%d,%d)", + lspRange.Start.Line, lspRange.Start.Character, + lspRange.End.Line, lspRange.End.Character) + + // Test with a range that extends beyond the text (this was the original issue) + textRangeBeyondEnd := core.NewTextRange(0, textLen+1) + lspRangeBeyondEnd := converters.ToLSPRange(script, textRangeBeyondEnd) + + t.Logf("Range beyond end conversion successful: start=(%d,%d), end=(%d,%d)", + lspRangeBeyondEnd.Start.Line, lspRangeBeyondEnd.Start.Character, + lspRangeBeyondEnd.End.Line, lspRangeBeyondEnd.End.Character) +} \ No newline at end of file From 0f6e9fc3aab12a26d2149b116bc6184a750e3c59 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 17:35:43 +0000 Subject: [PATCH 4/5] Remove compiler tests - not needed for editor-related PRs Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- .../compiler/formatMultiByteTrailingNewlines.js | 9 --------- .../compiler/formatMultiByteTrailingNewlines.symbols | 7 ------- .../compiler/formatMultiByteTrailingNewlines.types | 7 ------- .../cases/compiler/formatMultiByteTrailingNewlines.ts | 7 ------- 4 files changed, 30 deletions(-) delete mode 100644 testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js delete mode 100644 testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols delete mode 100644 testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types delete mode 100644 testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts diff --git a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js deleted file mode 100644 index b0b4792053..0000000000 --- a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.js +++ /dev/null @@ -1,9 +0,0 @@ -//// [tests/cases/compiler/formatMultiByteTrailingNewlines.ts] //// - -//// [foo.ts] -"→" ; - - - -//// [foo.js] -"→"; diff --git a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols deleted file mode 100644 index b559d630af..0000000000 --- a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.symbols +++ /dev/null @@ -1,7 +0,0 @@ -//// [tests/cases/compiler/formatMultiByteTrailingNewlines.ts] //// - -=== foo.ts === - -"→" ; - - diff --git a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types b/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types deleted file mode 100644 index 3cab5698e9..0000000000 --- a/testdata/baselines/reference/compiler/formatMultiByteTrailingNewlines.types +++ /dev/null @@ -1,7 +0,0 @@ -//// [tests/cases/compiler/formatMultiByteTrailingNewlines.ts] //// - -=== foo.ts === -"→" ; ->"→" : "→" - - diff --git a/testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts b/testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts deleted file mode 100644 index fb2e970279..0000000000 --- a/testdata/tests/cases/compiler/formatMultiByteTrailingNewlines.ts +++ /dev/null @@ -1,7 +0,0 @@ -// @target: esnext -// @module: preserve -// @moduleResolution: bundler - -// @filename: foo.ts -"→" ; - From 444d1743018bfd4cc8045569d06df874330e9844 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Jul 2025 17:44:00 +0000 Subject: [PATCH 5/5] Remove redundant issue1389_test.go file Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- internal/ls/issue1389_test.go | 53 ----------------------------------- 1 file changed, 53 deletions(-) delete mode 100644 internal/ls/issue1389_test.go diff --git a/internal/ls/issue1389_test.go b/internal/ls/issue1389_test.go deleted file mode 100644 index 5c25a4d25d..0000000000 --- a/internal/ls/issue1389_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package ls - -import ( - "testing" - - "github.com/microsoft/typescript-go/internal/core" - "github.com/microsoft/typescript-go/internal/lsp/lsproto" -) - -// TestIssue1389Fix tests the specific scenario from issue #1389 -func TestIssue1389Fix(t *testing.T) { - t.Parallel() - - // This is the exact content from the issue report - content := "\"→\" ;\n\n\n" - - lineMap := ComputeLineStarts(content) - - converters := NewConverters(lsproto.PositionEncodingKindUTF16, func(fileName string) *LineMap { - return lineMap - }) - - script := &mockScript{ - fileName: "test.ts", - text: content, - } - - // Test the exact scenario that was causing the panic - textLen := len(content) - - // These should not panic - result1 := converters.PositionToLineAndCharacter(script, core.TextPos(textLen)) - t.Logf("Position at end of text (%d): line=%d, char=%d", textLen, result1.Line, result1.Character) - - result2 := converters.PositionToLineAndCharacter(script, core.TextPos(textLen+1)) - t.Logf("Position beyond end of text (%d): line=%d, char=%d", textLen+1, result2.Line, result2.Character) - - // Also test converting ranges, which is what the formatter does - textRange := core.NewTextRange(0, textLen) - lspRange := converters.ToLSPRange(script, textRange) - - t.Logf("Range conversion successful: start=(%d,%d), end=(%d,%d)", - lspRange.Start.Line, lspRange.Start.Character, - lspRange.End.Line, lspRange.End.Character) - - // Test with a range that extends beyond the text (this was the original issue) - textRangeBeyondEnd := core.NewTextRange(0, textLen+1) - lspRangeBeyondEnd := converters.ToLSPRange(script, textRangeBeyondEnd) - - t.Logf("Range beyond end conversion successful: start=(%d,%d), end=(%d,%d)", - lspRangeBeyondEnd.Start.Line, lspRangeBeyondEnd.Start.Character, - lspRangeBeyondEnd.End.Line, lspRangeBeyondEnd.End.Character) -} \ No newline at end of file