Skip to content

Fix deep property access in TypeScript. #482

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bm-w
Copy link

@bm-w bm-w commented Apr 3, 2025

A couple of stanzas were only matching member_expression syntax nodes that had as their object: field either another member_expression or an identifier syntax node. E.g.:

[
  (nested_identifier object:(_)@mod)
  (member_expression object:[(member_expression) (identifier)]@mod)
]@nested {
  node @nested.expr_def
  node @nested.type_def

  edge @nested.expr_def -> @mod.expr_def
  edge @nested.type_def -> @mod.type_def
}

However, if that object was itself a member_expression the stanzas would create edges to its expr_[dr]ef graph nodes indiscriminately, regardless of whether it would itself be matched and thus those nodes created. In a “deep” property access involving two nested member_expressions and a third non-matching inner syntax node, e.g. a parenthesized_expression, these stanzas would cause errors, e.g.:

0: Error executing statement edge @nested.expr_def -> @mod.expr_def at (2377, 3)
     src/stack-graphs.tsg:2377:3:
     2377 |   edge @nested.expr_def -> @mod.expr_def
          |   ^
     in stanza
     src/stack-graphs.tsg:2371:1:
     2371 | [
          | ^
     matching (member_expression) node
     test/expressions/access-properties.ts:15:5:
     15 |     (x).y.z;
        |     ^
1: Evaluating edge sink
2: Undefined scoped variable [syntax node member_expression (15, 5)].expr_def

The fragment of the parsed Tree-sitter tree for such a deep property access is:

(member_expression ; Matching fine, `expr_[dr]ef` graph nodes created, but edge creation would fail
  object: (member_expression; Not matching, so `expr_[dr]ef` graph nodes not created
    object: (parenthesized_expression …)
    …)
  …)

Separating graph nodes and edges creation fixes the issue. Copious testing (on our TS test suite and on all of microsoft/vscode) reveals no negative side effects.

@Copilot Copilot AI review requested due to automatic review settings April 3, 2025 14:04
@bm-w bm-w requested review from a team as code owners April 3, 2025 14:04
Copy link

@Copilot 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

This PR fixes the deep property access issue in TypeScript by separating graph nodes and edge creation to prevent errors when dealing with nested member expressions.

  • Added tests to cover various deep property access cases (normal, parenthesized, and subscript expressions).
  • The changes in the tests file validate that the error no longer occurs with non-matching inner syntax nodes.
Files not reviewed (1)
  • languages/tree-sitter-stack-graphs-typescript/src/stack-graphs.tsg: Language not supported

// ^ defined: 8
}

{ /// Deep with arenthesized expression
Copy link
Preview

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

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

The comment contains a spelling error 'arenthesized'; please change it to 'parenthesized' for clarity.

Suggested change
{ /// Deep with arenthesized expression
{ /// Deep with parenthesized expression

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@bm-w bm-w force-pushed the bm-w/fix-typescript-deep-property-access branch from 1654924 to 12b03af Compare April 3, 2025 14:05
@bjchambers
Copy link

I ran into this playing around with stack graphs. What's the time-line like for this (and the other fixes) being merged and/or released?

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.

2 participants