From e02ef9fddbc90dd74a9a527d89789f66818e33cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maria=20Jos=C3=A9=20Solano?= Date: Thu, 20 Apr 2023 16:58:29 -0700 Subject: [PATCH] Add quotes when renaming numerical indices (#53596) --- src/harness/client.ts | 8 +++++--- src/harness/fourslashImpl.ts | 12 +++++++++--- src/harness/fourslashInterfaceImpl.ts | 1 + src/harness/harnessLanguageService.ts | 4 ++-- src/server/session.ts | 5 ++--- src/services/findAllReferences.ts | 15 ++++++++++++--- src/services/services.ts | 7 +++++-- src/services/shims.ts | 8 ++++---- src/services/types.ts | 2 ++ .../baselines/reference/api/tsserverlibrary.d.ts | 2 ++ tests/baselines/reference/api/typescript.d.ts | 2 ++ .../reference/renameNumericalIndex.baseline.jsonc | 11 +++++++++++ ...enameNumericalIndexSingleQuoted.baseline.jsonc | 15 +++++++++++++++ tests/cases/fourslash/fourslash.ts | 2 +- tests/cases/fourslash/renameNumericalIndex.ts | 6 ++++++ .../fourslash/renameNumericalIndexSingleQuoted.ts | 6 ++++++ 16 files changed, 85 insertions(+), 21 deletions(-) create mode 100644 tests/baselines/reference/renameNumericalIndex.baseline.jsonc create mode 100644 tests/baselines/reference/renameNumericalIndexSingleQuoted.baseline.jsonc create mode 100644 tests/cases/fourslash/renameNumericalIndex.ts create mode 100644 tests/cases/fourslash/renameNumericalIndexSingleQuoted.ts diff --git a/src/harness/client.ts b/src/harness/client.ts index a74e6160723..04ef0003f0e 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -568,15 +568,17 @@ export class SessionClient implements LanguageService { return notImplemented(); } - findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): RenameLocation[] { + findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences: UserPreferences | boolean | undefined): RenameLocation[] { if (!this.lastRenameEntry || this.lastRenameEntry.inputs.fileName !== fileName || this.lastRenameEntry.inputs.position !== position || this.lastRenameEntry.inputs.findInStrings !== findInStrings || this.lastRenameEntry.inputs.findInComments !== findInComments) { - if (providePrefixAndSuffixTextForRename !== undefined) { + const providePrefixAndSuffixTextForRename = typeof preferences === "boolean" ? preferences : preferences?.providePrefixAndSuffixTextForRename; + const quotePreference = typeof preferences === "boolean" ? undefined : preferences?.quotePreference; + if (providePrefixAndSuffixTextForRename !== undefined || quotePreference !== undefined) { // User preferences have to be set through the `Configure` command - this.configure({ providePrefixAndSuffixTextForRename }); + this.configure({ providePrefixAndSuffixTextForRename, quotePreference }); // Options argument is not used, so don't pass in options this.getRenameInfo(fileName, position, /*preferences*/{}, findInStrings, findInComments); // Restore previous user preferences diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 85f8bd1118c..32c5d9b635e 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -1802,13 +1802,18 @@ export class TestState { isMarker(markerOrRange) ? markerOrRange : { fileName: markerOrRange.fileName, position: markerOrRange.pos }; - const { findInStrings = false, findInComments = false, providePrefixAndSuffixTextForRename = true } = options || {}; + const { + findInStrings = false, + findInComments = false, + providePrefixAndSuffixTextForRename = true, + quotePreference = "double" + } = options || {}; const locations = this.languageService.findRenameLocations( fileName, position, findInStrings, findInComments, - providePrefixAndSuffixTextForRename, + { providePrefixAndSuffixTextForRename, quotePreference }, ); if (!locations) { @@ -1818,7 +1823,8 @@ export class TestState { const renameOptions = options ? (options.findInStrings !== undefined ? `// @findInStrings: ${findInStrings}\n` : "") + (options.findInComments !== undefined ? `// @findInComments: ${findInComments}\n` : "") + - (options.providePrefixAndSuffixTextForRename !== undefined ? `// @providePrefixAndSuffixTextForRename: ${providePrefixAndSuffixTextForRename}\n` : "") : + (options.providePrefixAndSuffixTextForRename !== undefined ? `// @providePrefixAndSuffixTextForRename: ${providePrefixAndSuffixTextForRename}\n` : "") + + (options.quotePreference !== undefined ? `// @quotePreference: ${quotePreference}\n` : "") : ""; return renameOptions + (renameOptions ? "\n" : "") + this.getBaselineForDocumentSpansWithFileContents( diff --git a/src/harness/fourslashInterfaceImpl.ts b/src/harness/fourslashInterfaceImpl.ts index d7ad6357b83..89d00b7e318 100644 --- a/src/harness/fourslashInterfaceImpl.ts +++ b/src/harness/fourslashInterfaceImpl.ts @@ -1910,6 +1910,7 @@ export interface RenameOptions { readonly findInStrings?: boolean; readonly findInComments?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; + readonly quotePreference?: "auto" | "double" | "single"; } export type BaselineCommandWithMarkerOrRange = { type: "findAllReferences" | "goToDefinition" | "getDefinitionAtPosition" | "goToSourceDefinition" | "goToType" | "goToImplementation"; diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index d8b7e5eb9a4..e2cab0dc85e 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -527,8 +527,8 @@ class LanguageServiceShimProxy implements ts.LanguageService { getSmartSelectionRange(fileName: string, position: number): ts.SelectionRange { return unwrapJSONCallResult(this.shim.getSmartSelectionRange(fileName, position)); } - findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): ts.RenameLocation[] { - return unwrapJSONCallResult(this.shim.findRenameLocations(fileName, position, findInStrings, findInComments, providePrefixAndSuffixTextForRename)); + findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences?: ts.UserPreferences | boolean): ts.RenameLocation[] { + return unwrapJSONCallResult(this.shim.findRenameLocations(fileName, position, findInStrings, findInComments, preferences)); } getDefinitionAtPosition(fileName: string, position: number): ts.DefinitionInfo[] { return unwrapJSONCallResult(this.shim.getDefinitionAtPosition(fileName, position)); diff --git a/src/server/session.ts b/src/server/session.ts index 030a1941ccd..209f14c9ae3 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -136,7 +136,6 @@ import { toFileNameLowerCase, tracing, unmangleScopedPackageName, - UserPreferences, version, WithMetadata, } from "./_namespaces/ts"; @@ -497,14 +496,14 @@ function getRenameLocationsWorker( initialLocation: DocumentPosition, findInStrings: boolean, findInComments: boolean, - { providePrefixAndSuffixTextForRename }: UserPreferences + preferences: protocol.UserPreferences ): readonly RenameLocation[] { const perProjectResults = getPerProjectReferences( projects, defaultProject, initialLocation, /*isForRename*/ true, - (project, position) => project.getLanguageService().findRenameLocations(position.fileName, position.pos, findInStrings, findInComments, providePrefixAndSuffixTextForRename), + (project, position) => project.getLanguageService().findRenameLocations(position.fileName, position.pos, findInStrings, findInComments, preferences), (renameLocation, cb) => cb(documentSpanLocation(renameLocation)), ); diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 108f2191ef5..5f3eb986565 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -72,6 +72,7 @@ import { getNodeKind, getPropertySymbolFromBindingElement, getPropertySymbolsFromContextualType, + getQuoteFromPreference, getReferencedFileLocation, getSuperContainer, getSymbolId, @@ -151,6 +152,7 @@ import { isNamespaceExportDeclaration, isNewExpressionTarget, isNoSubstitutionTemplateLiteral, + isNumericLiteral, isObjectBindingElementWithoutPropertyName, isObjectLiteralExpression, isObjectLiteralMethod, @@ -205,6 +207,7 @@ import { PropertyAssignment, PropertyDeclaration, punctuationPart, + QuotePreference, rangeIsOnSingleLine, ReferencedSymbol, ReferencedSymbolDefinitionInfo, @@ -673,8 +676,8 @@ function getDefinitionKindAndDisplayParts(symbol: Symbol, checker: TypeChecker, } /** @internal */ -export function toRenameLocation(entry: Entry, originalNode: Node, checker: TypeChecker, providePrefixAndSuffixText: boolean): RenameLocation { - return { ...entryToDocumentSpan(entry), ...(providePrefixAndSuffixText && getPrefixAndSuffixText(entry, originalNode, checker)) }; +export function toRenameLocation(entry: Entry, originalNode: Node, checker: TypeChecker, providePrefixAndSuffixText: boolean, quotePreference: QuotePreference): RenameLocation { + return { ...entryToDocumentSpan(entry), ...(providePrefixAndSuffixText && getPrefixAndSuffixText(entry, originalNode, checker, quotePreference)) }; } function toReferencedSymbolEntry(entry: Entry, symbol: Symbol | undefined): ReferencedSymbolEntry { @@ -716,7 +719,7 @@ function entryToDocumentSpan(entry: Entry): DocumentSpan { } interface PrefixAndSuffix { readonly prefixText?: string; readonly suffixText?: string; } -function getPrefixAndSuffixText(entry: Entry, originalNode: Node, checker: TypeChecker): PrefixAndSuffix { +function getPrefixAndSuffixText(entry: Entry, originalNode: Node, checker: TypeChecker, quotePreference: QuotePreference): PrefixAndSuffix { if (entry.kind !== EntryKind.Span && isIdentifier(originalNode)) { const { node, kind } = entry; const parent = node.parent; @@ -760,6 +763,12 @@ function getPrefixAndSuffixText(entry: Entry, originalNode: Node, checker: TypeC } } + // If the node is a numerical indexing literal, then add quotes around the property access. + if (entry.kind !== EntryKind.Span && isNumericLiteral(entry.node) && isAccessExpression(entry.node.parent)) { + const quote = getQuoteFromPreference(quotePreference); + return { prefixText: quote, suffixText: quote }; + } + return emptyOptions; } diff --git a/src/services/services.ts b/src/services/services.ts index 0dd67dac08f..c887061cdb7 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -105,6 +105,7 @@ import { getNonAssignedNameOfDeclaration, getNormalizedAbsolutePath, getObjectFlags, + getQuotePreference, getScriptKind, getSetExternalModuleIndicator, getSnapshotText, @@ -2128,7 +2129,7 @@ export function createLanguageService( return DocumentHighlights.getDocumentHighlights(program, cancellationToken, sourceFile, position, sourceFilesToSearch); } - function findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): RenameLocation[] | undefined { + function findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences?: UserPreferences | boolean): RenameLocation[] | undefined { synchronizeHostData(); const sourceFile = getValidSourceFile(fileName); const node = getAdjustedRenameLocation(getTouchingPropertyName(sourceFile, position)); @@ -2145,8 +2146,10 @@ export function createLanguageService( }); } else { + const quotePreference = getQuotePreference(sourceFile, preferences ?? emptyOptions); + const providePrefixAndSuffixTextForRename = typeof preferences === "boolean" ? preferences : preferences?.providePrefixAndSuffixTextForRename; return getReferencesWorker(node, position, { findInStrings, findInComments, providePrefixAndSuffixTextForRename, use: FindAllReferences.FindReferencesUse.Rename }, - (entry, originalNode, checker) => FindAllReferences.toRenameLocation(entry, originalNode, checker, providePrefixAndSuffixTextForRename || false)); + (entry, originalNode, checker) => FindAllReferences.toRenameLocation(entry, originalNode, checker, providePrefixAndSuffixTextForRename || false, quotePreference)); } } diff --git a/src/services/shims.ts b/src/services/shims.ts index 8adbc386641..1c3bdae55e9 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -256,7 +256,7 @@ export interface LanguageServiceShim extends Shim { * Returns a JSON-encoded value of the type: * { fileName: string, textSpan: { start: number, length: number } }[] */ - findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): string; + findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences?: UserPreferences | boolean): string; /** * Returns a JSON-encoded value of the type: @@ -952,10 +952,10 @@ class LanguageServiceShimObject extends ShimBase implements LanguageServiceShim ); } - public findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): string { + public findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences: UserPreferences): string { return this.forwardJSONCall( - `findRenameLocations('${fileName}', ${position}, ${findInStrings}, ${findInComments}, ${providePrefixAndSuffixTextForRename})`, - () => this.languageService.findRenameLocations(fileName, position, findInStrings, findInComments, providePrefixAndSuffixTextForRename) + `findRenameLocations('${fileName}', ${position}, ${findInStrings}, ${findInComments})`, + () => this.languageService.findRenameLocations(fileName, position, findInStrings, findInComments, preferences) ); } diff --git a/src/services/types.ts b/src/services/types.ts index a4d7b914742..e66340bd307 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -575,6 +575,8 @@ export interface LanguageService { /** @deprecated Use the signature with `UserPreferences` instead. */ getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo; + findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences: UserPreferences): readonly RenameLocation[] | undefined; + /** @deprecated Pass `providePrefixAndSuffixTextForRename` as part of a `UserPreferences` parameter. */ findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): readonly RenameLocation[] | undefined; getSmartSelectionRange(fileName: string, position: number): SelectionRange; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 90833b31015..79e0a080bc9 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -10095,6 +10095,8 @@ declare namespace ts { getRenameInfo(fileName: string, position: number, preferences: UserPreferences): RenameInfo; /** @deprecated Use the signature with `UserPreferences` instead. */ getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo; + findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences: UserPreferences): readonly RenameLocation[] | undefined; + /** @deprecated Pass `providePrefixAndSuffixTextForRename` as part of a `UserPreferences` parameter. */ findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): readonly RenameLocation[] | undefined; getSmartSelectionRange(fileName: string, position: number): SelectionRange; getDefinitionAtPosition(fileName: string, position: number): readonly DefinitionInfo[] | undefined; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 9615f8bbc1f..97efb8d175e 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -6153,6 +6153,8 @@ declare namespace ts { getRenameInfo(fileName: string, position: number, preferences: UserPreferences): RenameInfo; /** @deprecated Use the signature with `UserPreferences` instead. */ getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo; + findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, preferences: UserPreferences): readonly RenameLocation[] | undefined; + /** @deprecated Pass `providePrefixAndSuffixTextForRename` as part of a `UserPreferences` parameter. */ findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): readonly RenameLocation[] | undefined; getSmartSelectionRange(fileName: string, position: number): SelectionRange; getDefinitionAtPosition(fileName: string, position: number): readonly DefinitionInfo[] | undefined; diff --git a/tests/baselines/reference/renameNumericalIndex.baseline.jsonc b/tests/baselines/reference/renameNumericalIndex.baseline.jsonc new file mode 100644 index 00000000000..1596ec226f4 --- /dev/null +++ b/tests/baselines/reference/renameNumericalIndex.baseline.jsonc @@ -0,0 +1,11 @@ +// === findRenameLocations === +// === /tests/cases/fourslash/renameNumericalIndex.ts === +// const foo = { /*RENAME*/<|[|0RENAME|]: true|> }; +// foo[/*START PREFIX*/"[|0RENAME|]"/*END SUFFIX*/]; + + + +// === findRenameLocations === +// === /tests/cases/fourslash/renameNumericalIndex.ts === +// const foo = { <|[|0RENAME|]: true|> }; +// foo[/*START PREFIX*/"/*RENAME*/[|0RENAME|]"/*END SUFFIX*/]; \ No newline at end of file diff --git a/tests/baselines/reference/renameNumericalIndexSingleQuoted.baseline.jsonc b/tests/baselines/reference/renameNumericalIndexSingleQuoted.baseline.jsonc new file mode 100644 index 00000000000..1e602f9045b --- /dev/null +++ b/tests/baselines/reference/renameNumericalIndexSingleQuoted.baseline.jsonc @@ -0,0 +1,15 @@ +// === findRenameLocations === +// @quotePreference: single + +// === /tests/cases/fourslash/renameNumericalIndexSingleQuoted.ts === +// const foo = { /*RENAME*/<|[|0RENAME|]: true|> }; +// foo[/*START PREFIX*/'[|0RENAME|]'/*END SUFFIX*/]; + + + +// === findRenameLocations === +// @quotePreference: single + +// === /tests/cases/fourslash/renameNumericalIndexSingleQuoted.ts === +// const foo = { <|[|0RENAME|]: true|> }; +// foo[/*START PREFIX*/'/*RENAME*/[|0RENAME|]'/*END SUFFIX*/]; \ No newline at end of file diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 2bf83ba52e5..0f22a4b55eb 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -839,7 +839,7 @@ declare namespace FourSlashInterface { readonly providePrefixAndSuffixTextForRename?: boolean; }; - type RenameOptions = { readonly findInStrings?: boolean, readonly findInComments?: boolean, readonly providePrefixAndSuffixTextForRename?: boolean }; + type RenameOptions = { readonly findInStrings?: boolean, readonly findInComments?: boolean, readonly providePrefixAndSuffixTextForRename?: boolean, readonly quotePreference?: "auto" | "double" | "single" }; type RenameLocationOptions = Range | { readonly range: Range, readonly prefixText?: string, readonly suffixText?: string }; type DiagnosticIgnoredInterpolations = { template: string } type BaselineCommand = { diff --git a/tests/cases/fourslash/renameNumericalIndex.ts b/tests/cases/fourslash/renameNumericalIndex.ts new file mode 100644 index 00000000000..2e2fecb7bd9 --- /dev/null +++ b/tests/cases/fourslash/renameNumericalIndex.ts @@ -0,0 +1,6 @@ +/// + +////const foo = { [|0|]: true }; +////foo[[|0|]]; + +verify.baselineRenameAtRangesWithText("0"); \ No newline at end of file diff --git a/tests/cases/fourslash/renameNumericalIndexSingleQuoted.ts b/tests/cases/fourslash/renameNumericalIndexSingleQuoted.ts new file mode 100644 index 00000000000..3e2f81bed37 --- /dev/null +++ b/tests/cases/fourslash/renameNumericalIndexSingleQuoted.ts @@ -0,0 +1,6 @@ +/// + +////const foo = { [|0|]: true }; +////foo[[|0|]]; + +verify.baselineRenameAtRangesWithText("0", { quotePreference: "single" }); \ No newline at end of file