Skip to content

Commit 363ffa1

Browse files
authored
Use LocationLink in go to definition (#1884)
1 parent 691b316 commit 363ffa1

File tree

267 files changed

+464
-311
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

267 files changed

+464
-311
lines changed

internal/fourslash/_scripts/convertFourslash.mts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@ function parseFourslashStatement(statement: ts.Statement): Cmd[] | undefined {
200200
// - `verify.baselineGoToDefinition(...)` called getDefinitionAndBoundSpan
201201
// - `verify.baselineGetDefinitionAtPosition(...)` called getDefinitionAtPosition
202202
// LSP doesn't have two separate commands though.
203-
// It's unclear how we would model bound spans though.
204203
return parseBaselineGoToDefinitionArgs(func.text, callExpression.arguments);
205204
case "baselineRename":
206205
case "baselineRenameAtRangesWithText":
@@ -1081,6 +1080,10 @@ function parseBaselineGoToDefinitionArgs(
10811080
funcName: "baselineGoToDefinition" | "baselineGoToType" | "baselineGetDefinitionAtPosition",
10821081
args: readonly ts.Expression[],
10831082
): [VerifyBaselineGoToDefinitionCmd] | undefined {
1083+
let boundSpan: true | undefined;
1084+
if (funcName === "baselineGoToDefinition") {
1085+
boundSpan = true;
1086+
}
10841087
let kind: "verifyBaselineGoToDefinition" | "verifyBaselineGoToType";
10851088
switch (funcName) {
10861089
case "baselineGoToDefinition":
@@ -1105,6 +1108,7 @@ function parseBaselineGoToDefinitionArgs(
11051108
kind,
11061109
markers: [],
11071110
ranges: true,
1111+
boundSpan,
11081112
}];
11091113
}
11101114
else {
@@ -1116,6 +1120,7 @@ function parseBaselineGoToDefinitionArgs(
11161120
return [{
11171121
kind,
11181122
markers: newArgs,
1123+
boundSpan,
11191124
}];
11201125
}
11211126

@@ -1592,6 +1597,7 @@ interface VerifyBaselineFindAllReferencesCmd {
15921597
interface VerifyBaselineGoToDefinitionCmd {
15931598
kind: "verifyBaselineGoToDefinition" | "verifyBaselineGoToType";
15941599
markers: string[];
1600+
boundSpan?: true;
15951601
ranges?: boolean;
15961602
}
15971603

@@ -1710,20 +1716,20 @@ function generateBaselineDocumentHighlights({ args, preferences }: VerifyBaselin
17101716
return `f.VerifyBaselineDocumentHighlights(t, ${preferences}, ${args.join(", ")})`;
17111717
}
17121718

1713-
function generateBaselineGoToDefinition({ markers, ranges, kind }: VerifyBaselineGoToDefinitionCmd): string {
1714-
let goFunc;
1719+
function generateBaselineGoToDefinition({ markers, ranges, kind, boundSpan }: VerifyBaselineGoToDefinitionCmd): string {
1720+
const originalSelectionRange = boundSpan ? "true" : "false";
17151721
switch (kind) {
17161722
case "verifyBaselineGoToDefinition":
1717-
goFunc = "VerifyBaselineGoToDefinition";
1718-
break;
1723+
if (ranges || markers.length === 0) {
1724+
return `f.VerifyBaselineGoToDefinition(t, ${originalSelectionRange})`;
1725+
}
1726+
return `f.VerifyBaselineGoToDefinition(t, ${originalSelectionRange}, ${markers.join(", ")})`;
17191727
case "verifyBaselineGoToType":
1720-
goFunc = "VerifyBaselineGoToTypeDefinition";
1721-
break;
1722-
}
1723-
if (ranges || markers.length === 0) {
1724-
return `f.${goFunc}(t)`;
1728+
if (ranges || markers.length === 0) {
1729+
return `f.VerifyBaselineGoToTypeDefinition(t)`;
1730+
}
1731+
return `f.VerifyBaselineGoToTypeDefinition(t, ${markers.join(", ")})`;
17251732
}
1726-
return `f.${goFunc}(t, ${markers.join(", ")})`;
17271733
}
17281734

17291735
function generateGoToCommand({ funcName, args }: GoToCmd): string {

internal/fourslash/baselineutil.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ type baselineFourslashLocationsOptions struct {
131131

132132
startMarkerPrefix func(span lsproto.Location) *string
133133
endMarkerSuffix func(span lsproto.Location) *string
134+
135+
additionalLocation *lsproto.Location
134136
}
135137

136138
func (f *FourslashTest) getBaselineForLocationsWithFileContents(spans []lsproto.Location, options baselineFourslashLocationsOptions) string {
@@ -152,6 +154,7 @@ func (f *FourslashTest) getBaselineForGroupedLocationsWithFileContents(groupedRa
152154
// but don't want to print it twice at the end if it already
153155
// found in a file with ranges.
154156
foundMarker := false
157+
foundAdditionalLocation := false
155158

156159
baselineEntries := []string{}
157160
err := f.vfs.WalkDir("/", func(path string, d vfs.DirEntry, e error) error {
@@ -179,6 +182,10 @@ func (f *FourslashTest) getBaselineForGroupedLocationsWithFileContents(groupedRa
179182
foundMarker = true
180183
}
181184

185+
if options.additionalLocation != nil && options.additionalLocation.Uri == fileName {
186+
foundAdditionalLocation = true
187+
}
188+
182189
baselineEntries = append(baselineEntries, f.getBaselineContentForFile(path, content, ranges, nil, options))
183190
return nil
184191
})
@@ -187,6 +194,21 @@ func (f *FourslashTest) getBaselineForGroupedLocationsWithFileContents(groupedRa
187194
panic("walkdir error during fourslash baseline: " + err.Error())
188195
}
189196

197+
// In Strada, there is a bug where we only ever add additional spans to baselines if we haven't
198+
// already added the file to the baseline.
199+
if options.additionalLocation != nil && !foundAdditionalLocation {
200+
fileName := options.additionalLocation.Uri.FileName()
201+
if content, ok := f.vfs.ReadFile(fileName); ok {
202+
baselineEntries = append(
203+
baselineEntries,
204+
f.getBaselineContentForFile(fileName, content, []lsproto.Range{options.additionalLocation.Range}, nil, options),
205+
)
206+
if options.marker != nil && options.marker.FileName() == fileName {
207+
foundMarker = true
208+
}
209+
}
210+
}
211+
190212
if !foundMarker && options.marker != nil {
191213
// If we didn't find the marker in any file, we need to add it.
192214
markerFileName := options.marker.FileName()
@@ -195,7 +217,6 @@ func (f *FourslashTest) getBaselineForGroupedLocationsWithFileContents(groupedRa
195217
}
196218
}
197219

198-
// !!! foundAdditionalSpan
199220
// !!! skipDocumentContainingOnlyMarker
200221

201222
return strings.Join(baselineEntries, "\n\n")

internal/fourslash/fourslash.go

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,12 @@ var (
245245
ItemDefaults: &[]string{"commitCharacters", "editRange"},
246246
},
247247
}
248+
defaultDefinitionCapabilities = &lsproto.DefinitionClientCapabilities{
249+
LinkSupport: ptrTrue,
250+
}
251+
defaultTypeDefinitionCapabilities = &lsproto.TypeDefinitionClientCapabilities{
252+
LinkSupport: ptrTrue,
253+
}
248254
)
249255

250256
func getCapabilitiesWithDefaults(capabilities *lsproto.ClientCapabilities) *lsproto.ClientCapabilities {
@@ -278,6 +284,12 @@ func getCapabilitiesWithDefaults(capabilities *lsproto.ClientCapabilities) *lspr
278284
if capabilitiesWithDefaults.Workspace.Configuration == nil {
279285
capabilitiesWithDefaults.Workspace.Configuration = ptrTrue
280286
}
287+
if capabilitiesWithDefaults.TextDocument.Definition == nil {
288+
capabilitiesWithDefaults.TextDocument.Definition = defaultDefinitionCapabilities
289+
}
290+
if capabilitiesWithDefaults.TextDocument.TypeDefinition == nil {
291+
capabilitiesWithDefaults.TextDocument.TypeDefinition = defaultTypeDefinitionCapabilities
292+
}
281293
return &capabilitiesWithDefaults
282294
}
283295

@@ -1032,6 +1044,7 @@ func (f *FourslashTest) VerifyBaselineFindAllReferences(
10321044

10331045
func (f *FourslashTest) VerifyBaselineGoToDefinition(
10341046
t *testing.T,
1047+
includeOriginalSelectionRange bool,
10351048
markers ...string,
10361049
) {
10371050
referenceLocations := f.lookupMarkersOrGetRanges(t, markers)
@@ -1064,17 +1077,35 @@ func (f *FourslashTest) VerifyBaselineGoToDefinition(
10641077
}
10651078

10661079
var resultAsLocations []lsproto.Location
1080+
var additionalLocation *lsproto.Location
10671081
if result.Locations != nil {
10681082
resultAsLocations = *result.Locations
10691083
} else if result.Location != nil {
10701084
resultAsLocations = []lsproto.Location{*result.Location}
10711085
} else if result.DefinitionLinks != nil {
1072-
t.Fatalf("Unexpected definition response type at marker '%s': %T", *f.lastKnownMarkerName, result.DefinitionLinks)
1086+
var originRange *lsproto.Range
1087+
resultAsLocations = core.Map(*result.DefinitionLinks, func(link *lsproto.LocationLink) lsproto.Location {
1088+
if originRange != nil && originRange != link.OriginSelectionRange {
1089+
panic("multiple different origin ranges in definition links")
1090+
}
1091+
originRange = link.OriginSelectionRange
1092+
return lsproto.Location{
1093+
Uri: link.TargetUri,
1094+
Range: link.TargetSelectionRange,
1095+
}
1096+
})
1097+
if originRange != nil && includeOriginalSelectionRange {
1098+
additionalLocation = &lsproto.Location{
1099+
Uri: lsconv.FileNameToDocumentURI(f.activeFilename),
1100+
Range: *originRange,
1101+
}
1102+
}
10731103
}
10741104

10751105
f.addResultToBaseline(t, "goToDefinition", f.getBaselineForLocationsWithFileContents(resultAsLocations, baselineFourslashLocationsOptions{
1076-
marker: markerOrRange,
1077-
markerName: "/*GOTO DEF*/",
1106+
marker: markerOrRange,
1107+
markerName: "/*GOTO DEF*/",
1108+
additionalLocation: additionalLocation,
10781109
}))
10791110
}
10801111
}
@@ -1118,7 +1149,12 @@ func (f *FourslashTest) VerifyBaselineGoToTypeDefinition(
11181149
} else if result.Location != nil {
11191150
resultAsLocations = []lsproto.Location{*result.Location}
11201151
} else if result.DefinitionLinks != nil {
1121-
t.Fatalf("Unexpected type definition response type at marker '%s': %T", *f.lastKnownMarkerName, result.DefinitionLinks)
1152+
resultAsLocations = core.Map(*result.DefinitionLinks, func(link *lsproto.LocationLink) lsproto.Location {
1153+
return lsproto.Location{
1154+
Uri: link.TargetUri,
1155+
Range: link.TargetSelectionRange,
1156+
}
1157+
})
11221158
}
11231159

11241160
f.addResultToBaseline(t, "goToType", f.getBaselineForLocationsWithFileContents(resultAsLocations, baselineFourslashLocationsOptions{

internal/fourslash/tests/gen/ambientShorthandGotoDefinition_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ import [|/*importBang*/bang|] = require("jquery");
2424
f.VerifyQuickInfoAt(t, "useBar", "(alias) module \"jquery\"\nimport bar", "")
2525
f.VerifyQuickInfoAt(t, "useBaz", "(alias) module \"jquery\"\nimport baz", "")
2626
f.VerifyQuickInfoAt(t, "useBang", "(alias) module \"jquery\"\nimport bang = require(\"jquery\")", "")
27-
f.VerifyBaselineGoToDefinition(t, "useFoo", "importFoo", "useBar", "useBaz", "importBaz", "useBang", "importBang")
27+
f.VerifyBaselineGoToDefinition(t, true, "useFoo", "importFoo", "useBar", "useBaz", "importBaz", "useBang", "importBang")
2828
}

internal/fourslash/tests/gen/declarationMapGoToDefinition_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,5 @@ import * as mod from "./indexdef";
4949
const instance = new mod.Foo();
5050
instance.[|/*1*/methodName|]({member: 12});`
5151
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
52-
f.VerifyBaselineGoToDefinition(t, "1")
52+
f.VerifyBaselineGoToDefinition(t, true, "1")
5353
}

internal/fourslash/tests/gen/declarationMapsGoToDefinitionRelativeSourceRoot_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,5 @@ import * as mod from "./out/indexdef";
4949
const instance = new mod.Foo();
5050
instance.[|/*1*/methodName|]({member: 12});`
5151
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
52-
f.VerifyBaselineGoToDefinition(t, "1")
52+
f.VerifyBaselineGoToDefinition(t, true, "1")
5353
}

internal/fourslash/tests/gen/declarationMapsGoToDefinitionSameNameDifferentDirectory_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,5 @@ class Button extends [|/*1*/Control|] {
6666
}
6767
}`
6868
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
69-
f.VerifyBaselineGoToDefinition(t, "1", "3")
69+
f.VerifyBaselineGoToDefinition(t, true, "1", "3")
7070
}

internal/fourslash/tests/gen/declarationMapsOutOfDateMapping_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,5 @@ export class /*2*/Foo {
3434
import { Foo/*1*/ } from "a";`
3535
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
3636
f.GoToFile(t, "/home/src/workspaces/project/index.ts")
37-
f.VerifyBaselineGoToDefinition(t, "1")
37+
f.VerifyBaselineGoToDefinition(t, false, "1")
3838
}

internal/fourslash/tests/gen/definition01_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ var x = new n.Foo();
1717
// @Filename: a.ts
1818
/*2*/export class Foo {}`
1919
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
20-
f.VerifyBaselineGoToDefinition(t, "1")
20+
f.VerifyBaselineGoToDefinition(t, true, "1")
2121
}

internal/fourslash/tests/gen/definitionNameOnEnumMember_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ func TestDefinitionNameOnEnumMember(t *testing.T) {
1818
}
1919
var enumMember = e.[|/*1*/thirdMember|];`
2020
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
21-
f.VerifyBaselineGoToDefinition(t, "1")
21+
f.VerifyBaselineGoToDefinition(t, false, "1")
2222
}

0 commit comments

Comments
 (0)