Skip to content

Commit 8372401

Browse files
Fix completions crash and enable contextual type completions in array literals (#2258)
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: DanielRosenwasser <[email protected]>
1 parent 63a9e97 commit 8372401

File tree

4 files changed

+161
-0
lines changed

4 files changed

+161
-0
lines changed

internal/checker/services.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,18 @@ func (c *Checker) GetContextualDeclarationsForObjectLiteralElement(objectLiteral
671671
return result
672672
}
673673

674+
// GetContextualTypeForArrayElement returns the contextual type for an element at the given index
675+
// in an array with the given contextual type.
676+
func (c *Checker) GetContextualTypeForArrayElement(contextualArrayType *Type, elementIndex int) *Type {
677+
if contextualArrayType == nil {
678+
return nil
679+
}
680+
// Pass -1 for length, firstSpreadIndex, and lastSpreadIndex since we don't have
681+
// access to the actual array literal. This falls back to getting the iterated type
682+
// or checking numeric properties, which is appropriate for completion contexts.
683+
return c.getContextualTypeForElementExpression(contextualArrayType, elementIndex, -1, -1, -1)
684+
}
685+
674686
var knownGenericTypeNames = map[string]struct{}{
675687
"Array": {},
676688
"ArrayLike": {},
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
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/fourslash/tests/util"
8+
"github.com/microsoft/typescript-go/internal/testutil"
9+
)
10+
11+
// Test that string literal completions are suggested in tuple contexts
12+
// even without typing a quote character.
13+
func TestCompletionsInArrayLiteralWithContextualType(t *testing.T) {
14+
t.Parallel()
15+
16+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
17+
18+
// Test 1: Completions after `[` in a tuple should suggest string literals
19+
const content1 = `let y: ["foo" | "bar", string] = [/*a*/];`
20+
f1, done1 := fourslash.NewFourslash(t, nil /*capabilities*/, content1)
21+
defer done1()
22+
f1.VerifyCompletions(t, "a", &fourslash.CompletionsExpectedList{
23+
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
24+
CommitCharacters: &DefaultCommitCharacters,
25+
EditRange: Ignored,
26+
},
27+
Items: &fourslash.CompletionsExpectedItems{
28+
Includes: []fourslash.CompletionsExpectedItem{
29+
"\"foo\"",
30+
"\"bar\"",
31+
},
32+
},
33+
})
34+
35+
// Test 2: Completions after `,` in a tuple should provide contextual type for second element
36+
const content2 = `let z: ["a", "b" | "c"] = ["a", /*b*/];`
37+
f2, done2 := fourslash.NewFourslash(t, nil /*capabilities*/, content2)
38+
defer done2()
39+
f2.VerifyCompletions(t, "b", &fourslash.CompletionsExpectedList{
40+
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
41+
CommitCharacters: &DefaultCommitCharacters,
42+
EditRange: Ignored,
43+
},
44+
Items: &fourslash.CompletionsExpectedItems{
45+
Includes: []fourslash.CompletionsExpectedItem{
46+
"\"b\"",
47+
"\"c\"",
48+
},
49+
},
50+
})
51+
52+
// Test 3: Verify that properties named "-1" are NOT suggested in array literals
53+
// This was a bug in the old implementation where passing -1 as an index would
54+
// check for a property named "-1" and suggest its value
55+
const content3 = `let x: { "-1": "hello" } = [/*c*/];`
56+
f3, done3 := fourslash.NewFourslash(t, nil /*capabilities*/, content3)
57+
defer done3()
58+
f3.VerifyCompletions(t, "c", &fourslash.CompletionsExpectedList{
59+
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
60+
CommitCharacters: &DefaultCommitCharacters,
61+
EditRange: Ignored,
62+
},
63+
Items: &fourslash.CompletionsExpectedItems{
64+
Excludes: []string{
65+
"\"hello\"",
66+
},
67+
},
68+
})
69+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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/fourslash/tests/util"
8+
"github.com/microsoft/typescript-go/internal/testutil"
9+
)
10+
11+
// Test for issue: Completions crash in call to `new Map(...)`.
12+
// When requesting completions inside a Map constructor's array literal,
13+
// IndexOfNode returns -1 causing a panic in getContextualTypeForElementExpression
14+
// when trying to access array elements without a bounds check.
15+
func TestCompletionsInMapConstructorNoCrash(t *testing.T) {
16+
t.Parallel()
17+
18+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
19+
20+
// Test completion at position /*a*/ - before the string literal
21+
const content1 = `const m = new Map([
22+
[/*a*/'0', ['0', false]],
23+
]);`
24+
f1, done1 := fourslash.NewFourslash(t, nil /*capabilities*/, content1)
25+
defer done1()
26+
// Just verify that completions don't crash - accept any completion list
27+
f1.VerifyCompletions(t, "a", &fourslash.CompletionsExpectedList{
28+
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
29+
CommitCharacters: &DefaultCommitCharacters,
30+
EditRange: Ignored,
31+
},
32+
Items: &fourslash.CompletionsExpectedItems{},
33+
})
34+
35+
// Test completion at position /*b*/ - after the array literal
36+
const content2 = `const m = new Map([
37+
['0', ['0', false]]/*b*/,
38+
]);`
39+
f2, done2 := fourslash.NewFourslash(t, nil /*capabilities*/, content2)
40+
defer done2()
41+
// Just verify that completions don't crash - accept any completion list
42+
f2.VerifyCompletions(t, "b", &fourslash.CompletionsExpectedList{
43+
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
44+
CommitCharacters: &DefaultCommitCharacters,
45+
EditRange: Ignored,
46+
},
47+
Items: &fourslash.CompletionsExpectedItems{},
48+
})
49+
}

