Repository: jlongster/prettier (js)

Issue:
Unstable comment – between blocks?
```js
try {}
// comment
finally {}
```

becomes ([online demo](https://jlongster.github.io/prettier/#%7B%22content%22%3A%22try%E2%80%86%7B%7D%5Cn%2F%2F%20comment%5Cnfinally%20%7B%7D%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D))

```js
try {
} finally // comment
{
}
```

becomes ([online demo](https://jlongster.github.io/prettier/#%7B%22content%22%3A%22try%20%7B%5Cn%7D%20finally%20%2F%2F%20comment%5Cn%7B%5Cn%7D%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D))

```js
try {
} finally { // comment
}
```

Found using [eslump](https://github.com/lydell/eslump). I also tried this with all of @vjeux’s PRs merged.


Pull request description:
Better support try/catch comments
Uses the same technique as for if/then/else. Added a helper function to make it easier.

Fixes #601

Patch:
diff --git a/src/comments.js b/src/comments.js
index bd5c25d7f..67d40de30 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -145,7 +145,8 @@ function attach(comments, ast, text) {
       // We also need to check if it's the first line of the file.
       if (
         handleMemberExpressionComment(enclosingNode, followingNode, comment) ||
-          handleIfStatementComments(enclosingNode, followingNode, comment)
+          handleIfStatementComments(enclosingNode, followingNode, comment) ||
+          handleTryStatementComments(enclosingNode, followingNode, comment)
       ) {
         // We're good
       } else if (followingNode) {
@@ -286,6 +287,14 @@ function addBlockStatementFirstComment(node, comment) {
   }
 }
 
+function addBlockOrNotComment(node, comment) {
+  if (node.type === "BlockStatement") {
+    addBlockStatementFirstComment(node, comment);
+  } else {
+    addLeadingComment(node, comment);
+  }
+}
+
 // There are often comments before the else clause of if statements like
 //
 //   if (1) { ... }
@@ -315,11 +324,33 @@ function handleIfStatementComments(enclosingNode, followingNode, comment) {
   }
 
   if (followingNode.type === "IfStatement") {
-    if (followingNode.consequent.type === "BlockStatement") {
-      addBlockStatementFirstComment(followingNode.consequent, comment);
-    } else {
-      addLeadingComment(followingNode.consequent, comment);
-    }
+    addBlockOrNotComment(followingNode.consequent, comment);
+    return true;
+  }
+
+  return false;
+}
+
+// Same as IfStatement but for TryStatement
+function handleTryStatementComments(enclosingNode, followingNode, comment) {
+  if (
+    !enclosingNode || enclosingNode.type !== "TryStatement" || !followingNode
+  ) {
+    return false;
+  }
+
+  if (followingNode.type === "BlockStatement") {
+    addBlockStatementFirstComment(followingNode, comment);
+    return true;
+  }
+
+  if (followingNode.type === "TryStatement") {
+    addBlockOrNotComment(followingNode.finalizer, comment);
+    return true;
+  }
+
+  if (followingNode.type === "CatchClause") {
+    addBlockOrNotComment(followingNode.body, comment);
     return true;
   }
 
diff --git a/src/printer.js b/src/printer.js
index 6eff3025c..174c3beae 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -1377,9 +1377,15 @@ function genericPrintNoParens(path, options, print) {
         namedTypes.ObjectTypeProperty.check(parent) ||
         namedTypes.ObjectTypeCallProperty.check(parent) ||
         namedTypes.DeclareFunction.check(path.getParentNode(2)));
+
       var needsColon = isArrowFunctionTypeAnnotation &&
         namedTypes.TypeAnnotation.check(parent);
 
+      if (isObjectTypePropertyAFunction(parent)) {
+        isArrowFunctionTypeAnnotation = true;
+        needsColon = true;
+      }
+
       if (needsColon) {
         parts.push(": ");
       }
@@ -1499,6 +1505,11 @@ function genericPrintNoParens(path, options, print) {
       var isFunction = !n.variance &&
         !n.optional &&
         n.value.type === "FunctionTypeAnnotation";
+
+      if (isObjectTypePropertyAFunction(n)) {
+        isFunction = true;
+      }
+
       return concat([
         n.static ? "static " : "",
         variance,
@@ -2673,6 +2684,16 @@ function isLastStatement(path) {
   return body && body[body.length - 1] === node;
 }
 
+// Hack to differentiate between the following two which have the same ast
+// type T = { method: () => void };
+// type T = { method(): void };
+function isObjectTypePropertyAFunction(node) {
+  return node.type === "ObjectTypeProperty" &&
+    node.value.type === "FunctionTypeAnnotation" &&
+    !node.static &&
+    util.locStart(node.key) !== util.locStart(node.value);
+}
+
 function shouldPrintSameLine(node) {
   const type = node.type;
   return namedTypes.Literal.check(node) ||


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index 4627eed57..06a16a1fc 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -747,3 +747,59 @@ exports[`test jsx.js 2`] = `
 </div>;
 "
 `;
+
+exports[`test try.js 1`] = `
+"// comment 1
+try {
+  // comment 2
+}
+// comment 3
+catch(e) {
+  // comment 4
+}
+// comment 5
+finally // comment 6
+{
+  // comment 7
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// comment 1
+try {
+  // comment 2
+} catch (e) {
+  // comment 3
+  // comment 4
+} finally { // comment 6
+  // comment 5
+  // comment 7
+}
+"
+`;
+
+exports[`test try.js 2`] = `
+"// comment 1
+try {
+  // comment 2
+}
+// comment 3
+catch(e) {
+  // comment 4
+}
+// comment 5
+finally // comment 6
+{
+  // comment 7
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// comment 1
+try {
+  // comment 2
+} catch (e) {
+  // comment 3
+  // comment 4
+} finally { // comment 6
+  // comment 5
+  // comment 7
+}
+"
+`;
diff --git a/tests/comments/try.js b/tests/comments/try.js
new file mode 100644
index 000000000..1c9205765
--- /dev/null
+++ b/tests/comments/try.js
@@ -0,0 +1,13 @@
+// comment 1
+try {
+  // comment 2
+}
+// comment 3
+catch(e) {
+  // comment 4
+}
+// comment 5
+finally // comment 6
+{
+  // comment 7
+}
diff --git a/tests/flow/constructor_annots/__snapshots__/jsfmt.spec.js.snap b/tests/flow/constructor_annots/__snapshots__/jsfmt.spec.js.snap
index e3ff68427..d1f66e222 100644
--- a/tests/flow/constructor_annots/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/constructor_annots/__snapshots__/jsfmt.spec.js.snap
@@ -38,7 +38,7 @@ exports.Foo = Foo;
 
 // so you want to type Foo, by declaring it as a class
 interface IFooPrototype {
-  m(): number
+  m: () => number
 }
 interface IFoo extends IFooPrototype {
   static (): void,
diff --git a/tests/flow/object-method/__snapshots__/jsfmt.spec.js.snap b/tests/flow/object-method/__snapshots__/jsfmt.spec.js.snap
index ba57ea5cc..13d76e253 100644
--- a/tests/flow/object-method/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/object-method/__snapshots__/jsfmt.spec.js.snap
@@ -129,7 +129,7 @@ function bar(f: () => void) {
 
 bar(foo); // error, since \`this\` is used non-trivially in \`foo\`
 
-function qux(o: { f(): void }) {
+function qux(o: { f: () => void }) {
   o.f(); // passing o as \`this\`
 }
 
diff --git a/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap b/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap
index 5d8d537ba..e569f13dd 100644
--- a/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap
@@ -43,7 +43,7 @@ function foo(x: ?number) {
 
 type Bar = {
   parent: ?Bar,
-  doStuff(): void
+  doStuff: () => void
 };
 
 function bar0(x: Bar) {
diff --git a/tests/flow/taint/__snapshots__/jsfmt.spec.js.snap b/tests/flow/taint/__snapshots__/jsfmt.spec.js.snap
index 856977b2f..5a9b44db6 100644
--- a/tests/flow/taint/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/taint/__snapshots__/jsfmt.spec.js.snap
@@ -151,13 +151,13 @@ function foo(x: $Tainted<string>, o: Object) {
   // Error
   o.f(x);
 }
-function foo1(x: $Tainted<string>, o: { f(y: $Tainted<string>): void }) {
+function foo1(x: $Tainted<string>, o: { f: (y: $Tainted<string>) => void }) {
   o.f(x);
 }
 function foo2(o1: Object, o2: { t: $Tainted<string> }) {
   o1.f(o2.t);
 }
-function foo3<T>(x: $Tainted<T>, o: { f(y: $Tainted<T>): void }) {
+function foo3<T>(x: $Tainted<T>, o: { f: (y: $Tainted<T>) => void }) {
   o.f(x);
 }
 function f_foo1(x: $Tainted<string>, f: Function) {
diff --git a/tests/flow/type_only_vars/__snapshots__/jsfmt.spec.js.snap b/tests/flow/type_only_vars/__snapshots__/jsfmt.spec.js.snap
index 119c8288b..bedfebb27 100644
--- a/tests/flow/type_only_vars/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/type_only_vars/__snapshots__/jsfmt.spec.js.snap
@@ -57,7 +57,7 @@ import typeof A from \"./A.js\";
 import type { Foo, Bar as Baz } from \"./A.js\";
 
 type duck = {
-  quack(): string
+  quack: () => string
 };
 
 // These string types should confict with the imported types
diff --git a/tests/flow_method/__snapshots__/jsfmt.spec.js.snap b/tests/flow_method/__snapshots__/jsfmt.spec.js.snap
new file mode 100644
index 000000000..59d94ddce
--- /dev/null
+++ b/tests/flow_method/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,14 @@
+exports[`test method.js 1`] = `
+"type T = { method: () => void };
+type T = { method(): void };
+declare class X { method(): void }
+declare function f(): void;
+var f: () => void;
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+type T = { method: () => void };
+type T = { method(): void };
+declare class X { method(): void }
+declare function f(): void;
+var f: () => void;
+"
+`;
diff --git a/tests/flow_method/jsfmt.spec.js b/tests/flow_method/jsfmt.spec.js
new file mode 100644
index 000000000..989047bcc
--- /dev/null
+++ b/tests/flow_method/jsfmt.spec.js
@@ -0,0 +1,1 @@
+run_spec(__dirname);
diff --git a/tests/flow_method/method.js b/tests/flow_method/method.js
new file mode 100644
index 000000000..405f3fe2b
--- /dev/null
+++ b/tests/flow_method/method.js
@@ -0,0 +1,5 @@
+type T = { method: () => void };
+type T = { method(): void };
+declare class X { method(): void }
+declare function f(): void;
+var f: () => void;