Repository: jlongster/prettier (js)

Issue:
Strange formatting for "if" with comment on the same line
Input:
```js
// Accept empty response
if (response.clone) { // for fetch api

} else { // for xhr

}
```

Output:
```js
// Accept empty response
if (
  response.clone // for fetch api
) {
} else { // for xhr
}
```

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22%2F%2F%20Accept%20empty%20response%5Cnif%20(response.clone)%20%7B%20%2F%2F%20for%20fetch%20api%5Cn%5Cn%7D%20else%20%7B%20%2F%2F%20for%20xhr%5Cn%5Cn%7D%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

Pull request description:
Always put a hardline before dangling comment
In #563 it looked odd that there was no space before `//`, it turns out that we don't automatically go to the new line for dangling comments. I think that we just should no matter what, so this is what this diff does.

Fixes #563

Patch:
diff --git a/README.md b/README.md
index 39fd19e3f..d408e9786 100644
--- a/README.md
+++ b/README.md
@@ -148,6 +148,10 @@ prettier.format(source, {
   // Controls the printing of spaces inside object literals
   bracketSpacing: true,
 
+  // If true, puts the `>` of a multi-line jsx element at the end of
+  // the last line instead of being alone on the next line
+  jsxBracketSameLine: false,
+
   // Which parser to use. Valid options are 'flow' and 'babylon'
   parser: 'babylon'
 });
