Repository: jlongster/prettier (js)

Issue:
Make `trailingComma` option more granular
This code:

```js
a(b.map(function(c) { return c; }));
```

is formatted like this:

```js
a(
  b.map(function(c) {
    return c;
  }),
);
```

with extra comma in a function call.

Demo: https://jlongster.github.io/prettier/#%7B%22content%22%3A%22a(b.map(function(c)%20%7B%20return%20c%3B%20%7D))%3B%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Atrue%2C%22trailingComma%22%3Atrue%2C%22bracketSpacing%22%3Atrue%7D%7D

Pull request description:
Make trailing-comma option support 2 different modes
#591 wants to remove support for trailing commas in function args. Let's just go ahead and make the option support multiple values and default to no commas in function args, but you can turn on commas everywhere with `--trailing-comma=all`. 

I wasn't sure what to name the `compat` level. The idea is it's a mode that's compatible with older JS environments. If you pass `--trailing-comma` it defaults to `compat` so users shouldn't need to explicitly type out that option.

Fixes #266

Patch:
diff --git a/README.md b/README.md
index c4ab30824..fd6c0b4a1 100644
--- a/README.md
+++ b/README.md
@@ -131,7 +131,7 @@ In the future we will have better support for formatting whole projects.
 Install it along with [pre-commit](https://github.com/observing/pre-commit) (or [husky](https://github.com/typicode/husky)):
 
 ```bash
-npm install --save-dev lint-staged pre-commit
+yarn add lint-staged pre-commit --dev
 ```
 
 and add this config to your `package.json`:
diff --git a/bin/prettier.js b/bin/prettier.js
index 7094b5af1..b91d20003 100755
--- a/bin/prettier.js
+++ b/bin/prettier.js
@@ -15,7 +15,6 @@ const argv = minimist(process.argv.slice(2), {
     "write",
     "stdin",
     "single-quote",
-    "trailing-comma",
     "bracket-spacing",
     "jsx-bracket-same-line",
     // The supports-color package (a sub sub dependency) looks directly at
@@ -30,7 +29,7 @@ const argv = minimist(process.argv.slice(2), {
     // Deprecated in 0.0.10
     "flow-parser"
   ],
-  string: ["print-width", "tab-width", "parser"],
+  string: ["print-width", "tab-width", "parser", "trailing-comma"],
   default: { color: true, "bracket-spacing": true, parser: "babylon" },
   alias: { help: "h", version: "v" },
   unknown: param => {
@@ -50,29 +49,6 @@ const filepatterns = argv["_"];
 const write = argv["write"];
 const stdin = argv["stdin"] || !filepatterns.length && !process.stdin.isTTY;
 
-if (argv["help"] || !filepatterns.length && !stdin) {
-  console.log(
-    "Usage: prettier [opts] [filename ...]\n\n" +
-      "Available options:\n" +
-      "  --write                  Edit the file in-place. (Beware!)\n" +
-      "  --stdin                  Read input from stdin.\n" +
-      "  --print-width <int>      Specify the length of line that the printer will wrap on. Defaults to 80.\n" +
-      "  --tab-width <int>        Specify the number of spaces per indentation-level. Defaults to 2.\n" +
-      "  --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" +
-      "\n" +
-      "Boolean options can be turned off like this:\n" +
-      "  --no-bracket-spacing\n" +
-      "  --bracket-spacing=false"
-  );
-  process.exit(argv["help"] ? 0 : 1);
-}
-
 function getParserOption() {
   const optionName = "parser";
   const value = argv[optionName];
@@ -122,14 +98,32 @@ function getIntOption(optionName) {
   process.exit(1);
 }
 
+function getTrailingComma() {
+  let trailingComma;
+  switch (argv["trailing-comma"]) {
+    case undefined:
+    case "none":
+      return "none";
+    case "":
+      console.warn("Warning: `--trailing-comma` was used without an argument. This is deprecated. " +
+                   'Specify "none", "es5", or "all".')
+    case "es5":
+      return "es5";
+    case "all":
+      return "all";
+    default:
+      throw new Error("Invalid option for --trailing-comma");
+  }
+}
+
 const options = {
   printWidth: getIntOption("print-width"),
   tabWidth: getIntOption("tab-width"),
   bracketSpacing: argv["bracket-spacing"],
-  parser: getParserOption(),
   singleQuote: argv["single-quote"],
-  trailingComma: argv["trailing-comma"],
-  jsxBracketSameLine: argv["jsx-bracket-same-line"]
+  jsxBracketSameLine: argv["jsx-bracket-same-line"],
+  trailingComma: getTrailingComma(),
+  parser: getParserOption()
 };
 
 function format(input) {
@@ -177,6 +171,30 @@ function handleError(filename, e) {
   process.exitCode = 2;
 }
 
+if (argv["help"] || !filepatterns.length && !stdin) {
+  console.log(
+    "Usage: prettier [opts] [filename ...]\n\n" +
+      "Available options:\n" +
+      "  --write                  Edit the file in-place. (Beware!)\n" +
+      "  --stdin                  Read input from stdin.\n" +
+      "  --print-width <int>      Specify the length of line that the printer will wrap on. Defaults to 80.\n" +
+      "  --tab-width <int>        Specify the number of spaces per indentation-level. Defaults to 2.\n" +
+      "  --single-quote           Use single quotes instead of double.\n" +
+      "  --bracket-spacing        Put spaces between brackets. Defaults to true.\n" +
+      "  --jsx-bracket-same-line  Put > on the last line. Defaults to false.\n" +
+      "  --trailing-comma <none|es5|all>\n" +
+      "                           Print trailing commas wherever possible. Defaults to none.\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" +
+      "\n" +
+      "Boolean options can be turned off like this:\n" +
+      "  --no-bracket-spacing\n" +
+      "  --bracket-spacing=false"
+  );
+  process.exit(argv["help"] ? 0 : 1);
+}
+
 if (stdin) {
   getStdin().then(input => {
     try {
diff --git a/src/options.js b/src/options.js
index 73fc4191d..9bc07f399 100644
--- a/src/options.js
+++ b/src/options.js
@@ -7,7 +7,7 @@ var defaults = {
   tabWidth: 2,
   printWidth: 80,
   singleQuote: false,
-  trailingComma: false,
+  trailingComma: "none",
   bracketSpacing: true,
   jsxBracketSameLine: false,
   parser: "babylon"
@@ -21,6 +21,17 @@ var exampleConfig = Object.assign({}, defaults, {
 
 // Copy options and fill in default values.
 function normalize(options) {
+  if(typeof options.trailingComma === "boolean") {
+    // Support a deprecated boolean type for the trailing comma config
+    // for a few versions. This code can be removed later.
+    options.trailingComma =  "es5";
+
+    console.warn(
+      "Warning: `trailingComma` without any argument is deprecated. " +
+      'Specify "none", "es5", or "all".'
+    );
+  }
+
   validate(options, { exampleConfig, deprecatedConfig });
   const normalized = Object.assign({}, options || {});
 
diff --git a/src/printer.js b/src/printer.js
index b0d796ebb..80b59eb91 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -34,6 +34,24 @@ function maybeAddParens(path, lines) {
   return path.needsParens() ? concat(["(", lines, ")"]) : lines;
 }
 
+function shouldPrintComma(options, level) {
+  level = level || "es5";
+
+  switch(options.trailingComma) {
+    case "all":
+      if(level === "all") {
+        return true;
+      }
+    case "es5":
+      if(level === "es5") {
+        return true;
+      }
+    case "none":
+    default:
+      return false;
+  }
+}
+
 function genericPrint(path, options, printPath) {
   assert.ok(path instanceof FastPath);
 
@@ -494,7 +512,7 @@ function genericPrintNoParens(path, options, print) {
                     join(concat([",", line]), grouped)
                   ])
                 ),
-                ifBreak(options.trailingComma ? "," : ""),
+                ifBreak(shouldPrintComma(options) ? "," : ""),
                 options.bracketSpacing ? line : softline,
                 "}"
               ])
@@ -683,7 +701,7 @@ function genericPrintNoParens(path, options, print) {
               options.tabWidth + (parentIsUnionTypeAnnotation ? 2 : 0),
               concat([options.bracketSpacing ? line : softline, concat(props)])
             ),
-            ifBreak(canHaveTrailingComma && options.trailingComma ? "," : ""),
+            ifBreak(canHaveTrailingComma && shouldPrintComma(options) ? "," : ""),
             indent(
               parentIsUnionTypeAnnotation ? 2 : 0,
               concat([options.bracketSpacing ? line : softline, rightBrace])
@@ -792,7 +810,7 @@ function genericPrintNoParens(path, options, print) {
               ifBreak(
                 canHaveTrailingComma &&
                   !needsForcedTrailingComma &&
-                  options.trailingComma
+                  shouldPrintComma(options)
                   ? ","
                   : ""
               ),
@@ -1867,7 +1885,7 @@ function printArgumentsList(path, options, print) {
                 options.tabWidth,
                 concat([line, join(concat([",", line]), printed)])
               ),
-              options.trailingComma ? "," : "",
+              shouldPrintComma(options, "all") ? "," : "",
               line,
               ")"
             ]),
@@ -1886,7 +1904,7 @@ function printArgumentsList(path, options, print) {
         options.tabWidth,
         concat([softline, join(concat([",", line]), printed)])
       ),
-      ifBreak(options.trailingComma ? "," : ""),
+      ifBreak(shouldPrintComma(options, "all") ? "," : ""),
       softline,
       ")"
     ]),
@@ -1932,7 +1950,7 @@ function printFunctionParams(path, print, options) {
       options.tabWidth,
       concat([softline, join(concat([",", line]), printed)])
     ),
-    ifBreak(canHaveTrailingComma && options.trailingComma ? "," : ""),
+    ifBreak(canHaveTrailingComma && shouldPrintComma(options) ? "," : ""),
     softline,
     ")"
   ]);
@@ -2043,7 +2061,7 @@ function printExportDeclaration(path, options, print) {
                   join(concat([",", line]), path.map(print, "specifiers"))
                 ])
               ),
-              ifBreak(options.trailingComma ? "," : ""),
+              ifBreak(shouldPrintComma(options) ? "," : ""),
               options.bracketSpacing ? line : softline,
               "}"
             ])


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index 038f3416f..d70983ab8 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -86,6 +86,32 @@ declare class Foo extends Qux<string> {
 "
 `;
 
+exports[`test dangling_array.js 1`] = `
+"expect(() => {}).toTriggerReadyStateChanges([
+  // Nothing.
+]);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+expect(() => {}).toTriggerReadyStateChanges(
+  [
+    // Nothing.
+  ]
+);
+"
+`;
+
+exports[`test dangling_array.js 2`] = `
+"expect(() => {}).toTriggerReadyStateChanges([
+  // Nothing.
+]);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+expect(() => {}).toTriggerReadyStateChanges(
+  [
+    // Nothing.
+  ]
+);
+"
+`;
+
 exports[`test first-line.js 1`] = `
 "a // comment
 b
diff --git a/tests/rest/jsfmt.spec.js b/tests/rest/jsfmt.spec.js
index 5495c026a..11c5ee1fa 100644
--- a/tests/rest/jsfmt.spec.js
+++ b/tests/rest/jsfmt.spec.js
@@ -1,1 +1,1 @@
-run_spec(__dirname, {trailingComma: true});
+run_spec(__dirname, {trailingComma: "all"});
diff --git a/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap b/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap
index 6dc4aca65..3a93f5dbc 100644
--- a/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/trailing_comma/__snapshots__/jsfmt.spec.js.snap
@@ -60,6 +60,37 @@ a(
 "
 `;
 
+exports[`test function-calls.js 3`] = `
+"const a = (param1, param2, param3) => {}
+
+a(\'value\', \'value2\', \'value3\');
+
+a(
+  \'a-long-value\',
+  \'a-really-really-long-value\',
+  \'a-really-really-really-long-value\',
+);
+
+a(\'value\', \'value2\', a(\'long-nested-value\', \'long-nested-value2\', \'long-nested-value3\'));
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+const a = (param1, param2, param3) => {};
+
+a(\"value\", \"value2\", \"value3\");
+
+a(
+  \"a-long-value\",
+  \"a-really-really-long-value\",
+  \"a-really-really-really-long-value\"
+);
+
+a(
+  \"value\",
+  \"value2\",
+  a(\"long-nested-value\", \"long-nested-value2\", \"long-nested-value3\")
+);
+"
+`;
+
 exports[`test object.js 1`] = `
 "const a = {
   b: true,
@@ -154,6 +185,53 @@ const aLong = {
 "
 `;
 
+exports[`test object.js 3`] = `
+"const a = {
+  b: true,
+  c: {
+    c1: \'hello\'
+  },
+  d: false
+};
+
+const aLong = {
+  bHasALongName: \'a-long-value\',
+  cHasALongName: {
+    c1: \'a-really-long-value\',
+    c2: \'a-really-really-long-value\',
+  },
+  dHasALongName: \'a-long-value-too\'
+};
+
+const aLong = {
+  dHasALongName: \'a-long-value-too\',
+  eHasABooleanExpression: a === a,
+};
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+const a = {
+  b: true,
+  c: {
+    c1: \"hello\",
+  },
+  d: false,
+};
+
+const aLong = {
+  bHasALongName: \"a-long-value\",
+  cHasALongName: {
+    c1: \"a-really-long-value\",
+    c2: \"a-really-really-long-value\",
+  },
+  dHasALongName: \"a-long-value-too\",
+};
+
+const aLong = {
+  dHasALongName: \"a-long-value-too\",
+  eHasABooleanExpression: a === a,
+};
+"
+`;
+
 exports[`test trailing_whitespace.js 1`] = `
 "let example = [
   \'FOO\',
@@ -305,3 +383,79 @@ function supersupersupersuperLongF(
 }
 "
 `;
+
+exports[`test trailing_whitespace.js 3`] = `
+"let example = [
+  \'FOO\',
+  \'BAR\',
+  // Comment
+];
+
+foo({},
+  // Comment
+);
+
+o = {
+  state,
+  // Comment
+};
+
+o = {
+  state,
+
+  // Comment
+};
+
+function supersupersupersuperLongF(
+  supersupersupersuperLongA,
+  supersupersupersuperLongB
+  // Comment
+) {
+  a
+}
+function supersupersupersuperLongF(
+  supersupersupersuperLongA,
+  supersupersupersuperLongB,
+  // Comment
+) {
+  a
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+let example = [
+  \"FOO\",
+  \"BAR\",
+  // Comment
+];
+
+foo(
+  {}
+  // Comment
+);
+
+o = {
+  state,
+  // Comment
+};
+
+o = {
+  state,
+
+  // Comment
+};
+
+function supersupersupersuperLongF(
+  supersupersupersuperLongA,
+  supersupersupersuperLongB,
+) // Comment
+{
+  a;
+}
+function supersupersupersuperLongF(
+  supersupersupersuperLongA,
+  supersupersupersuperLongB,
+) // Comment
+{
+  a;
+}
+"
+`;
diff --git a/tests/trailing_comma/jsfmt.spec.js b/tests/trailing_comma/jsfmt.spec.js
index 61e3318df..8d420a89b 100644
--- a/tests/trailing_comma/jsfmt.spec.js
+++ b/tests/trailing_comma/jsfmt.spec.js
@@ -1,3 +1,3 @@
 run_spec(__dirname);
-
-run_spec(__dirname, { trailingComma: true });
+run_spec(__dirname, { trailingComma: "all" });
+run_spec(__dirname, { trailingComma: "es5" });