Avoid unnecessary newline when inserting node at start of class (microsoft#23935)

Andy · web-flow · commit 004a5581259f · 2018-05-07T12:40:33.000-07:00
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
@@ -442,19 +442,25 @@ namespace ts.textChanges {
 
         public insertNodeAtClassStart(sourceFile: SourceFile, cls: ClassLikeDeclaration, newElement: ClassElement): void {
             const clsStart = cls.getStart(sourceFile);
-            let prefix = "";
-            let suffix = this.newLineCharacter;
-            if (addToSeen(this.classesWithNodesInsertedAtStart, getNodeId(cls), cls)) {
-                prefix = this.newLineCharacter;
-                // For `class C {\n}`, don't add the trailing "\n"
-                if (cls.members.length === 0 && !(positionsAreOnSameLine as any)(...getClassBraceEnds(cls, sourceFile), sourceFile)) { // TODO: GH#4130 remove 'as any'
-                    suffix = "";
-                }
-            }
-
             const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(getLineStartPositionForPosition(clsStart, sourceFile), clsStart, sourceFile, this.formatContext.options)
                 + this.formatContext.options.indentSize;
-            this.insertNodeAt(sourceFile, cls.members.pos, newElement, { indentation, prefix, suffix });
+            this.insertNodeAt(sourceFile, cls.members.pos, newElement, { indentation, ...this.getInsertNodeAtClassStartPrefixSuffix(sourceFile, cls) });
+        }
+
+        private getInsertNodeAtClassStartPrefixSuffix(sourceFile: SourceFile, cls: ClassLikeDeclaration): { prefix: string, suffix: string } {
+            if (cls.members.length === 0) {
+                if (addToSeen(this.classesWithNodesInsertedAtStart, getNodeId(cls), cls)) {
+                    // For `class C {\n}`, don't add the trailing "\n"
+                    const shouldSuffix = (positionsAreOnSameLine as any)(...getClassBraceEnds(cls, sourceFile), sourceFile); // TODO: GH#4130 remove 'as any'
+                    return { prefix: this.newLineCharacter, suffix: shouldSuffix ? this.newLineCharacter : "" };
+                }
+                else {
+                    return { prefix: "", suffix: this.newLineCharacter };
+                }
+            }
+            else {
+                return { prefix: this.newLineCharacter, suffix: "" };
+            }
         }
 
         public insertNodeAfter(sourceFile: SourceFile, after: Node, newNode: Node): this {
diff --git a/tests/cases/fourslash/codeFixAddMissingMember.ts b/tests/cases/fourslash/codeFixAddMissingMember.ts
@@ -11,7 +11,6 @@ verify.codeFix({
     index: 0,
     newFileContent: `class C {
     foo: number;
-
     method() {
         this.foo = 10;
     }
diff --git a/tests/cases/fourslash/codeFixAddMissingMember2.ts b/tests/cases/fourslash/codeFixAddMissingMember2.ts
@@ -11,7 +11,6 @@ verify.codeFix({
     index: 1,
     newFileContent: `class C {
     [x: string]: number;
-
     method() {
         this.foo = 10;
     }
diff --git a/tests/cases/fourslash/codeFixAddMissingMember3.ts b/tests/cases/fourslash/codeFixAddMissingMember3.ts
@@ -11,7 +11,6 @@ verify.codeFix({
     index: 0,
     newFileContent: `class C {
     static foo: number;
-
     static method() {
         this.foo = 10;
     }
diff --git a/tests/cases/fourslash/codeFixAddMissingMember_all.ts b/tests/cases/fourslash/codeFixAddMissingMember_all.ts
@@ -17,7 +17,6 @@ verify.codeFixAll({
     y(): any {
         throw new Error("Method not implemented.");
     }
-
     method() {
         this.x = 0;
         this.y();
diff --git a/tests/cases/fourslash/codeFixAddMissingMember_all_js.ts b/tests/cases/fourslash/codeFixAddMissingMember_all_js.ts
@@ -21,7 +21,6 @@ verify.codeFixAll({
     y() {
         throw new Error("Method not implemented.");
     }
-
     constructor() {
         this.x = undefined;
     }
diff --git a/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts b/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts
@@ -17,7 +17,6 @@ verify.codeFix({
     static m1(arg0: any, arg1: any, arg2: any): any {
         throw new Error("Method not implemented.");
     }
-
     static foo0() {
         this.m1(1,2,3);
         A.m2(1,2);
@@ -35,11 +34,9 @@ verify.codeFix({
     static m2(arg0: any, arg1: any): any {
         throw new Error("Method not implemented.");
     }
-
     static m1(arg0: any, arg1: any, arg2: any): any {
         throw new Error("Method not implemented.");
     }
-
     static foo0() {
         this.m1(1,2,3);
         A.m2(1,2);
@@ -55,15 +52,12 @@ verify.codeFix({
     newFileContent:
 `class A {
     static prop1: number;
-
     static m2(arg0: any, arg1: any): any {
         throw new Error("Method not implemented.");
     }
-
     static m1(arg0: any, arg1: any, arg2: any): any {
         throw new Error("Method not implemented.");
     }
-
     static foo0() {
         this.m1(1,2,3);
         A.m2(1,2);
@@ -80,15 +74,12 @@ verify.codeFix({
 `class A {
     static prop1: number;
     static prop2: string;
-
     static m2(arg0: any, arg1: any): any {
         throw new Error("Method not implemented.");
     }
-
     static m1(arg0: any, arg1: any, arg2: any): any {
         throw new Error("Method not implemented.");
     }
-
     static foo0() {
         this.m1(1,2,3);
         A.m2(1,2);
diff --git a/tests/cases/fourslash/codeFixUndeclaredMethod.ts b/tests/cases/fourslash/codeFixUndeclaredMethod.ts
@@ -18,7 +18,6 @@ verify.codeFix({
     foo1(arg0: any, arg1: any, arg2: any): any {
         throw new Error("Method not implemented.");
     }
-
     constructor() {
         this.foo1(1,2,3);
         // 7 type args
@@ -37,11 +36,9 @@ verify.codeFix({
     foo2<T, U, V, W, X, Y, Z>(): any {
         throw new Error("Method not implemented.");
     }
-
     foo1(arg0: any, arg1: any, arg2: any): any {
         throw new Error("Method not implemented.");
     }
-
     constructor() {
         this.foo1(1,2,3);
         // 7 type args
@@ -60,15 +57,12 @@ verify.codeFix({
     foo3<T0, T1, T2, T3, T4, T5, T6, T7>(): any {
         throw new Error("Method not implemented.");
     }
-
     foo2<T, U, V, W, X, Y, Z>(): any {
         throw new Error("Method not implemented.");
     }
-
     foo1(arg0: any, arg1: any, arg2: any): any {
         throw new Error("Method not implemented.");
     }
-
     constructor() {
         this.foo1(1,2,3);
         // 7 type args
diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts
@@ -16,7 +16,6 @@ edit.applyRefactor({
     public set a(value: string) {
         this._a = value;
     }
-
     constructor(private /*RENAME*/_a: string) { }
 }`,
 });
diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts
@@ -16,7 +16,6 @@ edit.applyRefactor({
     protected set a(value: string) {
         this._a = value;
     }
-
     constructor(private /*RENAME*/_a: string) { }
 }`,
 });
diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts
@@ -16,7 +16,6 @@ edit.applyRefactor({
     public set a(value: string) {
         this._a = value;
     }
-
     constructor(private /*RENAME*/_a: string) { }
 }`,
 });
diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess22.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess22.ts
@@ -17,7 +17,6 @@ edit.applyRefactor({
     public set a(value: string) {
         this._a = value;
     }
-
     public a_1: number;
     constructor(private /*RENAME*/_a: string) { }
 }`,

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,6 @@ verify.codeFix({`
`11`	`11`	`index: 0,`
`12`	`12`	newFileContent: `class C {
`13`	`13`	`foo: number;`
`14`		`-`
`15`	`14`	`method() {`
`16`	`15`	`this.foo = 10;`
`17`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,6 @@ verify.codeFixAll({`
`17`	`17`	`y(): any {`
`18`	`18`	`throw new Error("Method not implemented.");`
`19`	`19`	`}`
`20`		`-`
`21`	`20`	`method() {`
`22`	`21`	`this.x = 0;`
`23`	`22`	`this.y();`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,6 @@ verify.codeFixAll({`
`21`	`21`	`y() {`
`22`	`22`	`throw new Error("Method not implemented.");`
`23`	`23`	`}`
`24`		`-`
`25`	`24`	`constructor() {`
`26`	`25`	`this.x = undefined;`
`27`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,6 @@ verify.codeFix({`
`18`	`18`	`foo1(arg0: any, arg1: any, arg2: any): any {`
`19`	`19`	`throw new Error("Method not implemented.");`
`20`	`20`	`}`
`21`		`-`
`22`	`21`	`constructor() {`
`23`	`22`	`this.foo1(1,2,3);`
`24`	`23`	`// 7 type args`
`@@ -37,11 +36,9 @@ verify.codeFix({`
`37`	`36`	`foo2<T, U, V, W, X, Y, Z>(): any {`
`38`	`37`	`throw new Error("Method not implemented.");`
`39`	`38`	`}`
`40`		`-`
`41`	`39`	`foo1(arg0: any, arg1: any, arg2: any): any {`
`42`	`40`	`throw new Error("Method not implemented.");`
`43`	`41`	`}`
`44`		`-`
`45`	`42`	`constructor() {`
`46`	`43`	`this.foo1(1,2,3);`
`47`	`44`	`// 7 type args`
`@@ -60,15 +57,12 @@ verify.codeFix({`
`60`	`57`	`foo3<T0, T1, T2, T3, T4, T5, T6, T7>(): any {`
`61`	`58`	`throw new Error("Method not implemented.");`
`62`	`59`	`}`
`63`		`-`
`64`	`60`	`foo2<T, U, V, W, X, Y, Z>(): any {`
`65`	`61`	`throw new Error("Method not implemented.");`
`66`	`62`	`}`
`67`		`-`
`68`	`63`	`foo1(arg0: any, arg1: any, arg2: any): any {`
`69`	`64`	`throw new Error("Method not implemented.");`
`70`	`65`	`}`
`71`		`-`
`72`	`66`	`constructor() {`
`73`	`67`	`this.foo1(1,2,3);`
`74`	`68`	`// 7 type args`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,6 @@ edit.applyRefactor({`
`16`	`16`	`public set a(value: string) {`
`17`	`17`	`this._a = value;`
`18`	`18`	`}`
`19`		`-`
`20`	`19`	`constructor(private /RENAME/_a: string) { }`
`21`	`20`	}`,
`22`	`21`	`});`