diff --git a/bin/prettier.js b/bin/prettier.js
index f7b32acb6..66c516b8d 100755
--- a/bin/prettier.js
+++ b/bin/prettier.js
@@ -16,6 +16,7 @@ const argv = minimist(process.argv.slice(2), {
     "single-quote",
     "trailing-comma",
     "bracket-spacing",
+    "jsx-bracket-same-line",
     // The supports-color package (a sub sub dependency) looks directly at
     // `process.argv` for `--no-color` and such-like options. The reason it is
     // listed here is to avoid "Ignored unknown option: --no-color" warnings.
@@ -59,6 +60,7 @@ if (argv["help"] || !filepatterns.length && !stdin) {
       "  --single-quote           Use single quotes instead of double.\n" +
       "  --trailing-comma         Print trailing commas wherever possible.\n" +
       "  --bracket-spacing        Put spaces between brackets. Defaults to true.\n" +
+      "  --jsx-bracket-same-line  Put > on the last line. Defaults to false.\n" +
       "  --parser <flow|babylon>  Specify which parse to use. Defaults to babylon.\n" +
       "  --color                  Colorize error messages. Defaults to true.\n" +
       "  --version                Print prettier version.\n" +
@@ -125,7 +127,8 @@ const options = {
   bracketSpacing: argv["bracket-spacing"],
   parser: getParserOption(),
   singleQuote: argv["single-quote"],
-  trailingComma: argv["trailing-comma"]
+  trailingComma: argv["trailing-comma"],
+  jsxBracketSameLine: argv["jsx-bracket-same-line"],
 };
 
 function format(input) {
diff --git a/docs/index.html b/docs/index.html
index 90fb6e6e6..1bbb9d15f 100644
--- a/docs/index.html
+++ b/docs/index.html
@@ -91,6 +91,7 @@
     <label><input type="checkbox" id="singleQuote"></input> singleQuote</label>
     <label><input type="checkbox" id="trailingComma"></input> trailingComma</label>
     <label><input type="checkbox" id="bracketSpacing" checked></input> bracketSpacing</label>
+    <label><input type="checkbox" id="jsxBracketSameLine"></input> jsxBracketSameLine</label>
     <span style="flex: 1"></span>
     <label><input type="checkbox" id="doc"></input> doc</label>
   </div>
@@ -109,7 +110,7 @@
 </div>
 
 <script id="code">
-var OPTIONS = ['printWidth', 'tabWidth', 'singleQuote', 'trailingComma', 'bracketSpacing', 'doc'];
+var OPTIONS = ['printWidth', 'tabWidth', 'singleQuote', 'trailingComma', 'bracketSpacing', 'jsxBracketSameLine', 'doc'];
 function setOptions(options) {
   OPTIONS.forEach(function(option) {
     var elem = document.getElementById(option);
diff --git a/src/comments.js b/src/comments.js
index 59e12754c..cf6c2d7e7 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -10,6 +10,7 @@ var hardline = docBuilders.hardline;
 var breakParent = docBuilders.breakParent;
 var indent = docBuilders.indent;
 var lineSuffix = docBuilders.lineSuffix;
+var join = docBuilders.join;
 var util = require("./util");
 var comparePos = util.comparePos;
 var childNodesCacheKey = Symbol("child-nodes");
@@ -450,7 +451,7 @@ function printTrailingComment(commentPath, print, options, parentNode) {
   return concat([lineSuffix(" " + contents), !isBlock ? breakParent : ""]);
 }
 
-function printDanglingComments(path, options, noIndent) {
+function printDanglingComments(path, options, sameIndent) {
   const text = options.originalText;
   const parts = [];
   const node = path.getValue();
@@ -463,19 +464,20 @@ function printDanglingComments(path, options, noIndent) {
     commentPath => {
       const comment = commentPath.getValue();
       if (!comment.leading && !comment.trailing) {
-        if (util.hasNewline(text, locStart(comment), { backwards: true })) {
-          parts.push(hardline);
-        }
         parts.push(printComment(commentPath));
       }
     },
     "comments"
   );
 
-  if (!noIndent) {
-    return indent(options.tabWidth, concat(parts));
+  if (parts.length === 0) {
+    return "";
+  }
+
+  if (sameIndent) {
+    return join(hardline, parts);
   }
-  return concat(parts);
+  return indent(options.tabWidth, concat([hardline, join(hardline, parts)]));
 }
 
 function printComments(path, print, options) {
diff --git a/src/options.js b/src/options.js
index e696e7eed..73fc4191d 100644
--- a/src/options.js
+++ b/src/options.js
@@ -4,17 +4,12 @@ var validate = require("jest-validate").validate;
 var deprecatedConfig = require("./deprecated");
 
 var defaults = {
-  // Number of spaces the pretty-printer should use per tab
   tabWidth: 2,
-  // Fit code within this line limit
   printWidth: 80,
-  // If true, will use single instead of double quotes
   singleQuote: false,
-  // Controls the printing of trailing commas wherever possible
   trailingComma: false,
-  // Controls the printing of spaces inside array and objects
   bracketSpacing: true,
-  // Which parser to use. Valid options are 'flow' and 'babylon'
+  jsxBracketSameLine: false,
   parser: "babylon"
 };
 
diff --git a/src/printer.js b/src/printer.js
index 183911228..a9fc2fb2e 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -141,7 +141,7 @@ function genericPrintNoParens(path, options, print) {
       );
 
       parts.push(
-        comments.printDanglingComments(path, options, /* noIdent */ true)
+        comments.printDanglingComments(path, options, /* sameIndent */ true)
       );
 
       // Only force a trailing newline if there were any contents.
@@ -1219,9 +1219,9 @@ function genericPrintNoParens(path, options, print) {
                 path.map(attr => concat([line, print(attr)]), "attributes")
               )
             ),
-            n.selfClosing ? line : softline
+            n.selfClosing ? line : (options.jsxBracketSameLine ? ">" : softline)
           ]),
-          n.selfClosing ? "/>" : ">"
+          n.selfClosing ? "/>" : (options.jsxBracketSameLine ? "" : ">")
         ])
       );
     }
@@ -1230,7 +1230,7 @@ function genericPrintNoParens(path, options, print) {
     case "JSXText":
       throw new Error("JSXTest should be handled by JSXElement");
     case "JSXEmptyExpression":
-      return concat([comments.printDanglingComments(path, options), softline]);
+      return concat([comments.printDanglingComments(path, options, /* sameIndent */ true), softline]);
     case "TypeAnnotatedIdentifier":
       return concat([
         path.call(print, "annotation"),


Test patch:
diff --git a/tests/classes/__snapshots__/jsfmt.spec.js.snap b/tests/classes/__snapshots__/jsfmt.spec.js.snap
index 278feace6..c6b931fe3 100644
--- a/tests/classes/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/classes/__snapshots__/jsfmt.spec.js.snap
@@ -46,7 +46,8 @@ class A {
   // comment
 }
 
-class A {// comment
+class A {
+  // comment
 }
 
 class A {}
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index 246208c14..bb4262aab 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -39,15 +39,21 @@ function x() {
 }
 declare class Foo extends Qux<string> {/* dangling */}
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-var x = {/* dangling */};
+var x = {
+  /* dangling */
+};
 var x = {
   // dangling
 };
-var x = [/* dangling */];
+var x = [
+  /* dangling */
+];
 function x() {
   /* dangling */
 }
-declare class Foo extends Qux<string> {/* dangling */}
+declare class Foo extends Qux<string> {
+  /* dangling */
+}
 "
 `;
 
@@ -62,15 +68,21 @@ function x() {
 }
 declare class Foo extends Qux<string> {/* dangling */}
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-var x = {/* dangling */};
+var x = {
+  /* dangling */
+};
 var x = {
   // dangling
 };
-var x = [/* dangling */];
+var x = [
+  /* dangling */
+];
 function x() {
   /* dangling */
 }
-declare class Foo extends Qux<string> {/* dangling */}
+declare class Foo extends Qux<string> {
+  /* dangling */
+}
 "
 `;
 
@@ -701,9 +713,7 @@ exports[`test jsx.js 1`] = `
 </div>;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 <div>
-  {
-    /* comment */
-  }
+  {/* comment */}
 </div>;
 
 <div>
@@ -779,9 +789,7 @@ exports[`test jsx.js 2`] = `
 </div>;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 <div>
-  {
-    /* comment */
-  }
+  {/* comment */}
 </div>;
 
 <div>
diff --git a/tests/flow/more_path/__snapshots__/jsfmt.spec.js.snap b/tests/flow/more_path/__snapshots__/jsfmt.spec.js.snap
index ff4be3fdf..3e7301687 100644
--- a/tests/flow/more_path/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/more_path/__snapshots__/jsfmt.spec.js.snap
@@ -101,7 +101,8 @@ function goofy() {
   var x = g();
   if (typeof x == \"function\") {
     x();
-  } else {// if (typeof x == \'number\') {
+  } else {
+    // if (typeof x == \'number\') {
     //f(x);
   }
 }
diff --git a/tests/flow/new_react/__snapshots__/jsfmt.spec.js.snap b/tests/flow/new_react/__snapshots__/jsfmt.spec.js.snap
index ceee6a567..9af52af8d 100644
--- a/tests/flow/new_react/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/new_react/__snapshots__/jsfmt.spec.js.snap
@@ -179,10 +179,12 @@ type T1 = {};
 type T2 = { x: number };
 type T3 = { x: number, y: number };
 
-class C1 extends React.Component<T1, T2, any> {// error
+class C1 extends React.Component<T1, T2, any> {
+  // error
 }
 
-class C2 extends React.Component<void, T2, any> {// OK
+class C2 extends React.Component<void, T2, any> {
+  // OK
 }
 
 // no need to add type arguments to React.Component
@@ -198,7 +200,8 @@ class C4 extends React.Component {
   props: T2;
 }
 
-class C5 extends React.Component<T2, T3, any> {// error
+class C5 extends React.Component<T2, T3, any> {
+  // error
 }
 
 class C6 extends React.Component {
diff --git a/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap b/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap
index e569f13dd..9fffbbf19 100644
--- a/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/refinements/__snapshots__/jsfmt.spec.js.snap
@@ -1394,11 +1394,13 @@ let tests = [
     }
   },
   function(num: number, obj: { foo: number }) {
-    if (num === obj.bar) {// ok, typos allowed in conditionals
+    if (num === obj.bar) {
+      // ok, typos allowed in conditionals
     }
   },
   function(num: number, obj: { [key: string]: number }) {
-    if (num === obj.bar) {// ok
+    if (num === obj.bar) {
+      // ok
     }
   },
   function(n: number): Mode {
@@ -1974,11 +1976,13 @@ let tests = [
     }
   },
   function(str: string, obj: { foo: string }) {
-    if (str === obj.bar) {// ok, typos allowed in conditionals
+    if (str === obj.bar) {
+      // ok, typos allowed in conditionals
     }
   },
   function(str: string, obj: { [key: string]: string }) {
-    if (str === obj.bar) {// ok
+    if (str === obj.bar) {
+      // ok
     }
   },
   function(str: string): Mode {
diff --git a/tests/flow/try/__snapshots__/jsfmt.spec.js.snap b/tests/flow/try/__snapshots__/jsfmt.spec.js.snap
index fad94e973..6ab2f3e38 100644
--- a/tests/flow/try/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/try/__snapshots__/jsfmt.spec.js.snap
@@ -554,7 +554,8 @@ function bar(response) {
     throw new Error(\"...\");
   }
   // here via [try] only.
-  if (payload.error) {// ok
+  if (payload.error) {
+    // ok
     // ...
   }
 }
diff --git a/tests/jsx_last_line/__snapshots__/jsfmt.spec.js.snap b/tests/jsx_last_line/__snapshots__/jsfmt.spec.js.snap
new file mode 100644
index 000000000..0d19c4878
--- /dev/null
+++ b/tests/jsx_last_line/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,48 @@
+exports[`test last_line.js 1`] = `
+"<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>;
+"
+`;
+
+exports[`test last_line.js 2`] = `
+"<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>;
+"
+`;
diff --git a/tests/jsx_last_line/jsfmt.spec.js b/tests/jsx_last_line/jsfmt.spec.js
new file mode 100644
index 000000000..0cac44995
--- /dev/null
+++ b/tests/jsx_last_line/jsfmt.spec.js
@@ -0,0 +1,2 @@
+run_spec(__dirname, {jsxBracketSameLine: true});
+run_spec(__dirname, {jsxBracketSameLine: false});
diff --git a/tests/jsx_last_line/last_line.js b/tests/jsx_last_line/last_line.js
new file mode 100644
index 000000000..2417fa94b
--- /dev/null
+++ b/tests/jsx_last_line/last_line.js
@@ -0,0 +1,10 @@
+<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>