Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions internal/format/format_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package format_test

import (
"context"
"strings"
"testing"

Expand Down Expand Up @@ -58,3 +59,67 @@ func TestFormatNoTrailingNewline(t *testing.T) {
})
}
}

// Test for panic handling request textDocument/onTypeFormatting (issue #2042)
// The panic occurs when getStartLineAndCharacterForNode is called with a nil node
func TestFormatOnEnter_NilNodeHandling(t *testing.T) {
t.Parallel()

// Test various edge cases that could lead to nil nodes being passed
// to getStartLineAndCharacterForNode
testCases := []struct {
name string
text string
position int // position where enter is pressed
}{
{
name: "empty file",
text: "",
position: 0,
},
{
name: "simple statement",
text: "const x = 1;",
position: 12,
},
{
name: "file with newline",
text: "const x = 1;\n",
position: 13,
},
{
name: "incomplete code",
text: "if (",
position: 4,
},
{
name: "malformed if-else",
text: "if(a){}\nelse",
position: 12,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
sourceFile := parser.ParseSourceFile(ast.SourceFileParseOptions{
FileName: "/test.ts",
Path: "/test.ts",
}, tc.text, core.ScriptKindTS)

ctx := format.WithFormatCodeSettings(context.Background(), &format.FormatCodeSettings{
EditorSettings: format.EditorSettings{
TabSize: 4,
IndentSize: 4,
NewLineCharacter: "\n",
ConvertTabsToSpaces: true,
IndentStyle: format.IndentStyleSmart,
},
}, "\n")

// This should not panic even with nil nodes
edits := format.FormatOnEnter(ctx, sourceFile, tc.position)
_ = edits // Just ensuring no panic
})
}
}
14 changes: 12 additions & 2 deletions internal/format/indent.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ func findFirstNonWhitespaceCharacterAndColumn(startPos int, endPos int, sourceFi
func childStartsOnTheSameLineWithElseInIfStatement(parent *ast.Node, child *ast.Node, childStartLine int, sourceFile *ast.SourceFile) bool {
if parent.Kind == ast.KindIfStatement && parent.AsIfStatement().ElseStatement == child {
elseKeyword := astnav.FindPrecedingToken(sourceFile, child.Pos())
if elseKeyword == nil {
return false
}
debug.AssertIsDefined(elseKeyword)
elseKeywordStartLine, _ := getStartLineAndCharacterForNode(elseKeyword, sourceFile)
return elseKeywordStartLine == childStartLine
Expand All @@ -250,6 +253,9 @@ func childStartsOnTheSameLineWithElseInIfStatement(parent *ast.Node, child *ast.
}

func getStartLineAndCharacterForNode(n *ast.Node, sourceFile *ast.SourceFile) (line int, character int) {
if n == nil {
return 0, 0
}
Copy link
Member

Choose a reason for hiding this comment

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

@copilot this seems weird; we should just never be passing nil into this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I've fixed it properly now (commit a57e81c) by handling nil nodes in deriveActualIndentationFromList where they can actually occur. The AST can contain nil nodes in lists (especially with incomplete/malformed code), which is why NodeIsMissing explicitly checks for nil.

return scanner.GetECMALineAndCharacterOfPosition(sourceFile, scanner.GetTokenPosOfNode(n, sourceFile, false))
}

Expand Down Expand Up @@ -530,8 +536,12 @@ func childIsUnindentedBranchOfConditionalExpression(parent *ast.Node, child *ast
// ? 1 : ( L1: whenTrue indented because it's on a new line
// 0 L2: indented two stops, one because whenTrue was indented
// ); and one because of the parentheses spanning multiple lines
trueStartLine, _ := getStartLineAndCharacterForNode(parent.AsConditionalExpression().WhenTrue, sourceFile)
trueEndLine, _ := scanner.GetECMALineAndCharacterOfPosition(sourceFile, parent.AsConditionalExpression().WhenTrue.End())
whenTrue := parent.AsConditionalExpression().WhenTrue
if whenTrue == nil {
return false
}
trueStartLine, _ := getStartLineAndCharacterForNode(whenTrue, sourceFile)
trueEndLine, _ := scanner.GetECMALineAndCharacterOfPosition(sourceFile, whenTrue.End())
return conditionEndLine == trueStartLine && trueEndLine == childStartLine
}
}
Expand Down