From 69f73eba168f509aa709bb42706d14370c638d9c Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Wed, 2 May 2018 14:16:39 -0700 Subject: [PATCH 1/4] Return mapped locations in alternate fields --- src/server/session.ts | 29 ++++++++++++++++++++++------- src/services/services.ts | 8 ++++++-- src/services/types.ts | 11 ++++++++--- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index 8d6defa1562..6cd3a7df560 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -665,9 +665,8 @@ namespace ts.server { if (simplifiedResult) { return this.mapDefinitionInfo(definitions, project); } - else { - return definitions; - } + + return definitions.map(Session.mapToOriginalLocation); } private getDefinitionAndBoundSpan(args: protocol.FileLocationRequestArgs, simplifiedResult: boolean): protocol.DefinitionInfoAndBoundSpan | DefinitionInfoAndBoundSpan { @@ -691,13 +690,30 @@ namespace ts.server { }; } - return definitionAndBoundSpan; + return { + ...definitionAndBoundSpan, + definitions: definitionAndBoundSpan.definitions.map(Session.mapToOriginalLocation) + }; } private mapDefinitionInfo(definitions: ReadonlyArray, project: Project): ReadonlyArray { return definitions.map(def => this.toFileSpan(def.fileName, def.textSpan, project)); } + private static mapToOriginalLocation(def: T): T { + if (def.originalFileName) { + Debug.assert(def.originalTextSpan !== undefined, "originalTextSpan should be present if originalFileName is"); + return { + ...def, + fileName: def.originalFileName, + textSpan: def.originalTextSpan, + targetFileName: def.fileName, + targetTextSpan: def.textSpan + }; + } + return def; + } + private toFileSpan(fileName: string, textSpan: TextSpan, project: Project): protocol.FileSpan { const ls = project.getLanguageService(); const start = ls.toLineColumnOffset(fileName, textSpan.start); @@ -732,9 +748,8 @@ namespace ts.server { if (simplifiedResult) { return implementations.map(({ fileName, textSpan }) => this.toFileSpan(fileName, textSpan, project)); } - else { - return implementations; - } + + return implementations.map(Session.mapToOriginalLocation); } private getOccurrences(args: protocol.FileLocationRequestArgs): ReadonlyArray { diff --git a/src/services/services.ts b/src/services/services.ts index 700b07085a0..401576c57b8 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1634,7 +1634,9 @@ namespace ts { textSpan: { start: newLoc.position, length: info.textSpan.length - } + }, + originalFileName: info.fileName, + originalTextSpan: info.textSpan }) ); @@ -1678,7 +1680,9 @@ namespace ts { textSpan: { start: newLoc.position, length: info.textSpan.length - } + }, + originalFileName: info.fileName, + originalTextSpan: info.textSpan }) ); diff --git a/src/services/types.ts b/src/services/types.ts index cf2e58a3712..0d15213cb12 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -542,6 +542,13 @@ namespace ts { export interface DocumentSpan { textSpan: TextSpan; fileName: string; + + /** + * If the span represents a location that was remapped (e.g. via a .d.ts.map file), + * then the original filename and span will be specified here + */ + originalTextSpan?: TextSpan; + originalFileName?: string; } export interface RenameLocation extends DocumentSpan { @@ -654,9 +661,7 @@ namespace ts { indentMultiLineObjectLiteralBeginningOnBlankLine?: boolean; } - export interface DefinitionInfo { - fileName: string; - textSpan: TextSpan; + export interface DefinitionInfo extends DocumentSpan { kind: ScriptElementKind; name: string; containerKind: ScriptElementKind; From 86145eedb13c27c8ce8c3917347836ce2b3fc233 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Mon, 14 May 2018 09:58:32 -0700 Subject: [PATCH 2/4] Update API baselines --- tests/baselines/reference/api/tsserverlibrary.d.ts | 11 ++++++++--- tests/baselines/reference/api/typescript.d.ts | 10 +++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index c069dbf4d04..42e15980a9c 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4701,6 +4701,12 @@ declare namespace ts { interface DocumentSpan { textSpan: TextSpan; fileName: string; + /** + * If the span represents a location that was remapped (e.g. via a .d.ts.map file), + * then the original filename and span will be specified here + */ + originalTextSpan?: TextSpan; + originalFileName?: string; } interface RenameLocation extends DocumentSpan { } @@ -4798,9 +4804,7 @@ declare namespace ts { insertSpaceBeforeTypeAnnotation?: boolean; indentMultiLineObjectLiteralBeginningOnBlankLine?: boolean; } - interface DefinitionInfo { - fileName: string; - textSpan: TextSpan; + interface DefinitionInfo extends DocumentSpan { kind: ScriptElementKind; name: string; containerKind: ScriptElementKind; @@ -8410,6 +8414,7 @@ declare namespace ts.server { private getDefinition; private getDefinitionAndBoundSpan; private mapDefinitionInfo; + private static mapToOriginalLocation; private toFileSpan; private getTypeDefinition; private getImplementation; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 4887de7fd2f..05425f80539 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4701,6 +4701,12 @@ declare namespace ts { interface DocumentSpan { textSpan: TextSpan; fileName: string; + /** + * If the span represents a location that was remapped (e.g. via a .d.ts.map file), + * then the original filename and span will be specified here + */ + originalTextSpan?: TextSpan; + originalFileName?: string; } interface RenameLocation extends DocumentSpan { } @@ -4798,9 +4804,7 @@ declare namespace ts { insertSpaceBeforeTypeAnnotation?: boolean; indentMultiLineObjectLiteralBeginningOnBlankLine?: boolean; } - interface DefinitionInfo { - fileName: string; - textSpan: TextSpan; + interface DefinitionInfo extends DocumentSpan { kind: ScriptElementKind; name: string; containerKind: ScriptElementKind; From 64b1c23a9bd09ca3f78e4ae35911afd5f65ef849 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Mon, 14 May 2018 10:54:40 -0700 Subject: [PATCH 3/4] Push through original mapping location --- src/services/services.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 401576c57b8..628892960af 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1600,10 +1600,10 @@ namespace ts { function makeGetTargetOfMappedPosition( extract: (original: TIn) => sourcemaps.SourceMappableLocation, - create: (result: sourcemaps.SourceMappableLocation, original: TIn) => TIn + create: (result: sourcemaps.SourceMappableLocation, original: TIn, firstOriginal: TIn) => TIn ) { return getTargetOfMappedPosition; - function getTargetOfMappedPosition(input: TIn): TIn { + function getTargetOfMappedPosition(input: TIn, firstOriginal = input): TIn { const info = extract(input); if (endsWith(info.fileName, Extension.Dts)) { let file: SourceFileLike = program.getSourceFile(info.fileName); @@ -1617,7 +1617,7 @@ namespace ts { const mapper = getSourceMapper(info.fileName, file); const newLoc = mapper.getOriginalPosition(info); if (newLoc === info) return input; - return getTargetOfMappedPosition(create(newLoc, input)); + return getTargetOfMappedPosition(create(newLoc, input, firstOriginal), firstOriginal); } return input; } @@ -1625,7 +1625,7 @@ namespace ts { const getTargetOfMappedDeclarationInfo = makeGetTargetOfMappedPosition( (info: DefinitionInfo) => ({ fileName: info.fileName, position: info.textSpan.start }), - (newLoc, info) => ({ + (newLoc, info, firstOriginal) => ({ containerKind: info.containerKind, containerName: info.containerName, fileName: newLoc.fileName, @@ -1635,13 +1635,13 @@ namespace ts { start: newLoc.position, length: info.textSpan.length }, - originalFileName: info.fileName, - originalTextSpan: info.textSpan + originalFileName: firstOriginal.fileName, + originalTextSpan: firstOriginal.textSpan }) ); function getTargetOfMappedDeclarationFiles(infos: ReadonlyArray): DefinitionInfo[] { - return map(infos, getTargetOfMappedDeclarationInfo); + return map(infos, d => getTargetOfMappedDeclarationInfo(d)); } /// Goto definition @@ -1687,7 +1687,7 @@ namespace ts { ); function getTargetOfMappedImplementationLocations(infos: ReadonlyArray): ImplementationLocation[] { - return map(infos, getTargetOfMappedImplementationLocation); + return map(infos, d => getTargetOfMappedImplementationLocation(d)); } function getImplementationAtPosition(fileName: string, position: number): ImplementationLocation[] { From f01338fa338c3efdd628cc45970593a091de1b9f Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Mon, 14 May 2018 18:27:21 -0700 Subject: [PATCH 4/4] Comments/naming --- src/server/session.ts | 7 +++++++ src/services/services.ts | 12 ++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index 6cd3a7df560..d0c58b20922 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -700,6 +700,13 @@ namespace ts.server { return definitions.map(def => this.toFileSpan(def.fileName, def.textSpan, project)); } + /* + * When we map a .d.ts location to .ts, Visual Studio gets confused because there's no associated Roslyn Document in + * the same project which corresponds to the file. VS Code has no problem with this, and luckily we have two protocols. + * This retains the existing behavior for the "simplified" (VS Code) protocol but stores the .d.ts location in a + * set of additional fields, and does the reverse for VS (store the .d.ts location where + * it used to be and stores the .ts location in the additional fields). + */ private static mapToOriginalLocation(def: T): T { if (def.originalFileName) { Debug.assert(def.originalTextSpan !== undefined, "originalTextSpan should be present if originalFileName is"); diff --git a/src/services/services.ts b/src/services/services.ts index 628892960af..3fcd4e8e151 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1600,10 +1600,10 @@ namespace ts { function makeGetTargetOfMappedPosition( extract: (original: TIn) => sourcemaps.SourceMappableLocation, - create: (result: sourcemaps.SourceMappableLocation, original: TIn, firstOriginal: TIn) => TIn + create: (result: sourcemaps.SourceMappableLocation, unmapped: TIn, original: TIn) => TIn ) { return getTargetOfMappedPosition; - function getTargetOfMappedPosition(input: TIn, firstOriginal = input): TIn { + function getTargetOfMappedPosition(input: TIn, original = input): TIn { const info = extract(input); if (endsWith(info.fileName, Extension.Dts)) { let file: SourceFileLike = program.getSourceFile(info.fileName); @@ -1617,7 +1617,7 @@ namespace ts { const mapper = getSourceMapper(info.fileName, file); const newLoc = mapper.getOriginalPosition(info); if (newLoc === info) return input; - return getTargetOfMappedPosition(create(newLoc, input, firstOriginal), firstOriginal); + return getTargetOfMappedPosition(create(newLoc, input, original), original); } return input; } @@ -1625,7 +1625,7 @@ namespace ts { const getTargetOfMappedDeclarationInfo = makeGetTargetOfMappedPosition( (info: DefinitionInfo) => ({ fileName: info.fileName, position: info.textSpan.start }), - (newLoc, info, firstOriginal) => ({ + (newLoc, info, original) => ({ containerKind: info.containerKind, containerName: info.containerName, fileName: newLoc.fileName, @@ -1635,8 +1635,8 @@ namespace ts { start: newLoc.position, length: info.textSpan.length }, - originalFileName: firstOriginal.fileName, - originalTextSpan: firstOriginal.textSpan + originalFileName: original.fileName, + originalTextSpan: original.textSpan }) );