From dd89a49b1dddfa577600bed5c63510a9b8ac399e Mon Sep 17 00:00:00 2001 From: BigAru Date: Thu, 28 Mar 2019 15:05:22 +0100 Subject: [PATCH] copy comments from string to template literal --- .../convertStringOrTemplateLiteral.ts | 97 ++++++++++++++----- ...StringOrTemplateLiteral_ToStringComment.ts | 4 +- ...ringOrTemplateLiteral_ToTemplateComment.ts | 4 +- ...eLiteral_ToTemplateCommentAfterOperator.ts | 13 +++ ...TemplateLiteral_ToTemplateCommentNoExpr.ts | 13 +++ ...emplateLiteral_ToTemplateCommentParenth.ts | 13 +++ ...teLiteral_ToTemplateCommentTrailingExpr.ts | 13 +++ ...ateLiteral_ToTemplateCommentTrailingStr.ts | 13 +++ 8 files changed, 144 insertions(+), 26 deletions(-) create mode 100644 tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentAfterOperator.ts create mode 100644 tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentNoExpr.ts create mode 100644 tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentParenth.ts create mode 100644 tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingExpr.ts create mode 100644 tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingStr.ts diff --git a/src/services/refactors/convertStringOrTemplateLiteral.ts b/src/services/refactors/convertStringOrTemplateLiteral.ts index fb8be410684..3c6ef7179a9 100644 --- a/src/services/refactors/convertStringOrTemplateLiteral.ts +++ b/src/services/refactors/convertStringOrTemplateLiteral.ts @@ -64,9 +64,24 @@ namespace ts.refactor.convertStringOrTemplateLiteral { function getEditsForToTemplateLiteral(context: RefactorContext, node: Node) { const maybeBinary = getParentBinaryExpression(node); - const arrayOfNodes = transformTreeToArray(maybeBinary); - const templateLiteral = nodesToTemplate(arrayOfNodes); - return textChanges.ChangeTracker.with(context, t => t.replaceNode(context.file, maybeBinary, templateLiteral)); + const file = context.file; + + const templateLiteral = nodesToTemplate(treeToArray(maybeBinary), file); + const trailingCommentRanges = getTrailingCommentRanges(file.text, maybeBinary.end); + + if (trailingCommentRanges) { + const lastComment = trailingCommentRanges[trailingCommentRanges.length - 1]; + const trailingRange = { pos: trailingCommentRanges[0].pos, end: lastComment.end }; + + return textChanges.ChangeTracker.with(context, t => { + t.deleteRange(file, trailingRange); + t.replaceNode(file, maybeBinary, templateLiteral); + }); + } + else { + return textChanges.ChangeTracker.with(context, t => t.replaceNode(file, maybeBinary, templateLiteral)); + } + } const templateSpanToExpressions = (file: SourceFile) => (templateSpan: TemplateSpan): Expression[] => { @@ -132,59 +147,97 @@ namespace ts.refactor.convertStringOrTemplateLiteral { return containsString && areOperatorsValid; } - function transformTreeToArray(node: Node): ReadonlyArray { - return treeToArray(node).nodes; - } - - function treeToArray(node: Node): { nodes: ReadonlyArray, containsString: boolean, areOperatorsValid: boolean} { + function treeToArray(node: Node): { nodes: ReadonlyArray, operators: Token[], containsString: boolean, areOperatorsValid: boolean} { if (isBinaryExpression(node)) { - const { nodes: leftNodes, containsString: leftHasString, areOperatorsValid: leftOperatorValid } = treeToArray(node.left); + const { nodes: leftNodes, operators: leftOperator, containsString: leftHasString, areOperatorsValid: leftOperatorValid } = treeToArray(node.left); const { nodes: rightNodes, containsString: rightHasString, areOperatorsValid: rightOperatorValid } = treeToArray(node.right); if (!leftHasString && !rightHasString) { - return { nodes: [node], containsString: false, areOperatorsValid: true }; + return { nodes: [node], operators: [], containsString: false, areOperatorsValid: true }; } const currentOperatorValid = node.operatorToken.kind === SyntaxKind.PlusToken; const areOperatorsValid = leftOperatorValid && currentOperatorValid && rightOperatorValid; + leftOperator.push(node.operatorToken); - return { nodes: leftNodes.concat(rightNodes), containsString: true, areOperatorsValid }; + return { nodes: leftNodes.concat(rightNodes), operators: leftOperator, containsString: true, areOperatorsValid }; } - return { nodes: [node as Expression], containsString: isStringLiteral(node), areOperatorsValid: true }; + return { nodes: [node as Expression], operators: [], containsString: isStringLiteral(node), areOperatorsValid: true }; } - function concatConsecutiveString(index: number, nodes: ReadonlyArray): [number, string] { + const copyTrailingOperatorComments = (operators: Token[], file: SourceFile) => (index: number, targetNode: Node) => { + if (index < operators.length) { + copyTrailingComments(operators[index], targetNode, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ false); + } + }; + + const copyCommentFromMultiNode = (nodes: ReadonlyArray, file: SourceFile, copyOperatorComments: (index: number, targetNode: Node) => void) => + (indexes: number[], targetNode: Node) => { + while (indexes.length > 0) { + const index = indexes.shift()!; + copyTrailingComments(nodes[index], targetNode, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ false); + copyOperatorComments(index, targetNode); + } + }; + + function concatConsecutiveString(index: number, nodes: ReadonlyArray): [number, string, number[]] { let text = ""; + const indexes = []; while (index < nodes.length && isStringLiteral(nodes[index])) { text = text + decodeRawString(nodes[index].getText()); + indexes.push(index); index++; } text = escapeText(text); - return [index, text]; + return [index, text, indexes]; } - function nodesToTemplate(nodes: ReadonlyArray) { - const templateSpans: TemplateSpan[] = []; - const [begin, headText] = concatConsecutiveString(0, nodes); - const templateHead = createTemplateHead(headText); + function nodesToTemplate({nodes, operators}: {nodes: ReadonlyArray, operators: Token[]}, file: SourceFile) { + const copyOperatorComments = copyTrailingOperatorComments(operators, file); + const copyCommentFromStringLiterals = copyCommentFromMultiNode(nodes, file, copyOperatorComments); - if (begin === nodes.length) return createNoSubstitutionTemplateLiteral(headText); + const templateSpans: TemplateSpan[] = []; + const [begin, headText, headIndexes] = concatConsecutiveString(0, nodes); + + if (begin === nodes.length) { + const noSubstitutionTemplateLiteral = createNoSubstitutionTemplateLiteral(headText); + copyCommentFromStringLiterals(headIndexes, noSubstitutionTemplateLiteral); + return noSubstitutionTemplateLiteral; + } + + const templateHead = createTemplateHead(headText); + copyCommentFromStringLiterals(headIndexes, templateHead); for (let i = begin; i < nodes.length; i++) { - const expression = isParenthesizedExpression(nodes[i]) ? (nodes[i] as ParenthesizedExpression).expression : nodes[i]; - const [newIndex, subsequentText] = concatConsecutiveString(i + 1, nodes); + let currentNode = nodes[i]; + + if (isParenthesizedExpression(currentNode)) { + copyCommentsWhenParenthesized(currentNode); + currentNode = currentNode.expression; + } + + copyOperatorComments(i, currentNode); + const [newIndex, subsequentText, stringIndexes] = concatConsecutiveString(i + 1, nodes); i = newIndex - 1; const templatePart = i === nodes.length - 1 ? createTemplateTail(subsequentText) : createTemplateMiddle(subsequentText); - templateSpans.push(createTemplateSpan(expression, templatePart)); + copyCommentFromStringLiterals(stringIndexes, templatePart); + + templateSpans.push(createTemplateSpan(currentNode, templatePart)); } return createTemplateExpression(templateHead, templateSpans); } + function copyCommentsWhenParenthesized(node: ParenthesizedExpression) { + const file = node.getSourceFile(); + copyTrailingComments(node, node.expression, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ false); + copyTrailingAsLeadingComments(node.expression, node.expression, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ false); + } + const hexToUnicode = (_match: string, grp: string) => String.fromCharCode(parseInt(grp, 16)); const octalToUnicode = (_match: string, grp: string) => String.fromCharCode(parseInt(grp, 8)); diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringComment.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringComment.ts index 123236d948f..0ed6df9afb6 100644 --- a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringComment.ts +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToStringComment.ts @@ -1,6 +1,6 @@ /// -//// const foo = `/*x*/H/*y*/EAD ${ /* C0 */ 42 /* C1 */} Span1 ${ /* C2 */ 43 /* C3 */} Span2 ${ /* C4 */ 44 /* C5 */} Span3` +//// const foo = /* H */ `/*x*/H/*y*/EAD ${ /* C0 */ 42 /* C1 */} Span1 ${ /* C2 */ 43 /* C3 */} Span2 ${ /* C4 */ 44 /* C5 */} Span3` /* T */ goTo.select("x", "y"); edit.applyRefactor({ @@ -8,6 +8,6 @@ edit.applyRefactor({ actionName: "Convert to string concatenation", actionDescription: "Convert to string concatenation", newContent: -`const foo = "HEAD " + /* C0 */ 42 /* C1 */ + " Span1 " + /* C2 */ 43 /* C3 */ + " Span2 " + /* C4 */ 44 /* C5 */ + " Span3"`, +`const foo = /* H */ "HEAD " + /* C0 */ 42 /* C1 */ + " Span1 " + /* C2 */ 43 /* C3 */ + " Span2 " + /* C4 */ 44 /* C5 */ + " Span3" /* T */`, }); diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateComment.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateComment.ts index 617e95b86e7..4bf761cfcd5 100644 --- a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateComment.ts +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateComment.ts @@ -1,6 +1,6 @@ /// -//// const foo = /* C0 */ /*x*/"/*y*/foobar" /* C1 */ + " is" /* C2 */ + 42 /* C3 */ + "years old" /* C4 */ +//// const foo = /* C0 */ /*x*/"/*y*/foo" /* C1 */ + " is" /* C2 */ + 42 /* C3 */ + " and bar" /* C4 */ + " is" /* C5 */ + 52/* C6 */ goTo.select("x", "y"); edit.applyRefactor({ @@ -8,6 +8,6 @@ edit.applyRefactor({ actionName: "Convert to template literal", actionDescription: "Convert to template literal", newContent: -`const foo = /* C0 */ \`foobar is \${ /* C1 */ /* C2 */ 42 /* C3 */}years old\` /* C4 */`, +"const foo = /* C0 */ `foo is\${ /* C1 */ /* C2 */42 /* C3 */} and bar is\${ /* C4 */ /* C5 */52 /* C6 */}`", }); diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentAfterOperator.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentAfterOperator.ts new file mode 100644 index 00000000000..362a6728c41 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentAfterOperator.ts @@ -0,0 +1,13 @@ +/// + +//// const foo = /* C0 */ /*x*/"/*y*/foo" + /* C1 */ " is" + /* C2 */ 42 + /* C3 */ " and bar" + /* C4 */ " is" + /* C5 */ 52/* C6 */ + +goTo.select("x", "y"); +edit.applyRefactor({ + refactorName: "Convert string concatenation or template literal", + actionName: "Convert to template literal", + actionDescription: "Convert to template literal", + newContent: +"const foo = /* C0 */ `foo is\${ /* C1 */ /* C2 */42 /* C3 */} and bar is\${ /* C4 */ /* C5 */52 /* C6 */}`", +}); + diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentNoExpr.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentNoExpr.ts new file mode 100644 index 00000000000..75ea3a28e9c --- /dev/null +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentNoExpr.ts @@ -0,0 +1,13 @@ +/// + +//// const foo = /* C0 */ /*x*/"/*y*/foo" /* C1 */ + " is"/* C2 */ /* C3 */ + +goTo.select("x", "y"); +edit.applyRefactor({ + refactorName: "Convert string concatenation or template literal", + actionName: "Convert to template literal", + actionDescription: "Convert to template literal", + newContent: +"const foo = /* C0 */ `foo is` /* C1 */ /* C2 */ /* C3 */", +}); + diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentParenth.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentParenth.ts new file mode 100644 index 00000000000..b7b3bb2838b --- /dev/null +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentParenth.ts @@ -0,0 +1,13 @@ +/// + +//// const foo = /*x*/"/*y*/foobar is" + ( /* C1 */ 42 ) /* C2 */ + /* C3 */ " years old" + +goTo.select("x", "y"); +edit.applyRefactor({ + refactorName: "Convert string concatenation or template literal", + actionName: "Convert to template literal", + actionDescription: "Convert to template literal", + newContent: +"const foo = `foobar is\${/* C1 */ 42 /* C2 */ /* C3 */} years old`", +}); + diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingExpr.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingExpr.ts new file mode 100644 index 00000000000..25e006bd169 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingExpr.ts @@ -0,0 +1,13 @@ +/// + +//// const foo = /* C0 */ /*x*/"/*y*/foo" /* C1 */ + " is" /* C2 */ + 42/* C3 */ + +goTo.select("x", "y"); +edit.applyRefactor({ + refactorName: "Convert string concatenation or template literal", + actionName: "Convert to template literal", + actionDescription: "Convert to template literal", + newContent: +"const foo = /* C0 */ `foo is\${ /* C1 */ /* C2 */42 /* C3 */}`", +}); + diff --git a/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingStr.ts b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingStr.ts new file mode 100644 index 00000000000..9c5e6a5f4cb --- /dev/null +++ b/tests/cases/fourslash/refactorConvertStringOrTemplateLiteral_ToTemplateCommentTrailingStr.ts @@ -0,0 +1,13 @@ +/// + +//// const foo = /* C0 */ /*x*/"/*y*/foo" /* C1 */ + " is" /* C2 */ + 42 + " years old"/* C3 */ + +goTo.select("x", "y"); +edit.applyRefactor({ + refactorName: "Convert string concatenation or template literal", + actionName: "Convert to template literal", + actionDescription: "Convert to template literal", + newContent: +"const foo = /* C0 */ `foo is\${ /* C1 */ /* C2 */42} years old` /* C3 */", +}); +