Clean up refactoring

Author: RyanCavanaugh

src/services/refactors/convertFunctionToEs6Class.ts

@@ -130,6 +130,12 @@ namespace ts.refactor {
 
             return memberElements;
 
+            function shouldConvertDeclaration(_target: PropertyAccessExpression, source: Expression) {
+                // Right now the only thing we can convert are function expressions - other values shouldn't get
+                // transformed. We can update this once ES public class properties are available.
+                return isFunctionLike(source);
+            }
+
             function createClassElement(symbol: Symbol, modifiers: Modifier[]): ClassElement {
                 // both properties and methods are bound as property symbols
                 if (!(symbol.flags & SymbolFlags.Property)) {
@@ -139,6 +145,10 @@ namespace ts.refactor {
                 const memberDeclaration = symbol.valueDeclaration as PropertyAccessExpression;
                 const assignmentBinaryExpression = memberDeclaration.parent as BinaryExpression;
 
+                if (!shouldConvertDeclaration(memberDeclaration, assignmentBinaryExpression.right)) {
+                    return;
+                }
+
                 // delete the entire statement if this expression is the sole expression to take care of the semicolon at the end
                 const nodeToDelete = assignmentBinaryExpression.parent && assignmentBinaryExpression.parent.kind === SyntaxKind.ExpressionStatement
                     ? assignmentBinaryExpression.parent : assignmentBinaryExpression;
@@ -171,7 +181,12 @@ namespace ts.refactor {
                         }
                         return createMethod(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined,
                             /*typeParameters*/ undefined, arrowFunction.parameters, /*type*/ undefined, bodyBlock);
+
                     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);
                 }

tests/cases/fourslash/convertFunctionToEs6Class1.ts

@@ -11,17 +11,16 @@
 //// /*7*/foo.staticMethod1 = function() { return "this is static name"; };
 //// /*8*/foo.staticMethod2 = () => "this is static name";|]
 
-
 ['1', '2', '3', '4', '5', '6', '7', '8'].forEach(m => verify.applicableRefactorAvailableAtMarker(m));
 verify.fileAfterApplyingRefactorAtMarker('1',
 `class foo {
     constructor() { }
     instanceMethod1() { return "this is name"; }
     instanceMethod2() { return "this is name"; }
-    instanceProp1 = "hello";
-    instanceProp2 = undefined;
-    static staticProp = "world";
     static staticMethod1() { return "this is static name"; }
     static staticMethod2() { return "this is static name"; }
 }
+foo.prototype.instanceProp1 = "hello";
+foo.prototype.instanceProp2 = undefined;
+foo.staticProp = "world";
 `, 'Convert to ES2015 class');

tests/cases/fourslash/convertFunctionToEs6Class2.ts

@@ -5,8 +5,8 @@
 //// [|var /*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;
+//// /*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";|]
@@ -18,10 +18,10 @@ verify.fileAfterApplyingRefactorAtMarker('4',
     constructor() { }
     instanceMethod1() { return "this is name"; }
     instanceMethod2() { return "this is name"; }
-    instanceProp1 = "hello";
-    instanceProp2 = undefined;
-    static staticProp = "world";
     static staticMethod1() { return "this is static name"; }
     static staticMethod2() { return "this is static name"; }
 }
+foo.instanceProp1 = "hello";
+foo.instanceProp2 = undefined;
+foo.staticProp = "world";
 `, 'Convert to ES2015 class');

tests/cases/fourslash/convertFunctionToEs6Class3.ts

@@ -19,10 +19,10 @@ class foo {
     constructor() { }
     instanceMethod1() { return "this is name"; }
     instanceMethod2() { return "this is name"; }
-    instanceProp1 = "hello";
-    instanceProp2 = undefined;
-    static staticProp = "world";
     static staticMethod1() { return "this is static name"; }
     static staticMethod2() { return "this is static name"; }
 }
+foo.prototype.instanceProp1 = "hello";
+foo.prototype.instanceProp2 = undefined;
+foo.staticProp = "world";
 `, 'Convert to ES2015 class');

tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts

@@ -1,21 +1,25 @@
 // @allowNonTsExtensions: true
 // @Filename: test123.js
 
+/// <reference path="../fourslash.ts" />
+
+//// // Comment
 //// function fn() {
 ////     this.baz = 10;
 //// }
 //// /*1*/fn.prototype.bar = function () {
+////     console.log('hello world');
 //// }
 
-debugger;
 verify.applicableRefactorAvailableAtMarker('1');
 verify.fileAfterApplyingRefactorAtMarker('1',
-`class fn {
+`// Comment
+class fn {
     constructor() {
         this.baz = 10;
     }
-    baz;
     bar() {
+        console.log('hello world');
     }
 }
-`, 'Convert to ES2015 class');
+`, 'Convert to ES2015 class');