internal/ls/completions.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2967,6 +2967,37 @@ func getContextualType(previousToken *ast.Node, position int, file *ast.SourceFi
29672967
return typeChecker.GetContextualTypeForJsxAttribute(parent.Parent)
29682968
}
29692969
return nil
2970+
case ast.KindOpenBracketToken:
2971+
// When completing after `[` in an array literal (e.g., `[/*here*/]`),
2972+
// we should provide contextual type for the first element
2973+
if ast.IsArrayLiteralExpression(parent) {
2974+
contextualArrayType := typeChecker.GetContextualType(parent, checker.ContextFlagsNone)
2975+
if contextualArrayType != nil {
2976+
// Get the type for the first element (index 0)
2977+
return typeChecker.GetContextualTypeForArrayElement(contextualArrayType, 0)
2978+
}
2979+
}
2980+
return nil
2981+
case ast.KindCommaToken:
2982+
// When completing after `,` in an array literal (e.g., `[x, /*here*/]`),
2983+
// we should provide contextual type for the element after the comma
2984+
if ast.IsArrayLiteralExpression(parent) {
2985+
contextualArrayType := typeChecker.GetContextualType(parent, checker.ContextFlagsNone)
2986+
if contextualArrayType != nil {
2987+
// Count how many elements come before the cursor position
2988+
arrayLiteral := parent.AsArrayLiteralExpression()
2989+
elementIndex := 0
2990+
for _, elem := range arrayLiteral.Elements.Nodes {
2991+
if elem.Pos() < position {
2992+
elementIndex++
2993+
} else {
2994+
break
2995+
}
2996+
}
2997+
return typeChecker.GetContextualTypeForArrayElement(contextualArrayType, elementIndex)
2998+
}
2999+
}
3000+
return nil
29703001
case ast.KindQuestionToken:
29713002
// When completing after `?` in a ternary conditional (e.g., `foo(a ? /*here*/)`),
29723003
// we need to look at the parent conditional expression to find the contextual type.

0 commit comments

Comments
 (0)