Only allow same-line arrow-less body for explicit nodes

In practice, trying to allow calls to be inlined is causing a lot of code to look very weird and unstable as seen by the four issues this is fixing. It also requires us to add a conditional group and do hackery around it. Fixes prettier#959 Fixes prettier#760 Fixes prettier#615 Fixes prettier#625
vjeux · Mar 9, 2017 · 85fc2ea · 85fc2ea
1 parent dc2fa2c
commit 85fc2ea
Show file tree

Hide file tree

Showing 5 changed files with 253 additions and 49 deletions.
diff --git a/src/printer.js b/src/printer.js
@@ -345,25 +345,18 @@ function genericPrintNoParens(path, options, print) {
         n.body.type === "ArrayExpression" ||
         n.body.type === "ObjectExpression" ||
         n.body.type === "JSXElement" ||
-        n.body.type === "BlockStatement"
+        n.body.type === "BlockStatement" ||
+        n.body.type === "TaggedTemplateExpression" ||
+        n.body.type === "TemplateElement"
       ) {
         return group(collapsed);
       }
 
-      // These nested groups are a little wonky, but because
-      // `conditionalGroup` suppresses break propagation, we want to
-      // re-propagate it. We still want to allow the printer to choose
-      // the more collapsed version, but still break parents if there
-      // are any hard breaks in the content.
       return group(
-        conditionalGroup([
-          collapsed,
-          concat([
-            concat(parts),
-            group(indent(options.tabWidth, concat([line, body])))
-          ])
-        ]),
-        { shouldBreak: willBreak(body) }
+        concat([
+          concat(parts),
+          group(indent(options.tabWidth, concat([line, body])))
+        ])
       );
     case "MethodDefinition":
       if (n.static) {

diff --git a/tests/arrows/__snapshots__/jsfmt.spec.js.snap b/tests/arrows/__snapshots__/jsfmt.spec.js.snap
@@ -71,6 +71,145 @@ a = () => ({} = this);
 "
 `;
 
+exports[`call.js 1`] = `
+"Seq(typeDef.interface.groups).forEach(group =>
+  Seq(group.members).forEach((member, memberName) =>
+    markdownDoc(
+      member.doc,
+      { typePath: typePath.concat(memberName.slice(1)),
+       signatures: member.signatures }
+    )
+  )
+)
+
+const promiseFromCallback = fn =>
+  new Promise((resolve, reject) =>
+    fn((err, result) => {
+      if (err) return reject(err);
+      return resolve(result);
+    })
+  );
+
+runtimeAgent.getProperties(
+  objectId,
+  false, // ownProperties
+  false, // accessorPropertiesOnly
+  false, // generatePreview
+  (error, properties, internalProperties) => {
+    return 1
+  },
+);
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={(e) => this.setState({progress: Math.round(100 * e.nativeEvent.loaded / e.nativeEvent.total)})}
+      />
+    </View>
+  );
+}
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={e =>
+          this.setState({
+            progress: Math.round(
+              100 * e.nativeEvent.loaded / e.nativeEvent.total,
+            ),
+          })}
+      />
+    </View>
+  );
+}
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={e =>
+          this.setState({
+            progress: Math.round(
+              100 * e.nativeEvent.loaded / e.nativeEvent.total,
+            ),
+          })}
+      />
+    </View>
+  );
+}~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Seq(typeDef.interface.groups).forEach(group =>
+  Seq(group.members).forEach((member, memberName) =>
+    markdownDoc(member.doc, {
+      typePath: typePath.concat(memberName.slice(1)),
+      signatures: member.signatures
+    })));
+
+const promiseFromCallback = fn =>
+  new Promise((resolve, reject) =>
+    fn((err, result) => {
+      if (err) return reject(err);
+      return resolve(result);
+    }));
+
+runtimeAgent.getProperties(
+  objectId,
+  false, // ownProperties
+  false, // accessorPropertiesOnly
+  false, // generatePreview
+  (error, properties, internalProperties) => {
+    return 1;
+  }
+);
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={e =>
+          this.setState({
+            progress: Math.round(
+              100 * e.nativeEvent.loaded / e.nativeEvent.total
+            )
+          })}
+      />
+    </View>
+  );
+}
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={e =>
+          this.setState({
+            progress: Math.round(
+              100 * e.nativeEvent.loaded / e.nativeEvent.total
+            )
+          })}
+      />
+    </View>
+  );
+}
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={e =>
+          this.setState({
+            progress: Math.round(
+              100 * e.nativeEvent.loaded / e.nativeEvent.total
+            )
+          })}
+      />
+    </View>
+  );
+}
+"
+`;
+
 exports[`long-call-no-args.js 1`] = `
 "veryLongCall(VERY_VERY_VERY_VERY_VERY_VERY_VERY_VERY_VERY_VERY_LONG_CONSTANT, () => {})
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/tests/arrows/call.js b/tests/arrows/call.js
@@ -0,0 +1,67 @@
+Seq(typeDef.interface.groups).forEach(group =>
+  Seq(group.members).forEach((member, memberName) =>
+    markdownDoc(
+      member.doc,
+      { typePath: typePath.concat(memberName.slice(1)),
+       signatures: member.signatures }
+    )
+  )
+)
+
+const promiseFromCallback = fn =>
+  new Promise((resolve, reject) =>
+    fn((err, result) => {
+      if (err) return reject(err);
+      return resolve(result);
+    })
+  );
+
+runtimeAgent.getProperties(
+  objectId,
+  false, // ownProperties
+  false, // accessorPropertiesOnly
+  false, // generatePreview
+  (error, properties, internalProperties) => {
+    return 1
+  },
+);
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={(e) => this.setState({progress: Math.round(100 * e.nativeEvent.loaded / e.nativeEvent.total)})}
+      />
+    </View>
+  );
+}
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={e =>
+          this.setState({
+            progress: Math.round(
+              100 * e.nativeEvent.loaded / e.nativeEvent.total,
+            ),
+          })}
+      />
+    </View>
+  );
+}
+
+function render() {
+  return (
+    <View>
+      <Image
+        onProgress={e =>
+          this.setState({
+            progress: Math.round(
+              100 * e.nativeEvent.loaded / e.nativeEvent.total,
+            ),
+          })}
+      />
+    </View>
+  );
+}
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -656,12 +656,13 @@ export type BuckWebSocketMessage =
     };
 
 // Missing one level of indentation because of the comment
