-
Notifications
You must be signed in to change notification settings - Fork 726
Use LocationLink in go to definition
#1884
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
Purpose: Switch go to definition and go to type definition to return LocationLinks (with originSelectionRange) and extend fourslash baseline infrastructure to capture (optionally) the origin selection range, mirroring Strada behavior — including its known quirk of only showing the origin selection range when no definition appears in the same file.
Key changes:
- LanguageService and LSP server updated to pass and honor client definition capabilities, producing LocationLinks when supported.
- Fourslash harness enhanced (new default definition capabilities, updated VerifyBaselineGoToDefinition signature, baseline generation logic, conversion script adjustments) to optionally include the origin selection range.
- Widespread baseline test updates to reflect new [| |] origin selection markers and modified API usage.
Reviewed Changes
Copilot reviewed 266 out of 266 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/baselines/reference/fourslash/goToDefinition/importTypeNodeGoToDefinition.baseline.jsonc | Baseline updated to include origin selection ranges for import type nodes. |
| testdata/baselines/reference/fourslash/goToDefinition/goToTypeDefinition4.baseline.jsonc | Adds origin selection range for type definition reference. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionTypeOnlyImport.baseline.jsonc | Adds origin selection range for type-only import symbol. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionSimple.baseline.jsonc | Marks origin selection ranges on constructor calls. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionSignatureAlias_require.baseline.jsonc | Adds origin selection range for require alias references. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionShorthandProperty01.baseline.jsonc | Adds origin selection range for shorthand property. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionOverriddenMember8.baseline.jsonc | Marks override keyword origin selection. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionMultipleDefinitions.baseline.jsonc | Wraps multi-part marker span with selection markers. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionJsxNotSet.baseline.jsonc | Adds origin selection for JSX component reference. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionJsModuleNameAtImportName.baseline.jsonc | Adds origin selection range for JS module import name references. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionJsModuleName.baseline.jsonc | Adds origin selection range for module string literal. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionJsDocImportTag5.baseline.jsonc | Adds selection brackets inside JSDoc import tag parameter type. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionJsDocImportTag4.baseline.jsonc | Adds selection brackets in JSDoc import tag. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImports.baseline.jsonc | Marks origin selection ranges for imported identifiers. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames9.baseline.jsonc | Adds origin selection for destructured import usage. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames8.baseline.jsonc | Adds origin selection around import specifier. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames7.baseline.jsonc | Adds origin selection for default import reference. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames6.baseline.jsonc | Adds origin selection for require alias import. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames5.baseline.jsonc | Adds selection for export alias. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames4.baseline.jsonc | Adds selection for imported alias. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames3.baseline.jsonc | Adds selections for imported class references. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames2.baseline.jsonc | Adds selection for imported class specifier. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames11.baseline.jsonc | Adds selection for destructured require alias. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames10.baseline.jsonc | Same as above for different test variant. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImportedNames.baseline.jsonc | Adds selection for re-export specifier. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionImport1.baseline.jsonc | Adds selection around module string in import. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionExternalModuleName9.baseline.jsonc | Adds selection around external module string. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionExternalModuleName8.baseline.jsonc | Same pattern for export list. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionExternalModuleName7.baseline.jsonc | Same for import list. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionExternalModuleName6.baseline.jsonc | Adds selection for star import with pattern. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionExternalModuleName3.baseline.jsonc | Adds selection for require module string. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionExternalModuleName2.baseline.jsonc | Adds selection for relative module path in require. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionExternalModuleName.baseline.jsonc | Adds selection for relative module path with comment marker. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionDestructuredRequire2.baseline.jsonc | Adds selection for destructured require identifier. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionDestructuredRequire1.baseline.jsonc | Same for util require. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionDecorator.baseline.jsonc | Adds origin selection for decorator identifiers. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionClassConstructors.baseline.jsonc | Adds origin selection for class constructor references. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionCSSPatternAmbientModule.baseline.jsonc | Adds selection around CSS module import string. |
| testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionAcrossMultipleProjects.baseline.jsonc | Adds selection for cross-project reference. |
| testdata/baselines/reference/fourslash/goToDefinition/findAllRefsForDefaultExport.baseline.jsonc | Adds origin selection for default export reference. |
| testdata/baselines/reference/fourslash/goToDefinition/definition01.baseline.jsonc | Adds selection to external module path. |
| testdata/baselines/reference/fourslash/goToDefinition/definition.baseline.jsonc | Same change for variant. |
| testdata/baselines/reference/fourslash/goToDefinition/declarationMapsGoToDefinitionSameNameDifferentDirectory.baseline.jsonc | Adds origin selections for class and property in declaration map scenario. |
| testdata/baselines/reference/fourslash/goToDefinition/declarationMapsGoToDefinitionRelativeSourceRoot.baseline.jsonc | Adds origin selection for method name. |
| testdata/baselines/reference/fourslash/goToDefinition/declarationMapGoToDefinition.baseline.jsonc | Adds origin selection for method name (non-relative source root). |
| internal/project/untitled_test.go | Updates ProvideDefinition call to new signature with capabilities param. |
| internal/lsp/server.go | Passes client definition capabilities to LanguageService for definition/typeDefinition. |
| internal/ls/utilities.go | Adds toContextRange helper for context ranges (origin selection support). |
| internal/ls/findallreferences.go | Separates TextRange creation from LSP range construction; renames helper. |
| internal/ls/definition.go | Core logic: ProvideDefinition / ProvideTypeDefinition now produce LocationLinks with originSelectionRange when supported. |
| internal/fourslash/tests/gen/*.go | Numerous test updates: adapt to new VerifyBaselineGoToDefinition signature and origin selection range expectations. |
| internal/fourslash/fourslash.go | Adds default definition capabilities; rewrites VerifyBaselineGoToDefinition to handle LocationLinks and optional origin selection range. |
| internal/fourslash/baselineutil.go | Supports optional additional (origin) location in baselines replicating Strada behavior. |
| internal/fourslash/_scripts/convertFourslash.mts | Script updated to emit new Go calls including originSelectionRange boolean parameter and boundSpan handling. |
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.
Nice, thanks for tackling this!
This PR makes go to definition and go to type definition use
LocationLinks, which has roughly the same shape of what we return in Strada.It also adds support for location link's origin selection range in the baseline tests. In Strada, we had to
verify.baseline...methods for go to def, one which included the origin selection range (response body'stextSpan), and one which did not, so now we do the same here.The origin selection range also shows up in baselines, although in a buggy way: it only shows up if there are no definition locations in the same file. I believe this is a bug in Strada's fourslash, but I've preserved the behavior so it's easier to compare the baselines. Another oddity here is that Strada's fourslash does not use the
contextSpanproperty of definitions in the baselines, even though baselines for e.g. rename do use context spans (they show up as<| ... |>in baselines).I'll enable go to def baseline diffs in another PR, and this PR also helps reduce noise in the diffs.
Fixes #1439