From 16109dfd00c7eb1a649f111db0f36833170128ab Mon Sep 17 00:00:00 2001 From: BigAru Date: Sun, 9 Dec 2018 00:04:50 +0100 Subject: [PATCH] optimize templateLiteral check --- .../convertStringOrTemplateLiteral.ts | 33 +++++++------------ ...gOrTemplateLiteral_ToStringAvailability.ts | 2 +- ...ngOrTemplateLiteral_ToStringNestedInner.ts | 14 ++++++++ ...mplateLiteral_ToStringNestedInnerNonSub.ts | 14 ++++++++ ...gOrTemplateLiteral_ToStringNestedOuter.ts} | 2 +- 5 files changed, 42 insertions(+), 23 deletions(-) create mode 100644 tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInner.ts create mode 100644 tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInnerNonSub.ts rename tests/cases/fourslash/{refactorConvertStringOrTemplateLiteral_ToStringNested.ts => refactorConvertStringOrTemplateLiteral_ToStringNestedOuter.ts} (86%) diff --git a/src/services/refactors/convertStringOrTemplateLiteral.ts b/src/services/refactors/convertStringOrTemplateLiteral.ts index 6b0fd4f7dfa..c63a4f48f1e 100644 --- a/src/services/refactors/convertStringOrTemplateLiteral.ts +++ b/src/services/refactors/convertStringOrTemplateLiteral.ts @@ -12,7 +12,7 @@ namespace ts.refactor.convertStringOrTemplateLiteral { function getAvailableActions(context: RefactorContext): ReadonlyArray { const { file, startPosition } = context; - const node = getNodeOrParentOfBraces(file, startPosition); + const node = getNodeOrParentOfParentheses(file, startPosition); const maybeBinary = getParentBinaryExpression(node); const actions: RefactorActionInfo[] = []; @@ -20,33 +20,24 @@ namespace ts.refactor.convertStringOrTemplateLiteral { actions.push({ name: toTemplateLiteralActionName, description: toTemplateLiteralDescription }); } - if (isTemplateLike(node)) { + const templateLiteral = findAncestor(node, n => isTemplateLiteral(n)); + + if (templateLiteral && !isTaggedTemplateExpression(templateLiteral.parent)) { actions.push({ name: toStringConcatenationActionName, description: toStringConcatenationDescription }); } return [{ name: refactorName, description: refactorDescription, actions }]; } - function getNodeOrParentOfBraces(file: SourceFile, startPosition: number) { + function getNodeOrParentOfParentheses(file: SourceFile, startPosition: number) { const node = getTokenAtPosition(file, startPosition); if (isParenthesizedExpression(node.parent) && isBinaryExpression(node.parent.parent)) return node.parent.parent; return node; } - function isTemplateLike(node: Node) { - const isTemplateWithoutSubstitution = isNoSubstitutionTemplateLiteral(node) && isNotTagged(node); - const isTemplate = (isTemplateHead(node) || isTemplateMiddleOrTemplateTail(node)) && isNotTagged(node.parent); - const isTemplateFromExpression = isTemplateSpan(node.parent) && isNotTagged(node.parent.parent); - return isTemplateWithoutSubstitution || isTemplate || isTemplateFromExpression; - } - - function isNotTagged(templateExpression: Node) { - return !isTaggedTemplateExpression(templateExpression.parent); - } - function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { const { file, startPosition } = context; - const node = getNodeOrParentOfBraces(file, startPosition); + const node = getNodeOrParentOfParentheses(file, startPosition); switch (actionName) { case toTemplateLiteralActionName: @@ -65,20 +56,20 @@ namespace ts.refactor.convertStringOrTemplateLiteral { } function getEditsForToStringConcatenation(context: RefactorContext, node: Node) { - if (isTemplateExpression(node.parent) || isTemplateSpan(node.parent)) { - const templateLiteralExpression = isTemplateSpan(node.parent) ? node.parent.parent : node.parent; - const { head, templateSpans } = templateLiteralExpression; + const templateLiteral = findAncestor(node, n => isTemplateLiteral(n))! as TemplateLiteral; + + if (isTemplateExpression(templateLiteral)) { + const { head, templateSpans } = templateLiteral; const arrayOfNodes = templateSpans.map(templateSpanToExpressions) .reduce((accumulator, nextArray) => accumulator.concat(nextArray)); if (head.text.length !== 0) arrayOfNodes.unshift(createStringLiteral(head.text)); const binaryExpression = arrayToTree(arrayOfNodes); - return textChanges.ChangeTracker.with(context, t => t.replaceNode(context.file, templateLiteralExpression, binaryExpression)); + return textChanges.ChangeTracker.with(context, t => t.replaceNode(context.file, templateLiteral, binaryExpression)); } else { - const templateWithoutSubstitution = node as NoSubstitutionTemplateLiteral; - const stringLiteral = createStringLiteral(templateWithoutSubstitution.text); + const stringLiteral = createStringLiteral(templateLiteral.text); return textChanges.ChangeTracker.with(context, t => t.replaceNode(context.file, node, stringLiteral)); } } diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringAvailability.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringAvailability.ts index 2725cd75aaf..03e0882bb2f 100644 --- a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringAvailability.ts +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringAvailability.ts @@ -25,5 +25,5 @@ verify.refactorAvailable("Convert string concatenation or template literal", "Co verify.not.refactorAvailable("Convert string concatenation or template literal", "Convert to template literal"); goTo.select("p", "o"); -verify.not.refactorAvailable("Convert string concatenation or template literal", "Convert to string concatenation"); +verify.refactorAvailable("Convert string concatenation or template literal", "Convert to string concatenation"); verify.not.refactorAvailable("Convert string concatenation or template literal", "Convert to template literal"); diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInner.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInner.ts new file mode 100644 index 00000000000..a094d5847c4 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInner.ts @@ -0,0 +1,14 @@ +/// + +//// const age = 42 +//// const foo = `foobar is a ${ age < 18 ? 'child' : /*x*/`/*y*/grown-up ${ age > 40 ? 'who needs probaply assistance' : ''}` }` + +goTo.select("x", "y"); +edit.applyRefactor({ + refactorName: "Convert string concatenation or template literal", + actionName: "Convert to string concatenation", + actionDescription: "Convert to string concatenation", + newContent: +`const age = 42 +const foo = \`foobar is a \${ age < 18 ? 'child' : "grown-up " + (age > 40 ? 'who needs probaply assistance' : '') }\``, +}); diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInnerNonSub.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInnerNonSub.ts new file mode 100644 index 00000000000..83783e4d658 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedInnerNonSub.ts @@ -0,0 +1,14 @@ +/// + +//// const age = 42 +//// const foo = `foobar is a ${ `/*x*/3/*y*/4` }` + +goTo.select("x", "y"); +edit.applyRefactor({ + refactorName: "Convert string concatenation or template literal", + actionName: "Convert to string concatenation", + actionDescription: "Convert to string concatenation", + newContent: +`const age = 42 +const foo = \`foobar is a \${ "34" }\``, +}); diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNested.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedOuter.ts similarity index 86% rename from tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNested.ts rename to tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedOuter.ts index f579508df23..fb09d7876b9 100644 --- a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNested.ts +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringNestedOuter.ts @@ -1,7 +1,7 @@ /// //// const age = 42 -//// const foo = `/*x*/f/*y*/oobar is a ${ age < 18 ? 'child' : `grown-up ${ age > 40 ? 'who needs probaply assistance': ''}` }` +//// const foo = `foobar is a ${ /*x*/a/*y*/ge < 18 ? 'child' : `grown-up ${ age > 40 ? 'who needs probaply assistance': ''}` }` goTo.select("x", "y"); edit.applyRefactor({