Skip to content

Fix resolveNodeHandle to check not only child nodes but ancestor nodes#4033

Open
jet2jet wants to merge 1 commit into
microsoft:mainfrom
jet2jet:fix/api_resolve_node_handle
Open

Fix resolveNodeHandle to check not only child nodes but ancestor nodes#4033
jet2jet wants to merge 1 commit into
microsoft:mainfrom
jet2jet:fix/api_resolve_node_handle

Conversation

@jet2jet
Copy link
Copy Markdown

@jet2jet jet2jet commented May 23, 2026

Fixes #3938

Add ancestor traversals in resolveNodeHandle to detect the actual Node.
Since ast.GetNodeAtPosition only uses pos (not pos and end), some nodes may be hidden by their children (with same pos), such as PropertyAccessExpression (itself and expression node may be the same pos, and if expression is such as Expression node, ast.GetNodeAtPosition returns expression, not the parent PropertyAccessExpression).
This PR fixes those patterns.

Copilot AI review requested due to automatic review settings May 23, 2026 08:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR improves Session.resolveNodeHandle by adding an ancestor-walk fallback to locate the exact AST node when the initially resolved node’s kind/end don’t match the expected values.

Changes:

  • Added an ancestor traversal to attempt an exact node match before falling back to child traversal.

Comment thread internal/api/session.go
Comment on lines 1841 to +1847
if node.Kind != kind || node.End() != end {
// Try to find the exact node by walking ancestors
for parent := node.Parent; parent != nil && parent.Kind != ast.KindSourceFile; parent = parent.Parent {
if parent.Pos() == pos && parent.End() == end && parent.Kind == kind {
return parent, nil
}
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

kind == ast.KindSourceFile is already checked in the previous line (1830). Therefore it is not necessary to check for parent with parent.kind == ast.KindSourceFile here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Result of getTypeOfLocation for PropertyAccessExpression and ElementAccessExpression may be different between 6 and 7

2 participants