Skip to content

Commit ffe925d

Browse files
andrewbranchandrewbranch
authored
Go to definition: pass unverified through server (#43483)
* Pass `unverified` through server * Update protocol baseline * Fix unit tests * Fix other tests
1 parent 4b556e3 commit ffe925d

File tree

12 files changed

+39
-26
lines changed

12 files changed

+39
-26
lines changed

‎src/harness/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,8 @@ namespace ts.server {
306306
fileName: entry.file,
307307
textSpan: this.decodeSpan(entry),
308308
kind: ScriptElementKind.unknown,
309-
name: ""
309+
name: "",
310+
unverified: entry.unverified,
310311
})),
311312
textSpan: this.decodeSpan(body.textSpan, request.arguments.file)
312313
};

‎src/harness/fourslashImpl.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -685,10 +685,10 @@ namespace FourSlash {
685685
}
686686

687687
public verifyGoToDefinitionIs(endMarker: ArrayOrSingle<string>) {
688-
this.verifyGoToXWorker(toArray(endMarker), () => this.getGoToDefinition());
688+
this.verifyGoToXWorker(/*startMarker*/ undefined, toArray(endMarker), () => this.getGoToDefinition());
689689
}
690690

691-
public verifyGoToDefinition(arg0: any, endMarkerNames?: ArrayOrSingle<string> | { file: string }) {
691+
public verifyGoToDefinition(arg0: any, endMarkerNames?: ArrayOrSingle<string | { marker: string, unverified?: boolean }> | { file: string, unverified?: boolean }) {
692692
this.verifyGoToX(arg0, endMarkerNames, () => this.getGoToDefinitionAndBoundSpan());
693693
}
694694

@@ -705,7 +705,7 @@ namespace FourSlash {
705705
this.languageService.getTypeDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition));
706706
}
707707

