fix: #468 - Removing class members should not assume it's in a class.

It could be in a class expression or object literal expression, for example.
dsherret · Oct 20, 2018 · 2c4db99 · 2c4db99
1 parent 9a9a7a6
commit 2c4db99
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 8 deletions.
diff --git a/src/manipulation/formatting/getClassMemberFormatting.ts b/src/manipulation/formatting/getClassMemberFormatting.ts
@@ -1,9 +1,9 @@
-import { ClassDeclaration, Node } from "../../compiler";
+import { Node } from "../../compiler";
 import { TypeGuards } from "../../utils";
 import { FormattingKind } from "./FormattingKind";
 
-export function getClassMemberFormatting(parent: ClassDeclaration, member: Node) {
-    if (parent.isAmbient())
+export function getClassMemberFormatting(parent: Node, member: Node) {
+    if (TypeGuards.isAmbientableNode(parent) && parent.isAmbient())
         return FormattingKind.Newline;
 
     if (hasBody(member))

diff --git a/src/manipulation/manipulations/removal.ts b/src/manipulation/manipulations/removal.ts
@@ -1,5 +1,6 @@
 import { ClassDeclaration, Node, OverloadableNode } from "../../compiler";
 import { SyntaxKind } from "../../typescript";
+import { TypeGuards } from "../../utils";
 import { FormattingKind, getClassMemberFormatting, getClausedNodeChildFormatting, getInterfaceMemberFormatting, getStatementedNodeChildFormatting } from "../formatting";
 import { NodeHandlerFactory } from "../nodeHandlers";
 import { RemoveChildrenTextManipulator, RemoveChildrenWithFormattingTextManipulator, UnwrapTextManipulator } from "../textManipulators";
@@ -65,7 +66,8 @@ export function removeChildrenWithFormatting<TNode extends Node>(opts: RemoveChi
 
 export function removeOverloadableClassMember(classMember: Node & OverloadableNode) {
     if (classMember.isOverload()) {
-        if ((classMember.getParentOrThrow() as ClassDeclaration).isAmbient())
+        const parent = classMember.getParentOrThrow();
+        if (TypeGuards.isAmbientableNode(parent) && parent.isAmbient())
             removeClassMember(classMember);
         else
             removeChildren({ children: [classMember], removeFollowingSpaces: true, removeFollowingNewLines: true });

diff --git a/src/tests/compiler/class/methodDeclarationTests.ts b/src/tests/compiler/class/methodDeclarationTests.ts
@@ -1,9 +1,9 @@
 import { expect } from "chai";
 import { ClassDeclaration, MethodDeclaration, Scope } from "../../../compiler";
 import { MethodDeclarationOverloadStructure, MethodDeclarationSpecificStructure, MethodDeclarationStructure } from "../../../structures";
-import { getInfoFromText } from "../testHelpers";
 import { SyntaxKind } from "../../../typescript";
-import { TypeGuards } from "../../../utils";
+import { ArrayUtils } from "../../../utils";
+import { getInfoFromText } from "../testHelpers";
 
 describe(nameof(MethodDeclaration), () => {
     describe(nameof<MethodDeclaration>(f => f.insertOverloads), () => {
@@ -94,8 +94,9 @@ describe(nameof(MethodDeclaration), () => {
     describe(nameof<MethodDeclaration>(m => m.remove), () => {
         describe("no overload", () => {
             function doTest(code: string, nameToRemove: string, expectedCode: string) {
-                const {firstChild, sourceFile} = getInfoFromText<ClassDeclaration>(code);
-                firstChild.getInstanceMethod(nameToRemove)!.remove();
+                const { sourceFile } = getInfoFromText<ClassDeclaration>(code);
+                const method = ArrayUtils.find(sourceFile.getDescendantsOfKind(SyntaxKind.MethodDeclaration), m => m.getName() === nameToRemove)!;
+                method.remove();
                 expect(sourceFile.getFullText()).to.equal(expectedCode);
             }
 
@@ -133,6 +134,11 @@ describe(nameof(MethodDeclaration), () => {
                 doTest("declare class Identifier {\n    method(): void;\n\n    prop: string;\n\n    method2(): void;\n}", "method",
                     "declare class Identifier {\n    prop: string;\n\n    method2(): void;\n}");
             });
+
+            it("should remove when it's in an class expression", () => {
+                doTest("const myTest = class {\n    method1() {\n    }\n\n    method2() {\n    }\n\n    method3() {\n    }\n}", "method2",
+                    "const myTest = class {\n    method1() {\n    }\n\n    method3() {\n    }\n}");
+            });
         });
 
         describe("overloads", () => {