-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix #7611: Add support for String Literal Types in find all refs and occurances #8366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4891,8 +4891,8 @@ namespace ts { | |
node.kind === SyntaxKind.ThisKeyword || | ||
node.kind === SyntaxKind.ThisType || | ||
node.kind === SyntaxKind.SuperKeyword || | ||
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || | ||
isNameOfExternalModuleImportOrDeclaration(node)) { | ||
node.kind === SyntaxKind.StringLiteral || | ||
isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) { | ||
|
||
const referencedSymbols = getReferencedSymbolsForNode(node, sourceFilesToSearch, /*findInStrings*/ false, /*findInComments*/ false); | ||
return convertReferencedSymbols(referencedSymbols); | ||
|
@@ -5559,8 +5559,8 @@ namespace ts { | |
// TODO (drosen): This should be enabled in a later release - currently breaks rename. | ||
// node.kind !== SyntaxKind.ThisKeyword && | ||
// node.kind !== SyntaxKind.SuperKeyword && | ||
!isLiteralNameOfPropertyDeclarationOrIndexAccess(node) && | ||
!isNameOfExternalModuleImportOrDeclaration(node)) { | ||
node.kind !== SyntaxKind.StringLiteral && | ||
!isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) { | ||
return undefined; | ||
} | ||
|
||
|
@@ -5595,6 +5595,10 @@ namespace ts { | |
|
||
const symbol = typeChecker.getSymbolAtLocation(node); | ||
|
||
if (!symbol && node.kind === SyntaxKind.StringLiteral) { | ||
return getReferencesForStringLiteral(<StringLiteral>node, sourceFiles); | ||
} | ||
|
||
// Could not find a symbol e.g. unknown identifier | ||
if (!symbol) { | ||
// Can't have references to something that we have no symbol for. | ||
|
@@ -6151,6 +6155,52 @@ namespace ts { | |
} | ||
} | ||
|
||
|
||
function getReferencesForStringLiteral(node: StringLiteral, sourceFiles: SourceFile[]): ReferencedSymbol[] { | ||
const typeChecker = program.getTypeChecker(); | ||
const type = getStringLiteralTypeForNode(node, typeChecker); | ||
|
||
if (!type) { | ||
// nothing to do here. moving on | ||
return undefined; | ||
} | ||
|
||
const references: ReferenceEntry[] = []; | ||
|
||
for (const sourceFile of sourceFiles) { | ||
const possiblePositions = getPossibleSymbolReferencePositions(sourceFile, type.text, sourceFile.getStart(), sourceFile.getEnd()); | ||
getReferencesForStringLiteralInFile(sourceFile, type, possiblePositions, references); | ||
} | ||
|
||
return [{ | ||
definition: { | ||
containerKind: "", | ||
containerName: "", | ||
fileName: node.getSourceFile().fileName, | ||
kind: ScriptElementKind.variableElement, | ||
name: type.text, | ||
textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pass sourceFile to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. more or less the same impact, i do not have the source file handy anyways. |
||
}, | ||
references: references | ||
}]; | ||
|
||
function getReferencesForStringLiteralInFile(sourceFile: SourceFile, searchType: Type, possiblePositions: number[], references: ReferenceEntry[]): void { | ||
for (const position of possiblePositions) { | ||
cancellationToken.throwIfCancellationRequested(); | ||
|
||
const node = getTouchingWord(sourceFile, position); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this won't work within the blank space in a string literal, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no it should. the literal is one token. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, it's not as granular as characters, just tokens |
||
if (!node || node.kind !== SyntaxKind.StringLiteral) { | ||
return; | ||
} | ||
|
||
const type = getStringLiteralTypeForNode(<StringLiteral>node, typeChecker); | ||
if (type === searchType) { | ||
references.push(getReferenceEntryFromNode(node)); | ||
} | ||
} | ||
} | ||
} | ||
|
||
function populateSearchSymbolSet(symbol: Symbol, location: Node): Symbol[] { | ||
// The search set contains at least the current symbol | ||
let result = [symbol]; | ||
|
@@ -7671,53 +7721,75 @@ namespace ts { | |
} | ||
} | ||
|
||
function getStringLiteralTypeForNode(node: StringLiteral | StringLiteralTypeNode, typeChecker: TypeChecker): StringLiteralType { | ||
const searchNode = node.parent.kind === SyntaxKind.StringLiteralType ? <StringLiteralTypeNode>node.parent : node; | ||
const type = typeChecker.getTypeAtLocation(searchNode); | ||
if (type && type.flags & TypeFlags.StringLiteral) { | ||
return <StringLiteralType>type; | ||
} | ||
return undefined; | ||
} | ||
|
||
function getRenameInfo(fileName: string, position: number): RenameInfo { | ||
synchronizeHostData(); | ||
|
||
const sourceFile = getValidSourceFile(fileName); | ||
const typeChecker = program.getTypeChecker(); | ||
|
||
const defaultLibFileName = host.getDefaultLibFileName(host.getCompilationSettings()); | ||
const canonicalDefaultLibName = getCanonicalFileName(ts.normalizePath(defaultLibFileName)); | ||
|
||
const node = getTouchingWord(sourceFile, position); | ||
|
||
// Can only rename an identifier. | ||
if (node && node.kind === SyntaxKind.Identifier) { | ||
const symbol = typeChecker.getSymbolAtLocation(node); | ||
|
||
// Only allow a symbol to be renamed if it actually has at least one declaration. | ||
if (symbol) { | ||
const declarations = symbol.getDeclarations(); | ||
if (declarations && declarations.length > 0) { | ||
// Disallow rename for elements that are defined in the standard TypeScript library. | ||
const defaultLibFileName = host.getDefaultLibFileName(host.getCompilationSettings()); | ||
const canonicalDefaultLibName = getCanonicalFileName(ts.normalizePath(defaultLibFileName)); | ||
if (defaultLibFileName) { | ||
for (const current of declarations) { | ||
const sourceFile = current.getSourceFile(); | ||
// TODO (drosen): When is there no source file? | ||
if (!sourceFile) { | ||
continue; | ||
} | ||
if (node) { | ||
if (node.kind === SyntaxKind.Identifier || | ||
node.kind === SyntaxKind.StringLiteral || | ||
isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) { | ||
const symbol = typeChecker.getSymbolAtLocation(node); | ||
|
||
// Only allow a symbol to be renamed if it actually has at least one declaration. | ||
if (symbol) { | ||
const declarations = symbol.getDeclarations(); | ||
if (declarations && declarations.length > 0) { | ||
// Disallow rename for elements that are defined in the standard TypeScript library. | ||
if (forEach(declarations, isDefinedInLibraryFile)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should just do a pass at the end for all the references you get, instead of doing this logic here and below |
||
return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library)); | ||
} | ||
|
||
const canonicalName = getCanonicalFileName(ts.normalizePath(sourceFile.fileName)); | ||
if (canonicalName === canonicalDefaultLibName) { | ||
return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library)); | ||
} | ||
const displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node)); | ||
const kind = getSymbolKind(symbol, node); | ||
if (kind) { | ||
return { | ||
canRename: true, | ||
kind, | ||
displayName, | ||
localizedErrorMessage: undefined, | ||
fullDisplayName: typeChecker.getFullyQualifiedName(symbol), | ||
kindModifiers: getSymbolModifiers(symbol), | ||
triggerSpan: createTriggerSpanForNode(node, sourceFile) | ||
}; | ||
} | ||
} | ||
|
||
const displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node)); | ||
const kind = getSymbolKind(symbol, node); | ||
if (kind) { | ||
return { | ||
canRename: true, | ||
kind, | ||
displayName, | ||
localizedErrorMessage: undefined, | ||
fullDisplayName: typeChecker.getFullyQualifiedName(symbol), | ||
kindModifiers: getSymbolModifiers(symbol), | ||
triggerSpan: createTextSpan(node.getStart(), node.getWidth()) | ||
}; | ||
} | ||
else if (node.kind === SyntaxKind.StringLiteral) { | ||
const type = getStringLiteralTypeForNode(<StringLiteral>node, typeChecker); | ||
if (type) { | ||
if (isDefinedInLibraryFile(node)) { | ||
return getRenameInfoError(getLocaleSpecificMessage(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library)); | ||
} | ||
else { | ||
const displayName = stripQuotes(type.text); | ||
return { | ||
canRename: true, | ||
kind: ScriptElementKind.variableElement, | ||
displayName, | ||
localizedErrorMessage: undefined, | ||
fullDisplayName: displayName, | ||
kindModifiers: ScriptElementKindModifier.none, | ||
triggerSpan: createTriggerSpanForNode(node, sourceFile) | ||
}; | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -7736,6 +7808,28 @@ namespace ts { | |
triggerSpan: undefined | ||
}; | ||
} | ||
|
||
function isDefinedInLibraryFile(declaration: Node) { | ||
if (defaultLibFileName) { | ||
const sourceFile = declaration.getSourceFile(); | ||
const canonicalName = getCanonicalFileName(ts.normalizePath(sourceFile.fileName)); | ||
if (canonicalName === canonicalDefaultLibName) { | ||
return true; | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
function createTriggerSpanForNode(node: Node, sourceFile: SourceFile) { | ||
let start = node.getStart(sourceFile); | ||
let width = node.getWidth(sourceFile); | ||
if (node.kind === SyntaxKind.StringLiteral) { | ||
// Exclude the quotes | ||
start += 1; | ||
width -= 2; | ||
} | ||
return createTextSpan(start, width); | ||
} | ||
} | ||
|
||
return { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/// <reference path='fourslash.ts'/> | ||
|
||
////type Options = "[|option 1|]" | "option 2"; | ||
////let myOption: Options = "[|option 1|]"; | ||
|
||
let ranges = test.ranges(); | ||
for (let range of ranges) { | ||
goTo.position(range.start); | ||
|
||
verify.referencesCountIs(ranges.length); | ||
for (let expectedReference of ranges) { | ||
verify.referencesAtPositionContains(expectedReference); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
////function foo(a: "[|option 1|]") { } | ||
////foo("[|option 1|]"); | ||
|
||
const ranges = test.ranges(); | ||
for (let r of ranges) { | ||
goTo.position(r.start); | ||
|
||
for (let range of ranges) { | ||
verify.occurrencesAtPositionContains(range, false); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
////var x = "[|string|]"; | ||
////function f(a = "[|initial value|]") { } | ||
|
||
const ranges = test.ranges(); | ||
for (let r of ranges) { | ||
goTo.position(r.start); | ||
verify.occurrencesAtPositionCount(0); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
////var y: "string" = "string; | ||
////var x = "/*1*/string"; | ||
////function f(a = "/*2*/initial value") { } | ||
|
||
|
||
goTo.marker("1"); | ||
verify.renameInfoFailed(); | ||
|
||
goTo.marker("2"); | ||
verify.renameInfoFailed(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
////var o = { | ||
//// [|prop|]: 0 | ||
////}; | ||
//// | ||
////o = { | ||
//// "[|prop|]": 1 | ||
////}; | ||
//// | ||
////o["[|prop|]"]; | ||
////o['[|prop|]']; | ||
////o.[|prop|]; | ||
|
||
|
||
let ranges = test.ranges(); | ||
for (let range of ranges) { | ||
goTo.position(range.start); | ||
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
|
||
////interface AnimationOptions { | ||
//// deltaX: number; | ||
//// deltaY: number; | ||
//// easing: "ease-in" | "ease-out" | "[|ease-in-out|]"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this work if you tried renaming adjacent to the dashed portions? Would it work if you had spaces around? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, the whole string literal is one token we do not break it apart and scan inside it. |
||
////} | ||
//// | ||
////function animate(o: AnimationOptions) { } | ||
//// | ||
////animate({ deltaX: 100, deltaY: 100, easing: "[|ease-in-out|]" }); | ||
|
||
let ranges = test.ranges(); | ||
for (let range of ranges) { | ||
goTo.position(range.start); | ||
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variableElement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally a new element "stringElement", but will need some handling on the VS side, so can get that done first.