-
Notifications
You must be signed in to change notification settings - Fork 747
Fix nil pointer panic in FormatOnEnter #2043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: jakebailey <[email protected]>
Co-authored-by: jakebailey <[email protected]>
The panic in issue #2042 occurs when getStartLineAndCharacterForNode is called with a nil node from various places in the formatting code. Added nil check to handle this case gracefully. Co-authored-by: jakebailey <[email protected]>
Added nil checks before calling getStartLineAndCharacterForNode in childStartsOnTheSameLineWithElseInIfStatement and childIsUnindentedBranchOfConditionalExpression to be more defensive. Co-authored-by: jakebailey <[email protected]>
|
You're absolutely right. The panic was actually in |
internal/format/indent.go
Outdated
| if n == nil { | ||
| return 0, 0 | ||
| } |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
…nFromList The panic in issue #2042 occurs when list.Nodes[index] is nil. Added nil checks in deriveActualIndentationFromList to properly handle nil nodes in AST node lists, which can occur during parsing of incomplete or malformed code. Co-authored-by: jakebailey <[email protected]>
Fixes panic when pressing Enter in TypeScript files. The panic occurred when
deriveActualIndentationFromListaccessed nil nodes in AST node lists.Root Cause
The stack trace showed the panic was in
getStartLineAndCharacterForNode, but the actual issue was inderiveActualIndentationFromListwherelist.Nodes[index]orlist.Nodes[i]can be nil. The AST implementation allows nil nodes in node lists (as confirmed byNodeIsMissingchecking for nil), especially when parsing incomplete or malformed code.Changes
deriveActualIndentationFromList:list.Nodes[index]is nil before using it (return -1)list.Nodes[i]is nil in the loop before accessing properties (continue to next iteration)TestFormatOnEnter_NilNodesInListvalidates the fix with various edge cases including empty files, incomplete code, and malformed syntaxOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.