Fix bug: don't insert a semicolon when inserting a FunctionDeclaration (#23240) (#23289)

Author: Andy

src/compiler/utilities.ts

@@ -5399,6 +5399,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/textChanges.ts

@@ -429,10 +429,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) {
@@ -453,7 +450,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)) {
@@ -904,4 +901,9 @@ namespace ts.textChanges {
             }
         }
     }
+
+    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

@@ -3219,6 +3219,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

@@ -3274,6 +3274,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;`,
+});