-const rootEpic = (actions, store) => combineEpics(...epics)(actions, store)
-  // Log errors and continue.
-  .catch((err, stream) => {
-    getLogger().error(err);
-    return stream;
-  });
+const rootEpic = (actions, store) =>
+  combineEpics(...epics)(actions, store)
+    // Log errors and continue.
+    .catch((err, stream) => {
+      getLogger().error(err);
+      return stream;
+    });
 
 // Two extra levels of indentation because of the comment
 export type AsyncExecuteOptions = child_process$execFileOpts & {
@@ -881,12 +882,13 @@ export type BuckWebSocketMessage =
     };
 
 // Missing one level of indentation because of the comment
-const rootEpic = (actions, store) => combineEpics(...epics)(actions, store)
-  // Log errors and continue.
-  .catch((err, stream) => {
-    getLogger().error(err);
-    return stream;
-  });
+const rootEpic = (actions, store) =>
+  combineEpics(...epics)(actions, store)
+    // Log errors and continue.
+    .catch((err, stream) => {
+      getLogger().error(err);
+      return stream;
+    });
 
 // Two extra levels of indentation because of the comment
 export type AsyncExecuteOptions = child_process$execFileOpts & {

diff --git a/tests/method-chain/__snapshots__/jsfmt.spec.js.snap b/tests/method-chain/__snapshots__/jsfmt.spec.js.snap
@@ -39,18 +39,20 @@ it('should group messages with same created time', () => {
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 export default store => {
   return callApi(endpoint, schema).then(
-    response => next(
-      actionWith({
-        response,
-        type: successType
-      })
-    ),
-    error => next(
-      actionWith({
-        type: failureType,
-        error: error.message || \\"Something bad happened\\"
-      })
-    )
+    response =>
+      next(
+        actionWith({
+          response,
+          type: successType
+        })
+      ),
+    error =>
+      next(
+        actionWith({
+          type: failureType,
+          error: error.message || \\"Something bad happened\\"
+        })
+      )
   );
 };
 
@@ -171,18 +173,19 @@ function f() {
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 export default function theFunction(action$, store) {
-  return action$.ofType(THE_ACTION).switchMap(action => Observable.webSocket({
-    url: THE_URL,
-    more: stuff(),
-    evenMore: stuff({
-      value1: true,
-      value2: false,
-      value3: false
+  return action$.ofType(THE_ACTION).switchMap(action =>
+    Observable.webSocket({
+      url: THE_URL,
+      more: stuff(),
+      evenMore: stuff({
+        value1: true,
+        value2: false,
+        value3: false
+      })
     })
-  })
-    .filter(data => theFilter(data))
-    .map(({ theType, ...data }) => theMap(theType, data))
-    .retryWhen(errors => errors));
+      .filter(data => theFilter(data))
+      .map(({ theType, ...data }) => theMap(theType, data))
+      .retryWhen(errors => errors));
 }
 
 function f() {