Fix bug: don't insert a semicolon when inserting a FunctionDeclaration

Author: Andy Hanson

src/compiler/utilities.ts

@@ -5618,6 +5618,10 @@ namespace ts {
             || kind === SyntaxKind.MissingDeclaration;
     }
 
+    export function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement {
+        return isTypeElement(node) || isClassElement(node);
+    }
+
     export function isObjectLiteralElementLike(node: Node): node is ObjectLiteralElementLike {
         const kind = node.kind;
         return kind === SyntaxKind.PropertyAssignment

src/services/completions.ts

@@ -2248,7 +2248,7 @@ namespace ts.Completions {
 
     // TODO: GH#19856 Would like to return `node is Node & { parent: (ClassElement | TypeElement) & { parent: ObjectTypeDeclaration } }` but then compilation takes > 10 minutes
     function isFromObjectTypeDeclaration(node: Node): boolean {
-        return node.parent && (isClassElement(node.parent) || isTypeElement(node.parent)) && isObjectTypeDeclaration(node.parent.parent);
+        return node.parent && isClassOrTypeElement(node.parent) && isObjectTypeDeclaration(node.parent.parent);
     }
 
     function hasIndexSignature(type: Type): boolean {

src/services/textChanges.ts

@@ -447,10 +447,7 @@ namespace ts.textChanges {
         }
 
         public insertNodeAfter(sourceFile: SourceFile, after: Node, newNode: Node): this {
-            if (isStatementButNotDeclaration(after) ||
-                after.kind === SyntaxKind.PropertyDeclaration ||
-                after.kind === SyntaxKind.PropertySignature ||
-                after.kind === SyntaxKind.MethodSignature) {
+            if (needSemicolonBetween(after, newNode)) {
                 // check if previous statement ends with semicolon
                 // if not - insert semicolon to preserve the code from changing the meaning due to ASI
                 if (sourceFile.text.charCodeAt(after.end - 1) !== CharacterCodes.semicolon) {
@@ -465,7 +462,7 @@ namespace ts.textChanges {
             if (isClassDeclaration(node) || isModuleDeclaration(node)) {
                 return { prefix: this.newLineCharacter, suffix: this.newLineCharacter };
             }
-            else if (isStatement(node) || isClassElement(node) || isTypeElement(node)) {
+            else if (isStatement(node) || isClassOrTypeElement(node)) {
                 return { suffix: this.newLineCharacter };
             }
             else if (isVariableDeclaration(node)) {
@@ -893,4 +890,9 @@ namespace ts.textChanges {
     export function isValidLocationToAddComment(sourceFile: SourceFile, position: number) {
         return !isInComment(sourceFile, position) && !isInString(sourceFile, position) && !isInTemplateString(sourceFile, position);
     }
+
+    function needSemicolonBetween(a: Node, b: Node): boolean {
+        return (isPropertySignature(a) || isPropertyDeclaration(a)) && isClassOrTypeElement(b) && b.name.kind === SyntaxKind.ComputedPropertyName
+            || isStatementButNotDeclaration(a) && isStatementButNotDeclaration(b); // TODO: only if b would start with a `(` or `[`
+    }
 }

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -3243,6 +3243,7 @@ declare namespace ts {
     function isClassLike(node: Node): node is ClassLikeDeclaration;
     function isAccessor(node: Node): node is AccessorDeclaration;
     function isTypeElement(node: Node): node is TypeElement;
+    function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement;
     function isObjectLiteralElementLike(node: Node): node is ObjectLiteralElementLike;
     /**
      * Node test that determines whether a node is a valid type node.

tests/baselines/reference/api/typescript.d.ts

@@ -3298,6 +3298,7 @@ declare namespace ts {
     function isClassLike(node: Node): node is ClassLikeDeclaration;
     function isAccessor(node: Node): node is AccessorDeclaration;
     function isTypeElement(node: Node): node is TypeElement;
+    function isClassOrTypeElement(node: Node): node is ClassElement | TypeElement;
     function isObjectLiteralElementLike(node: Node): node is ObjectLiteralElementLike;
     /**
      * Node test that determines whether a node is a valid type node.

tests/baselines/reference/textChanges/insertNodeAfterInClass1.js

@@ -7,6 +7,6 @@ class A {
 ===MODIFIED===
 
 class A {
-    x;
+    x
     a: boolean;
 }

tests/baselines/reference/textChanges/insertNodeInInterfaceAfterNodeWithoutSeparator2.js

@@ -7,6 +7,6 @@ interface A {
 ===MODIFIED===
 
 interface A {
-    x();
+    x()
     [1]: any;
 }

tests/cases/fourslash/convertFunctionToEs6ClassNoSemicolon.ts

@@ -0,0 +1,16 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+
+// @Filename: /a.js
+////var C = function() { this.x = 0; }
+////0;
+
+verify.codeFix({
+    description: "Convert function to an ES2015 class",
+    newFileContent:
+`class C {
+    constructor() { this.x = 0; }
+}
+0;`,
+});