Improve visual separation between header and body in classes with multiline headers (#10085)

* Add hardline for a class whose extends part is broken

refactor

Use groupId

Remove isPreviousLineEmpty

* Avoid leading comments

* Rename to heritage

* Add heuristic for implements

* Fix lint problems

* Fix

Fix

Fix

* Extract getGroupId patterns

* Adopots solution 2

* Fix

* Fix lint problem

* Add changelog

* Address reviews

* Update tests

* Rename to createGroupIdMapper

* Split files

* Add tests for complex extends

* Fix changelog

changelog_unreleased/javascript/10085.md

@@ -0,0 +1,28 @@
+#### Improve visual separation between header and body in classes with multiline headers (#10085 by @sosukesuzuki)
+
+<!-- prettier-ignore -->
+```ts
+// Input
+class loooooooooooooooooooong
+  extends looooooooooooooooooong
+  implements loooooooooooooooooooong
+{
+  property: string;
+}
+
+// Prettier stable
+class loooooooooooooooooooong
+  extends looooooooooooooooooong
+  implements loooooooooooooooooooong {
+  property: string;
+}
+
+// Prettier master
+class loooooooooooooooooooong
+  extends looooooooooooooooooong
+  implements loooooooooooooooooooong
+{
+  property: string;
+}
+
+```

src/common/util.js

@@ -628,6 +628,20 @@ function isNonEmptyArray(object) {
   return Array.isArray(object) && object.length > 0;
 }
 
+/**
+ * @param {string} description
+ * @returns {(node: any) => symbol}
+ */
+function createGroupIdMapper(description) {
+  const groupIds = new WeakMap();
+  return function (node) {
+    if (!groupIds.has(node)) {
+      groupIds.set(node, Symbol(description));
+    }
+    return groupIds.get(node);
+  };
+}
+
 module.exports = {
   inferParserByLanguage,
   replaceEndOfLineWith,
@@ -665,4 +679,5 @@ module.exports = {
   isFrontMatterNode,
   getShebang,
   isNonEmptyArray,
+  createGroupIdMapper,
 };

src/language-js/print/block.js

@@ -6,6 +6,7 @@ const {
   builders: { hardline, indent },
 } = require("../../document");
 const { hasComment, CommentCheckFlags, isNextLineEmpty } = require("../utils");
+const { printHardlineAfterHeritage } = require("./class");
 
 const { printBody } = require("./statement");
 
@@ -19,6 +20,11 @@ function printBlock(path, options, print) {
     parts.push("static ");
   }
 
+  if (node.type === "ClassBody" && isNonEmptyArray(node.body)) {
+    const parent = path.getParentNode();
+    parts.push(printHardlineAfterHeritage(parent));
+  }
+
   parts.push("{");
   const printed = printBlockBody(path, options, print);
   if (printed) {

src/language-js/print/class.js

@@ -1,6 +1,6 @@
 "use strict";
 
-const { isNonEmptyArray } = require("../../common/util");
+const { isNonEmptyArray, createGroupIdMapper } = require("../../common/util");
 const { printComments, printDanglingComments } = require("../../main/comments");
 const {
   builders: { join, line, hardline, softline, group, indent, ifBreak },
@@ -72,15 +72,16 @@ function printClass(path, options, print) {
 
   if (groupMode) {
     const printedExtends = extendsParts;
+    let printedPartsGroup;
     if (shouldIndentOnlyHeritageClauses(n)) {
-      parts.push(
-        group(
-          partsGroup.concat(ifBreak(indent(printedExtends), printedExtends))
-        )
-      );
+      printedPartsGroup = [
+        ...partsGroup,
+        ifBreak(indent(printedExtends), printedExtends),
+      ];
     } else {
-      parts.push(group(indent(partsGroup.concat(printedExtends))));
+      printedPartsGroup = indent([...partsGroup, printedExtends]);
     }
+    parts.push(group(printedPartsGroup, { id: getHeritageGroupId(n) }));
   } else {
     parts.push(...partsGroup, ...extendsParts);
   }
@@ -90,6 +91,12 @@ function printClass(path, options, print) {
   return parts;
 }
 
+const getHeritageGroupId = createGroupIdMapper("heritageGroup");
+
+function printHardlineAfterHeritage(node) {
+  return ifBreak(hardline, "", { groupId: getHeritageGroupId(node) });
+}
+
 function hasMultipleHeritage(node) {
   return (
     ["superClass", "extends", "mixins", "implements"].filter(
@@ -222,4 +229,5 @@ module.exports = {
   printClass,
   printClassMethod,
   printClassProperty,
+  printHardlineAfterHeritage,
 };

src/language-js/print/object.js

@@ -4,7 +4,12 @@ const { printDanglingComments } = require("../../main/comments");
 const {
   builders: { line, softline, group, indent, ifBreak, hardline },
 } = require("../../document");
-const { getLast, hasNewlineInRange, hasNewline } = require("../../common/util");
+const {
+  getLast,
+  hasNewlineInRange,
+  hasNewline,
+  isNonEmptyArray,
+} = require("../../common/util");
 const {
   shouldPrintComma,
   hasComment,
@@ -17,6 +22,7 @@ const { locStart, locEnd } = require("../loc");
 const { printOptionalToken, printTypeAnnotation } = require("./misc");
 const { shouldHugFunctionParameters } = require("./function-parameters");
 const { shouldHugType } = require("./type-annotation");
+const { printHardlineAfterHeritage } = require("./class");
 
 /** @typedef {import("../../document").Doc} Doc */
 
@@ -175,6 +181,9 @@ function printObject(path, options, print) {
     ]);
   } else {
     content = [
+      isFlowInterfaceLikeBody && isNonEmptyArray(n.properties)
+        ? printHardlineAfterHeritage(parent)
+        : "",
       leftBrace,
       indent([options.bracketSpacing ? line : softline, ...props]),
       ifBreak(

src/language-js/print/type-parameters.js

@@ -12,15 +12,10 @@ const {
   shouldPrintComma,
   getFunctionParameters,
 } = require("../utils");
+const { createGroupIdMapper } = require("../../common/util");
 const { shouldHugType } = require("./type-annotation");
 
-const typeParametersGroupIds = new WeakMap();
-function getTypeParametersGroupId(node) {
-  if (!typeParametersGroupIds.has(node)) {
-    typeParametersGroupIds.set(node, Symbol("typeParameters"));
-  }
-  return typeParametersGroupIds.get(node);
-}
+const getTypeParametersGroupId = createGroupIdMapper("typeParameters");
 
 function printTypeParameters(path, options, print, paramsKey) {
   const n = path.getValue();

tests/js/class-comment/__snapshots__/jsfmt.spec.js.snap

@@ -146,7 +146,8 @@ class c extends d {
 }
 
 class C2 // comment
-  extends Base {
+  extends Base
+{
   foo() {}
 }
 

tests/js/class-extends/__snapshots__/jsfmt.spec.js.snap

@@ -1,5 +1,53 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`complex.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+class loooooooooooooooooooong1 extends foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo()))))))) {}
+
+class loooooooooooooooooooong2 extends function (make, model, year, owner) {
+  this.make = make;
+  this.model = model;
+  this.year = year;
+  this.owner = owner;
+} {}
+
+class loooooooooooooooooooong3 extends class {
+  cconstructor(make, model, year, owner) {
+    this.make = make;
+    this.model = model;
+    this.year = year;
+    this.owner = owner;
+  }
+} {}
+
+=====================================output=====================================
+class loooooooooooooooooooong1 extends foooooooo(
+  foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo()))))))
+) {}
+
+class loooooooooooooooooooong2 extends function (make, model, year, owner) {
+  this.make = make;
+  this.model = model;
+  this.year = year;
+  this.owner = owner;
+} {}
+
+class loooooooooooooooooooong3 extends class {
+  cconstructor(make, model, year, owner) {
+    this.make = make;
+    this.model = model;
+    this.year = year;
+    this.owner = owner;
+  }
+} {}
+
+================================================================================
+`;
+
 exports[`extends.js format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow", "typescript"]

tests/js/class-extends/complex.js

@@ -0,0 +1,17 @@
+class loooooooooooooooooooong1 extends foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo(foooooooo()))))))) {}
+
+class loooooooooooooooooooong2 extends function (make, model, year, owner) {
+  this.make = make;
+  this.model = model;
+  this.year = year;
+  this.owner = owner;
+} {}
+
+class loooooooooooooooooooong3 extends class {
+  cconstructor(make, model, year, owner) {
+    this.make = make;
+    this.model = model;
+    this.year = year;
+    this.owner = owner;
+  }
+} {}

tests/typescript/class-comment/__snapshots__/jsfmt.spec.js.snap

@@ -83,7 +83,8 @@ b // comment-b2
 =====================================output=====================================
 class a1
   extends b // comment
-  implements z {
+  implements z
+{
   constructor() {}
 }
 
@@ -95,7 +96,8 @@ class a2 extends b implements z {
 class a3
   extends b
   // comment
-  implements z, y {
+  implements z, y
+{
   constructor() {}
 }
 
@@ -103,7 +105,8 @@ class a4
   extends b
   implements
     z, // comment
-    y {
+    y
+{
   constructor() {}
 }
 
@@ -111,7 +114,8 @@ class a5
   extends b
   implements
     z, // comment-z
-    y {
+    y
+{
   // comment-y
   constructor() {}
 }
@@ -122,7 +126,8 @@ class a6
   implements
     z, // comment-z2
     // comment-y1
-    y {
+    y
+{
   // comment-y2
   constructor() {}
 }
@@ -133,7 +138,8 @@ class a7
   implements
     z, // comment-z2
     // comment-y1
-    y {
+    y
+{
   // comment-y2
   // comment-y3
   //comment-body
@@ -146,7 +152,8 @@ class a8
   implements
     z, // comment-z2
     // comment-y1
-    y {
+    y
+{
   // comment-y2
   constructor() {}
 }
@@ -159,7 +166,8 @@ class a9
   implements
     z, // comment-z2
     // comment-y1
-    y {
+    y
+{
   // comment-y2
   constructor() {}
 }
@@ -188,7 +196,8 @@ extends B<T> // 3
 =====================================output=====================================
 declare class a // 1
   // extends b   // 2
-  implements z, x {
+  implements z, x
+{
   // 3
   doo: boolean;
 }
@@ -233,13 +242,15 @@ class G1<T> implements IPoly<T> {
 }
 
 class G2<T> // g2
-  implements IPoly<T> {
+  implements IPoly<T>
+{
   x: T;
 }
 
 class G3<T> // g3
   extends U
-  implements IPoly<T> {
+  implements IPoly<T>
+{
   x: T;
 }
 
@@ -247,7 +258,8 @@ class G4<
     T // g4
   >
   extends U
-  implements IPoly<T> {
+  implements IPoly<T>
+{
   x: T;
 }
 

tests/typescript/classes/__snapshots__/jsfmt.spec.js.snap

@@ -45,7 +45,8 @@ export class VisTimelineComponent2
 =====================================output=====================================
 class MyContractSelectionWidget
   extends React.Component<void, MyContractSelectionWidgetPropsType, void>
-  implements SomethingLarge {
+  implements SomethingLarge
+{
   method() {}
 }
 
@@ -89,3 +90,63 @@ export class VisTimelineComponent2
 
 ================================================================================
 `;
+
+exports[`break-heritage.ts format 1`] = `
+====================================options=====================================
+parsers: ["typescript", "babel", "flow"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong {
+  // leading comment
+  property: string;
+}
+
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong {
+  property: string;
+}
+
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong {
+
+  property: string;
+}
+
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong, loooooooooooooooooooong, loooooooooooooooooooong {
+  property: string;
+}
+
+=====================================output=====================================
+class loooooooooooooooooooong
+  extends looooooooooooooooooong
+  implements loooooooooooooooooooong
+{
+  // leading comment
+  property: string;
+}
+
+class loooooooooooooooooooong
+  extends looooooooooooooooooong
+  implements loooooooooooooooooooong
+{
+  property: string;
+}
+
+class loooooooooooooooooooong
+  extends looooooooooooooooooong
+  implements loooooooooooooooooooong
+{
+  property: string;
+}
+
+class loooooooooooooooooooong
+  extends looooooooooooooooooong
+  implements
+    loooooooooooooooooooong,
+    loooooooooooooooooooong,
+    loooooooooooooooooooong
+{
+  property: string;
+}
+
+================================================================================
+`;

tests/typescript/classes/break-heritage.ts

@@ -0,0 +1,17 @@
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong {
+  // leading comment
+  property: string;
+}
+
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong {
+  property: string;
+}
+
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong {
+
+  property: string;
+}
+
+class loooooooooooooooooooong extends looooooooooooooooooong implements loooooooooooooooooooong, loooooooooooooooooooong, loooooooooooooooooooong {
+  property: string;
+}