708-
private verifyGoToX(arg0: any, endMarkerNames: ArrayOrSingle<string> | { file: string } | undefined, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
708+
private verifyGoToX(arg0: any, endMarkerNames: ArrayOrSingle<string | { marker: string, unverified?: boolean }> | { file: string, unverified?: boolean } | undefined, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
709709
if (endMarkerNames) {
710710
this.verifyGoToXPlain(arg0, endMarkerNames, getDefs);
711711
}
@@ -725,7 +725,7 @@ namespace FourSlash {
725725
}
726726
}
727727

728-
private verifyGoToXPlain(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string> | { file: string }, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
728+
private verifyGoToXPlain(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string | { marker: string, unverified?: boolean }> | { file: string, unverified?: boolean }, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
729729
for (const start of toArray(startMarkerNames)) {
730730
this.verifyGoToXSingle(start, endMarkerNames, getDefs);
731731
}
@@ -737,12 +737,12 @@ namespace FourSlash {
737737
}
738738
}
739739

740-
private verifyGoToXSingle(startMarkerName: string, endMarkerNames: ArrayOrSingle<string> | { file: string }, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
740+
private verifyGoToXSingle(startMarkerName: string, endMarkerNames: ArrayOrSingle<string | { marker: string, unverified?: boolean }> | { file: string, unverified?: boolean }, getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined) {
741741
this.goToMarker(startMarkerName);
742-
this.verifyGoToXWorker(toArray(endMarkerNames), getDefs, startMarkerName);
742+
this.verifyGoToXWorker(startMarkerName, toArray(endMarkerNames), getDefs, startMarkerName);
743743
}
744744

745-
private verifyGoToXWorker(endMarkers: readonly (string | { file: string })[], getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined, startMarkerName?: string) {
745+
private verifyGoToXWorker(startMarker: string | undefined, endMarkers: readonly (string | { marker?: string, file?: string, unverified?: boolean })[], getDefs: () => readonly ts.DefinitionInfo[] | ts.DefinitionInfoAndBoundSpan | undefined, startMarkerName?: string) {
746746
const defs = getDefs();
747747
let definitions: readonly ts.DefinitionInfo[];
748748
let testName: string;
@@ -763,8 +763,11 @@ namespace FourSlash {
763763
}
764764

765765
ts.zipWith(endMarkers, definitions, (endMarkerOrFileResult, definition, i) => {
766-
const expectedFileName = typeof endMarkerOrFileResult === "string" ? this.getMarkerByName(endMarkerOrFileResult).fileName : endMarkerOrFileResult.file;
767-
const expectedPosition = typeof endMarkerOrFileResult === "string" ? this.getMarkerByName(endMarkerOrFileResult).position : 0;
766+
const markerName = typeof endMarkerOrFileResult === "string" ? endMarkerOrFileResult : endMarkerOrFileResult.marker;
767+
const marker = markerName !== undefined ? this.getMarkerByName(markerName) : undefined;
768+
const expectedFileName = marker?.fileName || typeof endMarkerOrFileResult !== "string" && endMarkerOrFileResult.file;
769+
ts.Debug.assert(typeof expectedFileName === "string");
770+
const expectedPosition = marker?.position || 0;
768771
if (ts.comparePaths(expectedFileName, definition.fileName, /*ignoreCase*/ true) !== ts.Comparison.EqualTo || expectedPosition !== definition.textSpan.start) {
769772
const filesToDisplay = ts.deduplicate([expectedFileName, definition.fileName], ts.equateValues);
770773
const markers = [{ text: "EXPECTED", fileName: expectedFileName, position: expectedPosition }, { text: "ACTUAL", fileName: definition.fileName, position: definition.textSpan.start }];
@@ -777,7 +780,15 @@ namespace FourSlash {
777780
return `// @Filename: ${fileName}\n${fileContent}`;
778781
}).join("\n\n");
779782

780-
this.raiseError(`${testName} failed for definition ${endMarkerOrFileResult} (${i}): expected ${expectedFileName} at ${expectedPosition}, got ${definition.fileName} at ${definition.textSpan.start}\n\n${text}\n`);
783+
this.raiseError(`${testName} failed for definition ${markerName || expectedFileName} (${i}): expected ${expectedFileName} at ${expectedPosition}, got ${definition.fileName} at ${definition.textSpan.start}\n\n${text}\n`);
784+
}
785+
if (definition.unverified && (typeof endMarkerOrFileResult === "string" || !endMarkerOrFileResult.unverified)) {
786+
const isFileResult = typeof endMarkerOrFileResult !== "string" && !!endMarkerOrFileResult.file;
787+
this.raiseError(
788+
`${testName} failed for definition ${markerName || expectedFileName} (${i}): The actual definition was an \`unverified\` result. Use:\n\n` +
789+
` verify.goToDefinition(${startMarker === undefined ? "startMarker" : `"${startMarker}"`}, { ${isFileResult ? `file: "${expectedFileName}"` : `marker: "${markerName}"`}, unverified: true })\n\n` +
790+
`if this is expected.`
791+
);
781792
}
782793
});
783794
}

‎src/server/protocol.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1013,7 +1013,7 @@ namespace ts.server.protocol {
10131013
* Definition response message. Gives text range for definition.
10141014
*/
10151015
export interface DefinitionResponse extends Response {
1016-
body?: FileSpanWithContext[];
1016+
body?: DefinitionInfo[];
10171017
}
10181018

10191019
export interface DefinitionInfoAndBoundSpanResponse extends Response {

‎src/server/session.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,7 @@ namespace ts.server {
12241224
containerName: info.containerName,
12251225
kind: info.kind,
12261226
name: info.name,
1227+
...info.unverified && { unverified: info.unverified },
12271228
};
12281229
});
12291230
}
@@ -1300,8 +1301,8 @@ namespace ts.server {
13001301
}));
13011302
}
13021303

