From da2aa9781e582bebd531750a4b8a81d9f8be2906 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 12 Jun 2019 14:29:43 -0700 Subject: [PATCH] Revert to using spread instead of mutating value later --- src/harness/client.ts | 13 ++-- src/harness/fourslash.ts | 24 +++--- src/server/session.ts | 74 ++++++++----------- src/services/findAllReferences.ts | 40 +++++----- src/services/goToDefinition.ts | 16 ++-- src/services/services.ts | 31 ++++---- .../unittests/tsserver/declarationFileMaps.ts | 12 +-- src/testRunner/unittests/tsserver/helpers.ts | 32 ++++---- 8 files changed, 115 insertions(+), 127 deletions(-) diff --git a/src/harness/client.ts b/src/harness/client.ts index ecfbbddecd0..777c69f9a6c 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -396,11 +396,14 @@ namespace ts.server { for (const entry of body.locs) { const fileName = entry.file; for (const { start, end, declarationStart, declarationEnd, ...prefixSuffixText } of entry.locs) { - const renameLocation: RenameLocation = { textSpan: this.decodeSpan({ start, end }, fileName), fileName, ...prefixSuffixText }; - if (declarationStart !== undefined) { - renameLocation.declarationSpan = this.decodeSpan({ start: declarationStart, end: declarationEnd! }, fileName); - } - locations.push(renameLocation); + locations.push({ + textSpan: this.decodeSpan({ start, end }, fileName), + fileName, + ...(declarationStart !== undefined ? + { declarationSpan: this.decodeSpan({ start: declarationStart, end: declarationEnd! }, fileName) } : + undefined), + ...prefixSuffixText + }); } } diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 252d9723209..fd6ada47e3c 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -957,17 +957,16 @@ namespace FourSlash { definition: typeof definition === "string" ? definition : { ...definition, range: ts.createTextSpanFromRange(definition.range) }, references: ranges.map(r => { const { isWriteAccess = false, isDefinition = false, isInString, declarationRangeIndex } = (r.marker && r.marker.data || {}) as { isWriteAccess?: boolean, isDefinition?: boolean, isInString?: true, declarationRangeIndex?: number }; - const result: ts.ReferenceEntry = { + return { fileName: r.fileName, textSpan: ts.createTextSpanFromRange(r), isWriteAccess, isDefinition, + ...(declarationRangeIndex !== undefined ? + { declarationSpan: ts.createTextSpanFromRange(this.getRanges()[declarationRangeIndex]) } : + undefined), ...(isInString ? { isInString: true } : undefined), }; - if (declarationRangeIndex !== undefined) { - result.declarationSpan = ts.createTextSpanFromRange(this.getRanges()[declarationRangeIndex]); - } - return result; }), })); @@ -1192,14 +1191,17 @@ Actual: ${stringify(fullActual)}`); const sort = (locations: ReadonlyArray | undefined) => locations && ts.sort(locations, (r1, r2) => ts.compareStringsCaseSensitive(r1.fileName, r2.fileName) || r1.textSpan.start - r2.textSpan.start); - assert.deepEqual(sort(references), sort(ranges.map(rangeOrOptions => { + assert.deepEqual(sort(references), sort(ranges.map((rangeOrOptions): ts.RenameLocation => { const { range, ...prefixSuffixText } = "range" in rangeOrOptions ? rangeOrOptions : { range: rangeOrOptions }; - const result: ts.RenameLocation = { fileName: range.fileName, textSpan: ts.createTextSpanFromRange(range), ...prefixSuffixText }; const { declarationRangeIndex } = (range.marker && range.marker.data || {}) as { declarationRangeIndex?: number; }; - if (declarationRangeIndex !== undefined) { - result.declarationSpan = ts.createTextSpanFromRange(this.getRanges()[declarationRangeIndex]); - } - return result; + return { + fileName: range.fileName, + textSpan: ts.createTextSpanFromRange(range), + ...(declarationRangeIndex !== undefined ? + { declarationSpan: ts.createTextSpanFromRange(this.getRanges()[declarationRangeIndex]) } : + undefined), + ...prefixSuffixText + }; }))); } } diff --git a/src/server/session.ts b/src/server/session.ts index 118a1d93468..7ecc569b6a8 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1066,14 +1066,11 @@ namespace ts.server { } private toDeclarationFileSpan(fileName: string, textSpan: TextSpan, declarationSpan: TextSpan | undefined, project: Project): protocol.DeclarationFileSpan { - const result = this.toFileSpan(fileName, textSpan, project) as protocol.DeclarationFileSpan; - if (declarationSpan) { - const declaration = this.toFileSpan(fileName, declarationSpan, project); - result.declarationStart = declaration.start; - result.declarationEnd = declaration.end; - } - return result; - + const fileSpan = this.toFileSpan(fileName, textSpan, project); + const declaration = declarationSpan && this.toFileSpan(fileName, declarationSpan, project); + return declaration ? + { ...fileSpan, declarationStart: declaration.start, declarationEnd: declaration.end } : + fileSpan; } private getTypeDefinition(args: protocol.FileLocationRequestArgs): ReadonlyArray { @@ -1099,36 +1096,27 @@ namespace ts.server { const { file, project } = this.getFileAndProject(args); const position = this.getPositionInFile(args, file); const implementations = this.mapImplementationLocations(project.getLanguageService().getImplementationAtPosition(file, position) || emptyArray, project); - if (simplifiedResult) { - return implementations.map(({ fileName, textSpan, declarationSpan }) => this.toDeclarationFileSpan(fileName, textSpan, declarationSpan, project)); - } - - return implementations.map(Session.mapToOriginalLocation); + return simplifiedResult ? + implementations.map(({ fileName, textSpan, declarationSpan }) => this.toDeclarationFileSpan(fileName, textSpan, declarationSpan, project)) : + implementations.map(Session.mapToOriginalLocation); } private getOccurrences(args: protocol.FileLocationRequestArgs): ReadonlyArray { const { file, project } = this.getFileAndProject(args); - const position = this.getPositionInFile(args, file); - const occurrences = project.getLanguageService().getOccurrencesAtPosition(file, position); - - if (!occurrences) { - return emptyArray; - } - - return occurrences.map(occurrence => { - const { fileName, isWriteAccess, textSpan, isInString, declarationSpan } = occurrence; - const scriptInfo = project.getScriptInfo(fileName)!; - const result = toProtocolDeclarationTextSpan(textSpan, declarationSpan, scriptInfo) as protocol.OccurrencesResponseItem; - result.file = fileName; - result.isWriteAccess = isWriteAccess; - // no need to serialize the property if it is not true - if (isInString) { - result.isInString = isInString; - } - return result; - }); + return occurrences ? + occurrences.map(occurrence => { + const { fileName, isWriteAccess, textSpan, isInString, declarationSpan } = occurrence; + const scriptInfo = project.getScriptInfo(fileName)!; + return { + ...toProtocolDeclarationTextSpan(textSpan, declarationSpan, scriptInfo), + file: fileName, + isWriteAccess, + ...(isInString ? { isInString } : undefined) + }; + }) : + emptyArray; } private getSyntacticDiagnosticsSync(args: protocol.SyntacticDiagnosticsSyncRequestArgs): ReadonlyArray | ReadonlyArray { @@ -1178,11 +1166,10 @@ namespace ts.server { const scriptInfo = project.getScriptInfo(fileName)!; return { file: fileName, - highlightSpans: highlightSpans.map(({ textSpan, kind, declarationSpan }) => { - const result = toProtocolDeclarationTextSpan(textSpan, declarationSpan, scriptInfo) as protocol.HighlightSpan; - result.kind = kind; - return result; - }) + highlightSpans: highlightSpans.map(({ textSpan, kind, declarationSpan }) => ({ + ...toProtocolDeclarationTextSpan(textSpan, declarationSpan, scriptInfo), + kind + })) }; }); } @@ -1328,8 +1315,7 @@ namespace ts.server { isDefinition }; })); - const result: protocol.ReferencesResponseBody = { refs, symbolName, symbolStartOffset, symbolDisplayString }; - return result; + return { refs, symbolName, symbolStartOffset, symbolDisplayString }; } /** * @param fileName is the name of the file to be opened @@ -2580,11 +2566,11 @@ namespace ts.server { } function toProtocolDeclarationTextSpan(span: TextSpan, declarationSpan: TextSpan | undefined, scriptInfo: ScriptInfo): protocol.DeclarationTextSpan { - const result = toProcolTextSpan(span, scriptInfo) as protocol.DeclarationTextSpan; - if (!declarationSpan) return result; - result.declarationStart = scriptInfo.positionToLineOffset(declarationSpan.start); - result.declarationEnd = scriptInfo.positionToLineOffset(textSpanEnd(declarationSpan)); - return result; + const textSpan = toProcolTextSpan(span, scriptInfo); + const declarationTextSpan = declarationSpan && toProcolTextSpan(declarationSpan, scriptInfo); + return declarationTextSpan ? + { ...textSpan, declarationStart: declarationTextSpan.start, declarationEnd: declarationTextSpan.end } : + textSpan; } function convertTextChangeToCodeEdit(change: TextChange, scriptInfo: ScriptInfoOrConfig): protocol.CodeEdit { diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index b8971d74a41..6eb6819e433 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -161,15 +161,14 @@ namespace ts.FindAllReferences { } } - export function setDeclarationSpan(span: DocumentSpan, sourceFile: SourceFile, declaration?: DeclarationNode) { - if (declaration) { - const declarationSpan = isDeclarationNodeWithStartAndEnd(declaration) ? - getTextSpan(declaration.start, sourceFile, declaration.end) : - getTextSpan(declaration, sourceFile); - if (declarationSpan.start !== span.textSpan.start || declarationSpan.length !== span.textSpan.length) { - span.declarationSpan = declarationSpan; - } - } + export function toDeclarationSpan(textSpan: TextSpan, sourceFile: SourceFile, declaration?: DeclarationNode): { declarationSpan: TextSpan } | undefined { + if (!declaration) return undefined; + const declarationSpan = isDeclarationNodeWithStartAndEnd(declaration) ? + getTextSpan(declaration.start, sourceFile, declaration.end) : + getTextSpan(declaration, sourceFile); + return declarationSpan.start !== textSpan.start || declarationSpan.length !== textSpan.length ? + { declarationSpan } : + undefined; } export interface Options { @@ -304,17 +303,17 @@ namespace ts.FindAllReferences { const { node, name, kind, displayParts, declaration } = info; const sourceFile = node.getSourceFile(); - const result: ReferencedSymbolDefinitionInfo = { + const textSpan = getTextSpan(isComputedPropertyName(node) ? node.expression : node, sourceFile); + return { containerKind: ScriptElementKind.unknown, containerName: "", fileName: sourceFile.fileName, kind, name, - textSpan: getTextSpan(isComputedPropertyName(node) ? node.expression : node, sourceFile), - displayParts + textSpan, + displayParts, + ...toDeclarationSpan(textSpan, sourceFile, declaration) }; - setDeclarationSpan(result, sourceFile, declaration); - return result; } function getDefinitionKindAndDisplayParts(symbol: Symbol, checker: TypeChecker, node: Node): { displayParts: SymbolDisplayPart[], kind: ScriptElementKind } { @@ -351,9 +350,12 @@ namespace ts.FindAllReferences { } else { const sourceFile = entry.node.getSourceFile(); - const result: DocumentSpan = { textSpan: getTextSpan(entry.node, sourceFile), fileName: sourceFile.fileName }; - setDeclarationSpan(result, sourceFile, entry.declaration); - return result; + const textSpan = getTextSpan(entry.node, sourceFile); + return { + textSpan, + fileName: sourceFile.fileName, + ...toDeclarationSpan(textSpan, sourceFile, entry.declaration) + }; } } @@ -438,10 +440,8 @@ namespace ts.FindAllReferences { textSpan: documentSpan.textSpan, kind: writeAccess ? HighlightSpanKind.writtenReference : HighlightSpanKind.reference, isInString: entry.kind === EntryKind.StringLiteral ? true : undefined, + ...documentSpan.declarationSpan && { declarationSpan: documentSpan.declarationSpan } }; - if (documentSpan.declarationSpan) { - span.declarationSpan = documentSpan.declarationSpan; - } return { fileName: documentSpan.fileName, span }; } diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 4970ae573bf..efa220faf9a 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -273,20 +273,20 @@ namespace ts.GoToDefinition { function createDefinitionInfoFromName(declaration: Declaration, symbolKind: ScriptElementKind, symbolName: string, containerName: string): DefinitionInfo { const name = getNameOfDeclaration(declaration) || declaration; const sourceFile = name.getSourceFile(); - const result: DefinitionInfo = { + const textSpan = createTextSpanFromNode(name, sourceFile); + return { fileName: sourceFile.fileName, - textSpan: createTextSpanFromNode(name, sourceFile), + textSpan, kind: symbolKind, name: symbolName, containerKind: undefined!, // TODO: GH#18217 containerName, + ...FindAllReferences.toDeclarationSpan( + textSpan, + sourceFile, + FindAllReferences.getDeclarationForDeclarationSpan(declaration) + ) }; - FindAllReferences.setDeclarationSpan( - result, - sourceFile, - FindAllReferences.getDeclarationForDeclarationSpan(declaration) - ); - return result; } function createDefinitionFromSignatureDeclaration(typeChecker: TypeChecker, decl: SignatureDeclaration): DefinitionInfo { diff --git a/src/services/services.ts b/src/services/services.ts index 032dee4a40c..931f9cce79d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1546,19 +1546,14 @@ namespace ts { function getOccurrencesAtPosition(fileName: string, position: number): ReadonlyArray | undefined { return flatMap( getDocumentHighlights(fileName, position, [fileName]), - entry => entry.highlightSpans.map(highlightSpan => { - const result: ReferenceEntry = { - fileName: entry.fileName, - textSpan: highlightSpan.textSpan, - isWriteAccess: highlightSpan.kind === HighlightSpanKind.writtenReference, - isDefinition: false, - isInString: highlightSpan.isInString, - }; - if (highlightSpan.declarationSpan) { - result.declarationSpan = highlightSpan.declarationSpan; - } - return result; - }) + entry => entry.highlightSpans.map(highlightSpan => ({ + fileName: entry.fileName, + textSpan: highlightSpan.textSpan, + isWriteAccess: highlightSpan.kind === HighlightSpanKind.writtenReference, + isDefinition: false, + ...highlightSpan.isInString && { isInString: true }, + ...highlightSpan.declarationSpan && { declarationSpan: highlightSpan.declarationSpan } + })) ); } @@ -1577,13 +1572,13 @@ namespace ts { const node = getTouchingPropertyName(sourceFile, position); if (isIdentifier(node) && (isJsxOpeningElement(node.parent) || isJsxClosingElement(node.parent)) && isIntrinsicJsxName(node.escapedText)) { const { openingElement, closingElement } = node.parent.parent; - return [openingElement, closingElement].map(node => { - const result: RenameLocation = { + return [openingElement, closingElement].map((node): RenameLocation => { + const textSpan = createTextSpanFromNode(node.tagName, sourceFile); + return { fileName: sourceFile.fileName, - textSpan: createTextSpanFromNode(node.tagName, sourceFile) + textSpan, + ...FindAllReferences.toDeclarationSpan(textSpan, sourceFile, node.parent) }; - FindAllReferences.setDeclarationSpan(result, sourceFile, node.parent); - return result; }); } else { diff --git a/src/testRunner/unittests/tsserver/declarationFileMaps.ts b/src/testRunner/unittests/tsserver/declarationFileMaps.ts index 1ab0928630c..f87192f527d 100644 --- a/src/testRunner/unittests/tsserver/declarationFileMaps.ts +++ b/src/testRunner/unittests/tsserver/declarationFileMaps.ts @@ -7,12 +7,12 @@ namespace ts.projectSystem { declarationOptions?: SpanFromSubstringOptions; } function documentSpanFromSubstring({ file, text, declarationText, options, declarationOptions }: DocumentSpanFromSubstring): DocumentSpan { - const result: DocumentSpan = { fileName: file.path, textSpan: textSpanFromSubstring(file.content, text, options) }; - if (declarationText) { - const declarationSpan = documentSpanFromSubstring({ file, text: declarationText, options: declarationOptions }); - result.declarationSpan = declarationSpan.textSpan; - } - return result; + const declarationSpan = declarationText !== undefined ? documentSpanFromSubstring({ file, text: declarationText, options: declarationOptions }) : undefined; + return { + fileName: file.path, + textSpan: textSpanFromSubstring(file.content, text, options), + ...declarationSpan && { declarationSpan: declarationSpan.textSpan } + }; } function renameLocation(input: DocumentSpanFromSubstring): RenameLocation { diff --git a/src/testRunner/unittests/tsserver/helpers.ts b/src/testRunner/unittests/tsserver/helpers.ts index e9c06a7e859..332e6f8bdc6 100644 --- a/src/testRunner/unittests/tsserver/helpers.ts +++ b/src/testRunner/unittests/tsserver/helpers.ts @@ -523,13 +523,16 @@ namespace ts.projectSystem { } export function protocolDeclarationFileSpanFromSubstring({ declarationText, declarationOptions, ...rest }: DeclarationFileSpanFromSubString): protocol.DeclarationFileSpan { const result = protocolFileSpanFromSubstring(rest); - if (!declarationText) return result; - const declarationSpan = protocolFileSpanFromSubstring({ file: rest.file, text: declarationText, options: declarationOptions }); - return { - ...result, - declarationStart: declarationSpan.start, - declarationEnd: declarationSpan.end - }; + const declarationSpan = declarationText !== undefined ? + protocolFileSpanFromSubstring({ file: rest.file, text: declarationText, options: declarationOptions }) : + undefined; + return declarationSpan ? + { + ...result, + declarationStart: declarationSpan.start, + declarationEnd: declarationSpan.end + } : + result; } export interface ProtocolDeclarationTextSpanFromString { @@ -542,16 +545,15 @@ namespace ts.projectSystem { export function protocolDeclarationTextSpanFromSubstring({ fileText, text, options, declarationText, declarationOptions }: ProtocolDeclarationTextSpanFromString): protocol.DeclarationTextSpan { const span = textSpanFromSubstring(fileText, text, options); const toLocation = protocolToLocation(fileText); - const result: protocol.DeclarationTextSpan = { + const declarationSpan = declarationText !== undefined ? textSpanFromSubstring(fileText, declarationText, declarationOptions) : undefined; + return { start: toLocation(span.start), - end: toLocation(textSpanEnd(span)) + end: toLocation(textSpanEnd(span)), + ...declarationSpan && { + declarationStart: toLocation(declarationSpan.start), + declarationEnd: toLocation(textSpanEnd(declarationSpan)) + } }; - if (declarationText) { - const span = textSpanFromSubstring(fileText, declarationText, declarationOptions); - result.declarationStart = toLocation(span.start); - result.declarationEnd = toLocation(textSpanEnd(span)); - } - return result; } export interface ProtocolRenameSpanFromSubstring extends ProtocolDeclarationTextSpanFromString {