From 04d750f9f8fa6d4f687f355713b548ee946b5d61 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Thu, 22 Jun 2017 11:42:12 -0700 Subject: [PATCH 1/3] Preserve method comments in JS->ES6 conversion. Fixes #16622 --- .../refactors/convertFunctionToEs6Class.ts | 42 +++++++++++++++---- .../convertFunctionToEs6ClassJsDoc.ts | 29 +++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts diff --git a/src/services/refactors/convertFunctionToEs6Class.ts b/src/services/refactors/convertFunctionToEs6Class.ts index d275924b76a..925f0856a3f 100644 --- a/src/services/refactors/convertFunctionToEs6Class.ts +++ b/src/services/refactors/convertFunctionToEs6Class.ts @@ -164,12 +164,15 @@ namespace ts.refactor { } switch (assignmentBinaryExpression.right.kind) { - case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionExpression: { const functionExpression = assignmentBinaryExpression.right as FunctionExpression; - return createMethod(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, + const method = createMethod(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, /*typeParameters*/ undefined, functionExpression.parameters, /*type*/ undefined, functionExpression.body); + copyComments(assignmentBinaryExpression, method); + return method; + } - case SyntaxKind.ArrowFunction: + case SyntaxKind.ArrowFunction: { const arrowFunction = assignmentBinaryExpression.right as ArrowFunction; const arrowFunctionBody = arrowFunction.body; let bodyBlock: Block; @@ -183,20 +186,40 @@ namespace ts.refactor { const expression = arrowFunctionBody as Expression; bodyBlock = createBlock([createReturn(expression)]); } - return createMethod(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, + const method = createMethod(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, /*typeParameters*/ undefined, arrowFunction.parameters, /*type*/ undefined, bodyBlock); + copyComments(assignmentBinaryExpression, method); + return method; + } - default: + default: { // Don't try to declare members in JavaScript files if (isSourceFileJavaScript(sourceFile)) { return; } return createProperty(/*decorators*/ undefined, modifiers, memberDeclaration.name, /*questionToken*/ undefined, /*type*/ undefined, assignmentBinaryExpression.right); + } } } } + function copyComments(sourceNode: Node, targetNode: Node) { + forEachLeadingCommentRange(sourceFile.text, sourceNode.pos, (pos, end, kind, htnl) => { + if (kind === SyntaxKind.MultiLineCommentTrivia) { + // Remove leading /* + pos += 2; + // Remove trailing */ + end -= 2; + } + else { + // Remove leading // + pos += 2; + } + addSyntheticLeadingComment(targetNode, kind, sourceFile.text.slice(pos, end), htnl); + }); + } + function createClassFromVariableDeclaration(node: VariableDeclaration): ClassDeclaration { const initializer = node.initializer as FunctionExpression; if (!initializer || initializer.kind !== SyntaxKind.FunctionExpression) { @@ -212,8 +235,10 @@ namespace ts.refactor { memberElements.unshift(createConstructor(/*decorators*/ undefined, /*modifiers*/ undefined, initializer.parameters, initializer.body)); } - return createClassDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, node.name, + const cls = createClassDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, node.name, /*typeParameters*/ undefined, /*heritageClauses*/ undefined, memberElements); + // Don't call copyComments here because we'll already leave them in place + return cls; } function createClassFromFunctionDeclaration(node: FunctionDeclaration): ClassDeclaration { @@ -221,8 +246,11 @@ namespace ts.refactor { if (node.body) { memberElements.unshift(createConstructor(/*decorators*/ undefined, /*modifiers*/ undefined, node.parameters, node.body)); } - return createClassDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, node.name, + + const cls = createClassDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, node.name, /*typeParameters*/ undefined, /*heritageClauses*/ undefined, memberElements); + // Don't call copyComments here because we'll already leave them in place + return cls; } } } \ No newline at end of file diff --git a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts new file mode 100644 index 00000000000..afa4c6acf3e --- /dev/null +++ b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts @@ -0,0 +1,29 @@ +/// + +// @allowNonTsExtensions: true +// @Filename: test123.js +//// function fn() { +//// this.x = 100; +//// } +//// +//// /** +//// * This is a cool function! +//// */ +//// /*1*/fn.prototype.bar = function (x, y, z) { +//// this.x = y; +//// }; + +verify.fileAfterApplyingRefactorAtMarker('1', +`class fn { + constructor() { + this.x = 100; + } + /** + * This is a cool function! + */ + bar(x, y, z) { + this.x = y; + } +} + +`, 'Convert to ES2015 class', 'convert'); From b52747e12c7444512eb73a03737e83d8a92bc9ba Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Mon, 26 Jun 2017 10:54:18 -0700 Subject: [PATCH 2/3] Add property comments as well --- src/services/refactors/convertFunctionToEs6Class.ts | 4 +++- tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/services/refactors/convertFunctionToEs6Class.ts b/src/services/refactors/convertFunctionToEs6Class.ts index 925f0856a3f..745ba6d15bf 100644 --- a/src/services/refactors/convertFunctionToEs6Class.ts +++ b/src/services/refactors/convertFunctionToEs6Class.ts @@ -197,8 +197,10 @@ namespace ts.refactor { if (isSourceFileJavaScript(sourceFile)) { return; } - return createProperty(/*decorators*/ undefined, modifiers, memberDeclaration.name, /*questionToken*/ undefined, + const prop = createProperty(/*decorators*/ undefined, modifiers, memberDeclaration.name, /*questionToken*/ undefined, /*type*/ undefined, assignmentBinaryExpression.right); + copyComments(assignmentBinaryExpression.parent, prop); + return prop; } } } diff --git a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts index afa4c6acf3e..87ee2a738c2 100644 --- a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts +++ b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts @@ -3,6 +3,7 @@ // @allowNonTsExtensions: true // @Filename: test123.js //// function fn() { +//// /** neat! */ //// this.x = 100; //// } //// @@ -16,6 +17,7 @@ verify.fileAfterApplyingRefactorAtMarker('1', `class fn { constructor() { + /** neat! */ this.x = 100; } /** From 277f4592c1674d4ca5c7314ed08e189210206d54 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Thu, 29 Jun 2017 15:14:09 -0700 Subject: [PATCH 3/3] Add tests --- .../fourslash/convertFunctionToEs6ClassJsDoc.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts index 87ee2a738c2..e9f6449c8fd 100644 --- a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts +++ b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts @@ -6,6 +6,13 @@ //// /** neat! */ //// this.x = 100; //// } +//// +//// /** awesome +//// * stuff +//// */ +//// fn.prototype.arr = () => { return ""; } +//// /** great */ +//// fn.prototype.arr2 = () => []; //// //// /** //// * This is a cool function! @@ -20,6 +27,12 @@ verify.fileAfterApplyingRefactorAtMarker('1', /** neat! */ this.x = 100; } + /** awesome + * stuff + */ + arr() { return ""; } + /** great */ + arr2() { return []; } /** * This is a cool function! */ @@ -28,4 +41,5 @@ verify.fileAfterApplyingRefactorAtMarker('1', } } + `, 'Convert to ES2015 class', 'convert');