diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index a76a71fb40735..8b6f0edb05914 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3822,6 +3822,10 @@ "category": "Suggestion", "code": 80001 }, + "This constructor function may be converted to a class declaration.": { + "category": "Suggestion", + "code": 80002 + }, "Add missing 'super()' call": { "category": "Message", diff --git a/src/services/refactors/convertFunctionToEs6Class.ts b/src/services/codefixes/convertFunctionToEs6Class.ts similarity index 71% rename from src/services/refactors/convertFunctionToEs6Class.ts rename to src/services/codefixes/convertFunctionToEs6Class.ts index ddb13c3e04ce7..67098a0a4dc60 100644 --- a/src/services/refactors/convertFunctionToEs6Class.ts +++ b/src/services/codefixes/convertFunctionToEs6Class.ts @@ -1,59 +1,28 @@ /* @internal */ - -namespace ts.refactor.convertFunctionToES6Class { - const refactorName = "Convert to ES2015 class"; - const actionName = "convert"; - const description = Diagnostics.Convert_function_to_an_ES2015_class.message; - registerRefactor(refactorName, { getEditsForAction, getAvailableActions }); - - function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined { - if (!isInJavaScriptFile(context.file)) { - return undefined; - } - - let symbol = getConstructorSymbol(context); - if (!symbol) { - return undefined; - } - - if (isDeclarationOfFunctionOrClassExpression(symbol)) { - symbol = (symbol.valueDeclaration as VariableDeclaration).initializer.symbol; - } - - if ((symbol.flags & SymbolFlags.Function) && symbol.members && (symbol.members.size > 0)) { - return [ - { - name: refactorName, - description, - actions: [ - { - description, - name: actionName - } - ] - } - ]; - } - } - - function getEditsForAction(context: RefactorContext, action: string): RefactorEditInfo | undefined { - // Somehow wrong action got invoked? - if (actionName !== action) { - return undefined; - } - - const { file: sourceFile } = context; - const ctorSymbol = getConstructorSymbol(context); - +namespace ts.codefix { + const fixId = "convertFunctionToEs6Class"; + const errorCodes = [Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration.code]; + registerCodeFix({ + errorCodes, + getCodeActions(context: CodeFixContext) { + const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, context.span.start, context.program.getTypeChecker())); + return [{ description: getLocaleSpecificMessage(Diagnostics.Convert_function_to_an_ES2015_class), changes, fixId }]; + }, + fixIds: [fixId], + getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, err) => doChange(changes, err.file!, err.start, context.program.getTypeChecker())), + }); + + function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, position: number, checker: TypeChecker): void { const deletedNodes: Node[] = []; - const deletes: (() => any)[] = []; + const deletes: (() => void)[] = []; + const ctorSymbol = checker.getSymbolAtLocation(getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false)); - if (!(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) { + if (!ctorSymbol || !(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) { + // Bad input return undefined; } const ctorDeclaration = ctorSymbol.valueDeclaration; - const changeTracker = textChanges.ChangeTracker.fromContext(context); let precedingNode: Node; let newClassDeclaration: ClassDeclaration; @@ -81,17 +50,11 @@ namespace ts.refactor.convertFunctionToES6Class { } // Because the preceding node could be touched, we need to insert nodes before delete nodes. - changeTracker.insertNodeAfter(sourceFile, precedingNode, newClassDeclaration); + changes.insertNodeAfter(sourceFile, precedingNode, newClassDeclaration); for (const deleteCallback of deletes) { deleteCallback(); } - return { - edits: changeTracker.getChanges(), - renameFilename: undefined, - renameLocation: undefined, - }; - function deleteNode(node: Node, inList = false) { if (deletedNodes.some(n => isNodeDescendantOf(node, n))) { // Parent node has already been deleted; do nothing @@ -99,10 +62,10 @@ namespace ts.refactor.convertFunctionToES6Class { } deletedNodes.push(node); if (inList) { - deletes.push(() => changeTracker.deleteNodeInList(sourceFile, node)); + deletes.push(() => changes.deleteNodeInList(sourceFile, node)); } else { - deletes.push(() => changeTracker.deleteNode(sourceFile, node)); + deletes.push(() => changes.deleteNode(sourceFile, node)); } } @@ -165,7 +128,7 @@ namespace ts.refactor.convertFunctionToES6Class { const fullModifiers = concatenate(modifiers, getModifierKindFromSource(functionExpression, SyntaxKind.AsyncKeyword)); const method = createMethod(/*decorators*/ undefined, fullModifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, /*typeParameters*/ undefined, functionExpression.parameters, /*type*/ undefined, functionExpression.body); - copyComments(assignmentBinaryExpression, method); + copyComments(assignmentBinaryExpression, method, sourceFile); return method; } @@ -185,7 +148,7 @@ namespace ts.refactor.convertFunctionToES6Class { const fullModifiers = concatenate(modifiers, getModifierKindFromSource(arrowFunction, SyntaxKind.AsyncKeyword)); const method = createMethod(/*decorators*/ undefined, fullModifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, /*typeParameters*/ undefined, arrowFunction.parameters, /*type*/ undefined, bodyBlock); - copyComments(assignmentBinaryExpression, method); + copyComments(assignmentBinaryExpression, method, sourceFile); return method; } @@ -196,29 +159,13 @@ namespace ts.refactor.convertFunctionToES6Class { } const prop = createProperty(/*decorators*/ undefined, modifiers, memberDeclaration.name, /*questionToken*/ undefined, /*type*/ undefined, assignmentBinaryExpression.right); - copyComments(assignmentBinaryExpression.parent, prop); + copyComments(assignmentBinaryExpression.parent, prop, sourceFile); return prop; } } } } - 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) { @@ -253,15 +200,25 @@ namespace ts.refactor.convertFunctionToES6Class { // Don't call copyComments here because we'll already leave them in place return cls; } + } - function getModifierKindFromSource(source: Node, kind: SyntaxKind) { - return filter(source.modifiers, modifier => modifier.kind === kind); - } + function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile) { + 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 getConstructorSymbol({ startPosition, file, program }: RefactorContext): Symbol { - const checker = program.getTypeChecker(); - const token = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false); - return checker.getSymbolAtLocation(token); + function getModifierKindFromSource(source: Node, kind: SyntaxKind): ReadonlyArray { + return filter(source.modifiers, modifier => modifier.kind === kind); } } \ No newline at end of file diff --git a/src/services/codefixes/fixes.ts b/src/services/codefixes/fixes.ts index 24b726717a04d..27435a56dbf59 100644 --- a/src/services/codefixes/fixes.ts +++ b/src/services/codefixes/fixes.ts @@ -1,4 +1,5 @@ /// +/// /// /// /// diff --git a/src/services/refactors/refactors.ts b/src/services/refactors/refactors.ts index bc82b2a24732f..c63f66dbbb8e6 100644 --- a/src/services/refactors/refactors.ts +++ b/src/services/refactors/refactors.ts @@ -1,4 +1,3 @@ /// -/// /// /// diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 2588dee2726bc..bb6e510d2fdea 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -9,6 +9,22 @@ namespace ts { diags.push(createDiagnosticForNode(sourceFile.commonJsModuleIndicator, Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module)); } + function check(node: Node) { + switch (node.kind) { + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.FunctionExpression: + const symbol = node.symbol; + if (symbol.members && (symbol.members.size > 0)) { + diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration)); + } + break; + } + node.forEachChild(check); + } + if (isInJavaScriptFile(sourceFile)) { + check(sourceFile); + } + return diags.concat(checker.getSuggestionDiagnostics(sourceFile)); } } diff --git a/tests/cases/fourslash/convertFunctionToEs6Class1.ts b/tests/cases/fourslash/convertFunctionToEs6Class1.ts index 51cc73fd64ac7..d14ca4f23bd00 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class1.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class1.ts @@ -2,17 +2,24 @@ // @allowNonTsExtensions: true // @Filename: test123.js -//// [|function /*1*/foo() { } -//// /*2*/foo.prototype.instanceMethod1 = function() { return "this is name"; }; -//// /*3*/foo.prototype.instanceMethod2 = () => { return "this is name"; }; -//// /*4*/foo.prototype.instanceProp1 = "hello"; -//// /*5*/foo.prototype.instanceProp2 = undefined; -//// /*6*/foo.staticProp = "world"; -//// /*7*/foo.staticMethod1 = function() { return "this is static name"; }; -//// /*8*/foo.staticMethod2 = () => "this is static name";|] +////function [|foo|]() { } +////foo.prototype.instanceMethod1 = function() { return "this is name"; }; +////foo.prototype.instanceMethod2 = () => { return "this is name"; }; +////foo.prototype.instanceProp1 = "hello"; +////foo.prototype.instanceProp2 = undefined; +////foo.staticProp = "world"; +////foo.staticMethod1 = function() { return "this is static name"; }; +////foo.staticMethod2 = () => "this is static name"; -['1', '2', '3', '4', '5', '6', '7', '8'].forEach(m => verify.applicableRefactorAvailableAtMarker(m)); -verify.fileAfterApplyingRefactorAtMarker('1', +verify.getSuggestionDiagnostics([{ + message: "This constructor function may be converted to a class declaration.", + category: "suggestion", + code: 80002, +}]); + +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `class foo { constructor() { } instanceMethod1() { return "this is name"; } @@ -23,4 +30,5 @@ verify.fileAfterApplyingRefactorAtMarker('1', foo.prototype.instanceProp1 = "hello"; foo.prototype.instanceProp2 = undefined; foo.staticProp = "world"; -`, 'Convert to ES2015 class', 'convert'); \ No newline at end of file +`, +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class2.ts b/tests/cases/fourslash/convertFunctionToEs6Class2.ts index d5ed277b45220..5326c30eea496 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class2.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class2.ts @@ -2,18 +2,18 @@ // @allowNonTsExtensions: true // @Filename: test123.js -//// [|var /*1*/foo = function() { }; -//// /*2*/foo.prototype.instanceMethod1 = function() { return "this is name"; }; -//// /*3*/foo.prototype.instanceMethod2 = () => { return "this is name"; }; -//// /*4*/foo.instanceProp1 = "hello"; -//// /*5*/foo.instanceProp2 = undefined; -//// /*6*/foo.staticProp = "world"; -//// /*7*/foo.staticMethod1 = function() { return "this is static name"; }; -//// /*8*/foo.staticMethod2 = () => "this is static name";|] +////var foo = function() { }; +////foo.prototype.instanceMethod1 = function() { return "this is name"; }; +////foo.prototype.instanceMethod2 = () => { return "this is name"; }; +////foo.instanceProp1 = "hello"; +////foo.instanceProp2 = undefined; +////foo.staticProp = "world"; +////foo.staticMethod1 = function() { return "this is static name"; }; +////foo.staticMethod2 = () => "this is static name"; - -['1', '2', '3', '4', '5', '6', '7', '8'].forEach(m => verify.applicableRefactorAvailableAtMarker(m)); -verify.fileAfterApplyingRefactorAtMarker('4', +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `class foo { constructor() { } instanceMethod1() { return "this is name"; } @@ -24,4 +24,5 @@ verify.fileAfterApplyingRefactorAtMarker('4', foo.instanceProp1 = "hello"; foo.instanceProp2 = undefined; foo.staticProp = "world"; -`, 'Convert to ES2015 class', 'convert'); +`, +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class3.ts b/tests/cases/fourslash/convertFunctionToEs6Class3.ts index fec4dd8edfae2..d88c4b80a8ddb 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class3.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class3.ts @@ -2,18 +2,18 @@ // @allowNonTsExtensions: true // @Filename: test123.js -//// var bar = 10, /*1*/foo = function() { }; -//// /*2*/foo.prototype.instanceMethod1 = function() { return "this is name"; }; -//// /*3*/foo.prototype.instanceMethod2 = () => { return "this is name"; }; -//// /*4*/foo.prototype.instanceProp1 = "hello"; -//// /*5*/foo.prototype.instanceProp2 = undefined; -//// /*6*/foo.staticProp = "world"; -//// /*7*/foo.staticMethod1 = function() { return "this is static name"; }; -//// /*8*/foo.staticMethod2 = () => "this is static name"; +////var bar = 10, foo = function() { }; +////foo.prototype.instanceMethod1 = function() { return "this is name"; }; +////foo.prototype.instanceMethod2 = () => { return "this is name"; }; +////foo.prototype.instanceProp1 = "hello"; +////foo.prototype.instanceProp2 = undefined; +////foo.staticProp = "world"; +////foo.staticMethod1 = function() { return "this is static name"; }; +////foo.staticMethod2 = () => "this is static name"; - -['1', '2', '3', '4', '5', '6', '7', '8'].forEach(m => verify.applicableRefactorAvailableAtMarker(m)); -verify.fileAfterApplyingRefactorAtMarker('7', +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `var bar = 10; class foo { constructor() { } @@ -25,4 +25,5 @@ class foo { foo.prototype.instanceProp1 = "hello"; foo.prototype.instanceProp2 = undefined; foo.staticProp = "world"; -`, 'Convert to ES2015 class', 'convert'); \ No newline at end of file +`, +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts index e9f6449c8fd4e..fe21e58544426 100644 --- a/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts +++ b/tests/cases/fourslash/convertFunctionToEs6ClassJsDoc.ts @@ -2,26 +2,29 @@ // @allowNonTsExtensions: true // @Filename: test123.js -//// function fn() { -//// /** neat! */ -//// this.x = 100; -//// } +////function fn() { +//// /** neat! */ +//// this.x = 100; +////} //// -//// /** awesome -//// * stuff -//// */ -//// fn.prototype.arr = () => { return ""; } -//// /** great */ -//// fn.prototype.arr2 = () => []; -//// -//// /** -//// * This is a cool function! -//// */ -//// /*1*/fn.prototype.bar = function (x, y, z) { -//// this.x = y; -//// }; +/////** awesome +//// * stuff +//// */ +////fn.prototype.arr = () => { return ""; } +/////** great */ +////fn.prototype.arr2 = () => []; +//// +/////** +//// * This is a cool function! +////*/ +////fn.prototype.bar = function (x, y, z) { +//// this.x = y; +////}; -verify.fileAfterApplyingRefactorAtMarker('1', +verify.codeFix({ + description: "Convert function to an ES2015 class", + index: 0, // TODO: GH#22240 + newFileContent: `class fn { constructor() { /** neat! */ @@ -42,4 +45,5 @@ verify.fileAfterApplyingRefactorAtMarker('1', } -`, 'Convert to ES2015 class', 'convert'); +`, +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts b/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts index ed230d504351d..fb3b46763c878 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts @@ -11,8 +11,9 @@ //// await 3; ////} -verify.applicableRefactorAvailableAtMarker(""); -verify.fileAfterApplyingRefactorAtMarker("", +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `export class MyClass { constructor() { } @@ -24,4 +25,4 @@ verify.fileAfterApplyingRefactorAtMarker("", } } `, -'Convert to ES2015 class', 'convert'); +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class_emptySwitchCase.ts b/tests/cases/fourslash/convertFunctionToEs6Class_emptySwitchCase.ts index 90bd48784ce8b..f055b9bbfa24f 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class_emptySwitchCase.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class_emptySwitchCase.ts @@ -10,8 +10,9 @@ //// } ////} -verify.applicableRefactorAvailableAtMarker(""); -verify.fileAfterApplyingRefactorAtMarker("", +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `class MyClass { constructor() { } @@ -22,4 +23,4 @@ verify.fileAfterApplyingRefactorAtMarker("", } } `, -'Convert to ES2015 class', 'convert'); +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier1.ts b/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier1.ts index 940a68a05b615..76a853c0de3cd 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier1.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier1.ts @@ -7,8 +7,9 @@ ////MyClass.prototype.foo = function() { ////} -verify.applicableRefactorAvailableAtMarker(""); -verify.fileAfterApplyingRefactorAtMarker("", +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `export class MyClass { constructor() { } @@ -16,4 +17,4 @@ verify.fileAfterApplyingRefactorAtMarker("", } } `, -'Convert to ES2015 class', 'convert'); +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier2.ts b/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier2.ts index fb1276d4f0352..ec0566440cb8c 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier2.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class_exportModifier2.ts @@ -2,13 +2,14 @@ // @allowNonTsExtensions: true // @Filename: test123.js -////export const /**/foo = function() { +////export const foo = function() { ////}; ////foo.prototype.instanceMethod = function() { ////}; -verify.applicableRefactorAvailableAtMarker(""); -verify.fileAfterApplyingRefactorAtMarker("", +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `export class foo { constructor() { } @@ -16,4 +17,4 @@ verify.fileAfterApplyingRefactorAtMarker("", } } `, -'Convert to ES2015 class', 'convert'); +}); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class_objectLiteralInArrowFunction.ts b/tests/cases/fourslash/convertFunctionToEs6Class_objectLiteralInArrowFunction.ts index 0bbbf4e00246c..a86af1e6d3d29 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class_objectLiteralInArrowFunction.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class_objectLiteralInArrowFunction.ts @@ -8,8 +8,9 @@ //// ({ bar: () => { } }) ////} -verify.applicableRefactorAvailableAtMarker(""); -verify.fileAfterApplyingRefactorAtMarker("", +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `class MyClass { constructor() { } @@ -18,4 +19,4 @@ verify.fileAfterApplyingRefactorAtMarker("", } } `, -'Convert to ES2015 class', 'convert'); +}); diff --git a/tests/cases/fourslash/convertToEs6Class_emptyCatchClause.ts b/tests/cases/fourslash/convertToEs6Class_emptyCatchClause.ts index e9027178aa71b..7fed94e84cc63 100644 --- a/tests/cases/fourslash/convertToEs6Class_emptyCatchClause.ts +++ b/tests/cases/fourslash/convertToEs6Class_emptyCatchClause.ts @@ -7,8 +7,9 @@ //// try {} catch() {} ////} -verify.applicableRefactorAvailableAtMarker(""); -verify.fileAfterApplyingRefactorAtMarker("", +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: `class MyClass { constructor() { } foo() { @@ -17,4 +18,4 @@ verify.fileAfterApplyingRefactorAtMarker("", } } `, -'Convert to ES2015 class', 'convert'); +}); diff --git a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts index 0bdd44f6753a0..498c976e71ecb 100644 --- a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts +++ b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts @@ -11,10 +11,11 @@ //// console.log('hello world'); //// } -verify.applicableRefactorAvailableAtMarker('1'); +verify.codeFix({ + description: "Convert function to an ES2015 class", + newFileContent: // NOTE: '// Comment' should be included, but due to incorrect handling of trivia, // it's omitted right now. -verify.fileAfterApplyingRefactorAtMarker('1', `class fn {\r constructor() {\r this.baz = 10;\r @@ -23,4 +24,5 @@ verify.fileAfterApplyingRefactorAtMarker('1', console.log('hello world');\r }\r }\r -`, 'Convert to ES2015 class', 'convert'); +`, +});