1303-
private mapDefinitionInfo(definitions: readonly DefinitionInfo[], project: Project): readonly protocol.FileSpanWithContext[] {
1304-
return definitions.map(def => this.toFileSpanWithContext(def.fileName, def.textSpan, def.contextSpan, project));
1304+
private mapDefinitionInfo(definitions: readonly DefinitionInfo[], project: Project): readonly protocol.DefinitionInfo[] {
1305+
return definitions.map(def => ({ ...this.toFileSpanWithContext(def.fileName, def.textSpan, def.contextSpan, project), ...def.unverified && { unverified: def.unverified } }));
13051306
}
13061307

13071308
/*

‎src/services/goToDefinition.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ namespace ts.GoToDefinition {
146146
end: node.getEnd(),
147147
fileName: node.text
148148
},
149-
unverified: !!verifiedFileName,
149+
unverified: !verifiedFileName,
150150
};
151151
}
152152
}

‎src/testRunner/unittests/tsserver/partialSemanticServer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ function fooB() { }`
218218
assert.deepEqual(response.definitions, [{
219219
file: file2.path,
220220
start: { line: 1, offset: 1 },
221-
end: { line: 1, offset: 1 }
221+
end: { line: 1, offset: 1 },
222222
}]);
223223
});
224224
});

‎src/testRunner/unittests/tsserver/projectReferencesSourcemap.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4436,4 +4436,4 @@ ${dependencyTs.content}`);
44364436
});
44374437
});
44384438
});
4439-
}
4439+
}

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7403,7 +7403,7 @@ declare namespace ts.server.protocol {
74037403
* Definition response message. Gives text range for definition.
74047404
*/
74057405
interface DefinitionResponse extends Response {
7406-
body?: FileSpanWithContext[];
7406+
body?: DefinitionInfo[];
74077407
}
74087408
interface DefinitionInfoAndBoundSpanResponse extends Response {
74097409
body?: DefinitionInfoAndBoundSpan;

‎tests/cases/fourslash/fourslash.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@ declare namespace FourSlashInterface {
286286
* `verify.goToDefinition(["a", "aa"], "b");` verifies that markers "a" and "aa" have the same definition "b".
287287
* `verify.goToDefinition("a", ["b", "bb"]);` verifies that "a" has multiple definitions available.
288288
*/
289-
goToDefinition(startMarkerNames: ArrayOrSingle<string>, fileResult: { file: string }): void;
290-
goToDefinition(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string>): void;
289+
goToDefinition(startMarkerNames: ArrayOrSingle<string>, fileResult: { file: string, unverified?: boolean }): void;
290+
goToDefinition(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string | { marker: string, unverified?: boolean }>): void;
291291
goToDefinition(startMarkerNames: ArrayOrSingle<string>, endMarkerNames: ArrayOrSingle<string>, range: Range): void;
292292
/** Performs `goToDefinition` for each pair. */
293293
goToDefinition(startsAndEnds: [ArrayOrSingle<string>, ArrayOrSingle<string>][]): void;

‎tests/cases/fourslash/goToDefinitionCSSPatternAmbientModule.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@
1414
// @Filename: index.ts
1515
//// import styles from [|/*1*/"./index.css"|];
1616

17-
verify.goToDefinition("1", ["2a", "2b"]);
17+
verify.goToDefinition("1", [{ marker: "2a", unverified: true }, "2b"]);

‎tests/cases/fourslash/goToDefinitionScriptImport.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@
1616
// not JS/TS, but if we can, you should be able to jump to it.
1717
//// import [|/*2*/"./stylez.css"|];
1818

19-
verify.goToDefinition("1", "1d");
20-
verify.goToDefinition("2", "2d");
19+
verify.goToDefinition("1", { marker: "1d", unverified: true });
20+
verify.goToDefinition("2", { marker: "2d", unverified: true });

‎tests/cases/fourslash/server/goToDefinitionScriptImportServer.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,6 @@
1919
// does not exist, but should return a response to it anyway so an editor can create it.
2020
//// import [|/*3*/"./foo.txt"|];
2121

22-
verify.goToDefinition("1", "1d");
23-
verify.goToDefinition("2", "2d");
24-
verify.goToDefinition("3", { file: "/foo.txt" });
22+
verify.goToDefinition("1", { marker: "1d", unverified: true });
23+
verify.goToDefinition("2", { marker: "2d", unverified: true });
24+
verify.goToDefinition("3", { file: "/foo.txt", unverified: true });

0 commit comments

Comments
 (0)