fix: respect newlines in parameters (#5836)

CHANGELOG.unreleased.md

@@ -42,6 +42,64 @@ Examples:
 
 -->
 
+- JavaScript: Respect newlines between parameters ([#5260] by [@evilebottnawi])
+
+  <!-- prettier-ignore -->
+  ```js
+  // Input
+  function foo(
+    one,
+  
+    two,
+    three,
+    four,
+  
+  
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+  
+    eleven
+  
+  ) {}
+    
+  // Output (Prettier stable)
+  function foo(
+    one,
+    two,
+    three,
+    four,
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+    eleven
+  ) {}
+    
+  // Output (Prettier master)
+  function foo(
+    one,
+  
+    two,
+    three,
+    four,
+  
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+  
+    eleven
+  ) {}
+  ```
+
 - JavaScript: Fix multiline dynamic import comments ([#6025] by [@noahsug])
 
   <!-- prettier-ignore -->

src/language-js/printer-estree.js

@@ -4104,7 +4104,12 @@ function printFunctionTypeParameters(path, options, print) {
 
 function printFunctionParams(path, print, options, expandArg, printTypeParams) {
   const fun = path.getValue();
+  const parent = path.getParentNode();
   const paramsField = fun.parameters ? "parameters" : "params";
+  const isParametersInTestCall = isTestCall(parent);
+  const shouldHugParameters = shouldHugArguments(fun);
+  const shouldExpandParameters =
+    expandArg && !(fun[paramsField] && fun[paramsField].some(n => n.comments));
 
   const typeParams = printTypeParams
     ? printFunctionTypeParameters(path, options, print)
@@ -4112,7 +4117,32 @@ function printFunctionParams(path, print, options, expandArg, printTypeParams) {
 
   let printed = [];
   if (fun[paramsField]) {
-    printed = path.map(print, paramsField);
+    const lastArgIndex = fun[paramsField].length - 1;
+
+    printed = path.map((childPath, index) => {
+      const parts = [];
+      const param = childPath.getValue();
+
+      parts.push(print(childPath));
+
+      if (index === lastArgIndex) {
+        if (fun.rest) {
+          parts.push(",", line);
+        }
+      } else if (
+        isParametersInTestCall ||
+        shouldHugParameters ||
+        shouldExpandParameters
+      ) {
+        parts.push(", ");
+      } else if (isNextLineEmpty(options.originalText, param, options)) {
+        parts.push(",", hardline, hardline);
+      } else {
+        parts.push(",", line);
+      }
+
+      return concat(parts);
+    }, paramsField);
   }
 
   if (fun.rest) {
@@ -4150,15 +4180,12 @@ function printFunctionParams(path, print, options, expandArg, printTypeParams) {
   //     }                     b,
   //   })                    ) => {
   //                         })
-  if (
+  if (shouldExpandParameters) {
-    expandArg &&
-    !(fun[paramsField] && fun[paramsField].some(n => n.comments))
-  ) {
     return group(
       concat([
         removeLines(typeParams),
         "(",
-        join(", ", printed.map(removeLines)),
+        concat(printed.map(removeLines)),
         ")"
       ])
     );
@@ -4171,15 +4198,13 @@ function printFunctionParams(path, print, options, expandArg, printTypeParams) {
   //   b,
   //   c
   // }) {}
-  if (shouldHugArguments(fun)) {
+  if (shouldHugParameters) {
-    return concat([typeParams, "(", join(", ", printed), ")"]);
+    return concat([typeParams, "(", concat(printed), ")"]);
   }
 
-  const parent = path.getParentNode();
-
   // don't break in specs, eg; `it("should maintain parens around done even when long", (done) => {})`
-  if (isTestCall(parent)) {
+  if (isParametersInTestCall) {
-    return concat([typeParams, "(", join(", ", printed), ")"]);
+    return concat([typeParams, "(", concat(printed), ")"]);
   }
 
   const isFlowShorthandWithOneArg =
@@ -4211,7 +4236,7 @@ function printFunctionParams(path, print, options, expandArg, printTypeParams) {
   return concat([
     typeParams,
     "(",
-    indent(concat([softline, join(concat([",", line]), printed)])),
+    indent(concat([softline, concat(printed)])),
     ifBreak(
       canHaveTrailingComma && shouldPrintComma(options, "all") ? "," : ""
     ),

tests/preserve_line/__snapshots__/jsfmt.spec.js.snap

@@ -194,6 +194,25 @@ doSomething(
 
 );
 
+function foo(
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+) {}
+
 =====================================output=====================================
 longArgNamesWithComments(
   // Hello World
@@ -356,6 +375,23 @@ doSomething(
   { helloWorld, someImportantStuff }
 );
 
+function foo(
+  one,
+
+  two,
+  three,
+  four,
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+) {}
+
 ================================================================================
 `;
 
@@ -556,3 +592,261 @@ const sel = this.connections
 
 ================================================================================
 `;
+
+exports[`parameter-list.js 1`] = `
+====================================options=====================================
+parsers: ["flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+class Foo {
+  constructor(
+    one,
+
+    two,
+    three,
+    four,
+
+
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+
+    eleven
+
+  ) {}
+}
+
+function foo(
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+) {}
+
+call((a, b) => {});
+
+call((
+  one,
+  two,
+  three,
+  four,
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+  eleven
+) => {});
+
+call((
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+) => {});
+
+function test({
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+}) {}
+
+function test({
+  one,
+  two,
+  three,
+  four,
+}) {}
+
+function test({
+  one,
+
+  two,
+  three,
+  four,
+
+}) {}
+
+function test({ one, two, three, four }, $a) {}
+
+
+function test(
+  { one, two, three, four },
+
+  $a
+) {}
+
+function foo(
+
+  ...rest
+
+) {}
+
+function foo(
+  one,
+
+  ...rest
+) {}
+
+function foo(one,...rest) {}
+
+f(
+  superSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperLong,...args
+);
+
+it(
+
+  "does something really long and complicated so I have to write a very long name for the test",
+
+  function(
+
+    done,
+
+    foo
+  ) {
+
+    console.log("hello!");
+  }
+);
+
+=====================================output=====================================
+class Foo {
+  constructor(
+    one,
+
+    two,
+    three,
+    four,
+
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+
+    eleven
+  ) {}
+}
+
+function foo(
+  one,
+
+  two,
+  three,
+  four,
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+) {}
+
+call((a, b) => {});
+
+call((one, two, three, four, five, six, seven, eight, nine, ten, eleven) => {});
+
+call((one, two, three, four, five, six, seven, eight, nine, ten, eleven) => {});
+
+function test({
+  one,
+
+  two,
+  three,
+  four,
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+}) {}
+
+function test({ one, two, three, four }) {}
+
+function test({
+  one,
+
+  two,
+  three,
+  four
+}) {}
+
+function test({ one, two, three, four }, $a) {}
+
+function test(
+  { one, two, three, four },
+
+  $a
+) {}
+
+function foo(...rest) {}
+
+function foo(
+  one,
+
+  ...rest
+) {}
+
+function foo(one, ...rest) {}
+
+f(
+  superSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperLong,
+  ...args
+);
+
+it("does something really long and complicated so I have to write a very long name for the test", function(done, foo) {
+  console.log("hello!");
+});
+
+================================================================================
+`;

tests/preserve_line/argument-list.js

@@ -185,3 +185,22 @@ doSomething(
 
 
 );
+
+function foo(
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+) {}

tests/preserve_line/parameter-list.js

@@ -0,0 +1,151 @@
+class Foo {
+  constructor(
+    one,
+
+    two,
+    three,
+    four,
+
+
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+
+    eleven
+
+  ) {}
+}
+
+function foo(
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+) {}
+
+call((a, b) => {});
+
+call((
+  one,
+  two,
+  three,
+  four,
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+  eleven
+) => {});
+
+call((
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+) => {});
+
+function test({
+  one,
+
+  two,
+  three,
+  four,
+
+
+  five,
+  six,
+  seven,
+  eight,
+  nine,
+  ten,
+
+  eleven
+
+}) {}
+
+function test({
+  one,
+  two,
+  three,
+  four,
+}) {}
+
+function test({
+  one,
+
+  two,
+  three,
+  four,
+
+}) {}
+
+function test({ one, two, three, four }, $a) {}
+
+
+function test(
+  { one, two, three, four },
+
+  $a
+) {}
+
+function foo(
+
+  ...rest
+
+) {}
+
+function foo(
+  one,
+
+  ...rest
+) {}
+
+function foo(one,...rest) {}
+
+f(
+  superSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperLong,...args
+);
+
+it(
+
+  "does something really long and complicated so I have to write a very long name for the test",
+
+  function(
+
+    done,
+
+    foo
+  ) {
+
+    console.log("hello!");
+  }
+);

tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap

@@ -1,5 +1,42 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`argument-list-preserve-line.ts 1`] = `
+====================================options=====================================
+parsers: ["typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+class Foo {
+    constructor(
+        @inject(Bar)
+        private readonly bar: IBar,
+
+        @inject(MyProcessor)
+        private readonly myProcessor: IMyProcessor,
+
+        @inject(InjectionTypes.AnotherThing)
+
+        private readonly anotherThing: IAnotherThing | undefined,
+    ) { }
+}
+
+=====================================output=====================================
+class Foo {
+  constructor(
+    @inject(Bar)
+    private readonly bar: IBar,
+
+    @inject(MyProcessor)
+    private readonly myProcessor: IMyProcessor,
+
+    @inject(InjectionTypes.AnotherThing)
+    private readonly anotherThing: IAnotherThing | undefined
+  ) {}
+}
+
+================================================================================
+`;
+
 exports[`decorator-type-assertion.ts 1`] = `
 ====================================options=====================================
 parsers: ["typescript"]

tests/typescript_decorators/argument-list-preserve-line.ts

@@ -0,0 +1,13 @@
+class Foo {
+    constructor(
+        @inject(Bar)
+        private readonly bar: IBar,
+
+        @inject(MyProcessor)
+        private readonly myProcessor: IMyProcessor,
+
+        @inject(InjectionTypes.AnotherThing)
+
+        private readonly anotherThing: IAnotherThing | undefined,
+    ) { }
+}