Make convertFunctionToEs6Class a codefix

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",

src/services/refactors/convertFunctionToEs6Class.ts → 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,28 +50,22 @@ 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
                 return;
             }
             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<Modifier> {
+        return filter(source.modifiers, modifier => modifier.kind === kind);
     }
 }

src/services/codefixes/fixes.ts

@@ -1,4 +1,5 @@
 /// <reference path="addMissingInvocationForDecorator.ts" />
+/// <reference path="convertFunctionToEs6Class.ts" />
 /// <reference path="convertToEs6Module.ts" />
 /// <reference path="correctQualifiedNameToIndexedAccessType.ts" />
 /// <reference path="fixClassIncorrectlyImplementsInterface.ts" />

src/services/refactors/refactors.ts

@@ -1,4 +1,3 @@
 /// <reference path="annotateWithTypeFromJSDoc.ts" />
-/// <reference path="convertFunctionToEs6Class.ts" />
 /// <reference path="extractSymbol.ts" />
 /// <reference path="useDefaultImport.ts" />

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));
     }
 }

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');
+`,
+});

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');
+`,
+});

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');
+`,
+});