Skip to content

Commit 09a558a

Browse files
Copilotandrewbranch
andcommitted
Fix getVisualListRange bug in formatting (partial fix for import promotion crash) (#2569)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com> Co-authored-by: Andrew Branch <andrew@wheream.io> Co-authored-by: Andrew Branch <andrewbranch@users.noreply.github.com>
1 parent cafebdc commit 09a558a

4 files changed

Lines changed: 95 additions & 4 deletions

File tree

internal/format/indent.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,12 +342,13 @@ func getVisualListRange(node *ast.Node, list core.TextRange, sourceFile *ast.Sou
342342
} else {
343343
priorEnd = prior.End()
344344
}
345-
next := astnav.FindNextToken(prior, node, sourceFile)
345+
// Find the token that starts at or after list.End() using the scanner
346+
scan := scanner.GetScannerForSourceFile(sourceFile, list.End())
346347
var nextStart int
347-
if next == nil {
348+
if scan.Token() == ast.KindEndOfFile {
348349
nextStart = list.End()
349350
} else {
350-
nextStart = next.Pos()
351+
nextStart = scan.TokenStart()
351352
}
352353
return core.NewTextRange(priorEnd, nextStart)
353354
}

internal/format/indent_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package format_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/microsoft/typescript-go/internal/ast"
7+
"github.com/microsoft/typescript-go/internal/core"
8+
"github.com/microsoft/typescript-go/internal/format"
9+
"github.com/microsoft/typescript-go/internal/parser"
10+
"gotest.tools/v3/assert"
11+
)
12+
13+
func TestGetContainingList_NamedImports(t *testing.T) {
14+
t.Parallel()
15+
16+
text := `import type {
17+
AAA,
18+
BBB,
19+
} from "./bar";`
20+
21+
sourceFile := parser.ParseSourceFile(ast.SourceFileParseOptions{
22+
FileName: "/test.ts",
23+
Path: "/test.ts",
24+
}, text, core.ScriptKindTS)
25+
26+
// Find ImportSpecifier nodes (AAA and BBB)
27+
var importSpecifiers []*ast.Node
28+
forEachDescendantOfKind(sourceFile.AsNode(), ast.KindImportSpecifier, func(node *ast.Node) {
29+
importSpecifiers = append(importSpecifiers, node)
30+
})
31+
32+
assert.Assert(t, len(importSpecifiers) == 2, "Expected 2 import specifiers, got %d", len(importSpecifiers))
33+
34+
// Test GetContainingList for each import specifier
35+
for _, specifier := range importSpecifiers {
36+
list := format.GetContainingList(specifier, sourceFile)
37+
assert.Assert(t, list != nil, "GetContainingList should return non-nil for import specifier")
38+
assert.Assert(t, len(list.Nodes) == 2, "Expected list with 2 elements, got %d", len(list.Nodes))
39+
}
40+
}
41+
42+
func forEachDescendantOfKind(node *ast.Node, kind ast.Kind, action func(*ast.Node)) {
43+
node.ForEachChild(func(child *ast.Node) bool {
44+
if child.Kind == kind {
45+
action(child)
46+
}
47+
forEachDescendantOfKind(child, kind, action)
48+
return false
49+
})
50+
}

internal/fourslash/_scripts/crashingTests.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,4 @@ TestFindReferencesBindingPatternInJsdocNoCrash1
33
TestFindReferencesBindingPatternInJsdocNoCrash2
44
TestFormatDocumentWithTrivia
55
TestFormattingJsxTexts4
6-
TestImportNameCodeFix_importType8
76
TestQuickInfoBindingPatternInJsdocNoCrash1
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package fourslash_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/microsoft/typescript-go/internal/fourslash"
7+
"github.com/microsoft/typescript-go/internal/testutil"
8+
)
9+
10+
// Test case for crash when promoting type-only import to value import
11+
// when existing type imports precede the new value import
12+
// https://github.com/microsoft/typescript-go/issues/2559
13+
func TestCodeFixPromoteTypeOnlyOrderingCrash(t *testing.T) {
14+
fourslash.SkipIfFailing(t)
15+
t.Parallel()
16+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
17+
const content = `// @module: node18
18+
// @verbatimModuleSyntax: true
19+
// @Filename: /bar.ts
20+
export interface AAA {}
21+
export class BBB {}
22+
// @Filename: /foo.ts
23+
import type {
24+
AAA,
25+
BBB,
26+
} from "./bar";
27+
28+
let x: AAA = new BBB()`
29+
f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
30+
defer done()
31+
f.GoToFile(t, "/foo.ts")
32+
33+
// TODO: fix formatting
34+
f.VerifyImportFixAtPosition(t, []string{
35+
`import {
36+
BBB, type AAA,
37+
} from "./bar";
38+
39+
let x: AAA = new BBB()`,
40+
}, nil /*preferences*/)
41+
}

0 commit comments

Comments
 (0)