-
Notifications
You must be signed in to change notification settings - Fork 748
Port call hierarchy, fourslash tests #2171
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
Conversation
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.
Pull request overview
Ports call hierarchy support into the language server and adds comprehensive fourslash baseline tests, aligning outputs with LSP/VS Code expectations (e.g., mapping script to file and getter to property). Also improves baseline diff handling and introduces a new fourslash verification path for call hierarchy.
- Add call hierarchy handlers to the LSP server (prepare, incoming, outgoing) and declare capability.
- Implement call hierarchy analysis in the language service, including item creation, incoming/outgoing call site collection, and formatting for baseline outputs.
- Add extensive fourslash tests and reference baselines; adjust baseline utilities to handle call-hierarchy outputs and TypeScript path normalizations; minor fixes in references and AST utilities.
Reviewed changes
Copilot reviewed 68 out of 68 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/lsp/server.go | Registers call hierarchy handlers and advertises capability; enables prepare/incoming/outgoing requests. |
| internal/ls/callhierarchy.go | New implementation for resolving declarations, building CallHierarchyItems, and computing incoming/outgoing call sites; adds formatting helpers for baselines. |
| internal/fourslash/fourslash.go | Adds VerifyBaselineCallHierarchy with formatting of spans; special-cases test name normalization; utilities for line starts and caret rendering. |
| internal/fourslash/baselineutil.go | Adds Call Hierarchy baseline command and path normalization to match TypeScript baseline formats. |
| internal/fourslash/_scripts/convertFourslash.mts | Parses and generates call hierarchy baseline commands from fourslash specs. |
| internal/ls/symbols.go | Adjusts SymbolKind mapping (e.g., source files to file/module; accessors to property; adds more kinds). |
| internal/ls/findallreferences.go | Uses entryKindNode for constructor and default export references; replaces local helpers with AST utilities. |
| internal/ast/utilities.go | Exports GetAssignedName; adds helpers for call/new/element access target detection and property/element climb functions. |
| internal/testutil/baseline/baseline.go | Returns NoContent for empty diffs (no hunks) to reduce noise. |
| testdata/baselines/reference/... | Adds multiple call hierarchy baseline files and diffs; changes “script” kind to “file” and “getter” to “property” where applicable. |
| internal/fourslash/tests/gen/... | Adds numerous fourslash tests covering functions, classes, static blocks, decorators, JSX, tagged templates, cross-file, ambiguity cases, and accessors. |
| testdata/baselines/reference/fourslash/findAllReferences/findAllReferencesOfConstructor.baseline.jsonc | Updates references baseline to include constructor symbol ranges for “new a.C()”. |
| // Special case: TypeScript has "callHierarchyFunctionAmbiguity.N" with periods | ||
| switch name { | ||
| case "callHierarchyFunctionAmbiguity1": | ||
| name = "callHierarchyFunctionAmbiguity.1" | ||
| case "callHierarchyFunctionAmbiguity2": | ||
| name = "callHierarchyFunctionAmbiguity.2" | ||
| case "callHierarchyFunctionAmbiguity3": | ||
| name = "callHierarchyFunctionAmbiguity.3" | ||
| case "callHierarchyFunctionAmbiguity4": | ||
| name = "callHierarchyFunctionAmbiguity.4" | ||
| case "callHierarchyFunctionAmbiguity5": | ||
| name = "callHierarchyFunctionAmbiguity.5" | ||
| } |
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.
really annoying but these tests had dots in their names
|
Yeesh, either our tests are bad or I did something very wrong (well, I'm wrong either way); this does not seem to work in the editor past a single resolve. |
| @@= skipped -0, +0 lines =@@ | ||
| // === Call Hierarchy === | ||
| ╭ name: bar | ||
| -├ kind: getter |
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.
Couldn't we also fix this kind of diff (and same for script -> file below) in the baseline process so we get rid of these diff files?
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.
Oh, great idea.
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.
Done!

This is all very well tested; fourslash tests use baseline diffs, and the only diffs are ones related to the fact that tsserver differed from LSP, and so the code here is once again sort of a hybrid of the old code plus the post-processing steps VS Code does on top of tsserver, with the difference that I made script files not report themselves as
variable, which VS Code happened to do because of its fallback condition (now,file).