Fix dangling comment print in function parameters (#18623)

changelog_unreleased/javascript/18623.md

@@ -0,0 +1,27 @@
+#### Fix dangling comment print in function parameters (#18623 by @fisker)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+fn = function(
+  // comment  
+) {}
+arrow = (
+  // comment  
+) => {}
+
+// Prettier stable
+fn = function () // comment
+{};
+arrow = () =>
+  // comment
+  {};
+
+// Prettier main
+fn = function (
+  // comment
+) {};
+arrow = (
+  // comment
+) => {};
+```

src/language-js/comments/handle-comments.js

@@ -21,6 +21,7 @@ import {
   isConditionalType,
   isIntersectionType,
   isMemberExpression,
+  isMethod,
   isObjectProperty,
   isPrettierIgnoreComment,
   isUnionType,
@@ -61,6 +62,7 @@ const isSingleLineComment = (comment, text) =>
  */
 function handleOwnLineComment(context) {
   return [
+    handleCommentInEmptyParens,
     handleIgnoreComments,
     handleConditionalExpressionComments,
     handleLastFunctionArgComments,
@@ -83,7 +85,6 @@ function handleOwnLineComment(context) {
     handleCommentsInDestructuringPattern,
     handleTSMappedTypeComments,
     handleBinaryCastExpressionComment,
-    handleCommentInEmptyParens,
   ].some((fn) => fn(context));
 }
 
@@ -93,6 +94,7 @@ function handleOwnLineComment(context) {
  */
 function handleEndOfLineComment(context) {
   return [
+    handleCommentInEmptyParens,
     handleClosureTypeCastComments,
     handleLastFunctionArgComments,
     handleConditionalExpressionComments,
@@ -114,7 +116,6 @@ function handleEndOfLineComment(context) {
     handleCommentAfterArrowExpression,
     handlePropertySignatureComments,
     handleBinaryCastExpressionComment,
-    handleCommentInEmptyParens,
   ].some((fn) => fn(context));
 }
 
@@ -124,10 +125,10 @@ function handleEndOfLineComment(context) {
  */
 function handleRemainingComment(context) {
   return [
+    handleCommentInEmptyParens,
     handleIgnoreComments,
     handleIfStatementComments,
     handleWhileComments,
-    handleCommentInEmptyParens,
     handleMethodNameComments,
     handleOnlyComments,
     handleCommentAfterArrowParams,
@@ -658,18 +659,14 @@ function handleCommentInEmptyParens({ comment, enclosingNode, options }) {
     return true;
   }
 
-  // This condition should be removed, but excluded for function parameters in #18615 to make PRs smaller
-  if (comment.placement !== "remaining") {
-    return false;
-  }
-
   // Only add dangling comments to fix the case when no params are present,
   // i.e. a function without any argument.
 
   const functionNode = isRealFunctionLikeNode(enclosingNode)
     ? enclosingNode
     : enclosingNode.type === "MethodDefinition" ||
-        enclosingNode.type === "TSAbstractMethodDefinition"
+        enclosingNode.type === "TSAbstractMethodDefinition" ||
+        (enclosingNode.type === "Property" && isMethod(enclosingNode))
       ? enclosingNode.value
       : undefined;
 
@@ -717,6 +714,7 @@ function handleLastComponentArgComments({
   return false;
 }
 
+// This function seem doing bad job for cases that `handleCommentInEmptyParens` do
 function handleLastFunctionArgComments({
   comment,
   precedingNode,

src/language-js/print/function-parameters.js

@@ -12,6 +12,7 @@ import {
 import { printDanglingComments } from "../../main/comments/print.js";
 import isNonEmptyArray from "../../utilities/is-non-empty-array.js";
 import {
+  CommentCheckFlags,
   getFunctionParameters,
   hasComment,
   hasRestParameter,
@@ -29,6 +30,10 @@ import {
 
 /** @import AstPath from "../../common/ast-path.js" */
 
+// `ArrowFunctionExpression` has other dangling comments
+const functionParameterDanglingCommentFilter = (comment) =>
+  comment.mark !== "commentBeforeArrow";
+
 /*
 - `ArrowFunctionExpression`
 - `FunctionDeclaration`
@@ -70,9 +75,27 @@ function printFunctionParameters(
     return [
       typeParametersDoc,
       "(",
-      printDanglingComments(path, options, {
-        filter: (comment) => comment.mark !== "commentBeforeArrow",
-      }),
+      hasComment(
+        functionNode,
+        CommentCheckFlags.Dangling,
+        functionParameterDanglingCommentFilter,
+      )
+        ? [
+            indent([
+              softline,
+              printDanglingComments(path, options, {
+                filter: functionParameterDanglingCommentFilter,
+              }),
+            ]),
+            hasComment(
+              functionNode,
+              CommentCheckFlags.Dangling | CommentCheckFlags.Line,
+              functionParameterDanglingCommentFilter,
+            )
+              ? hardline
+              : softline,
+          ]
+        : [],
       ")",
     ];
   }

tests/format/js/comments/in-list/__snapshots__/format.test.js.snap

@@ -10,7 +10,17 @@ array = [/* array */]
 object = {/* object */}
 call = call(/* call argument */)
 fn = function(/* function parameter */) {}
+arrow = (/* function parameter */) => {}
 newFoo = new Foo(/* new argument */)
+c = class {
+constructor(/* class constructor parameter */){}
+method(/* class method parameter */){}
+get getter(/* class getter parameter */){}
+}
+object = {
+method(/* object method parameter */){},
+get getter(/* object getter parameter */){}
+}
 
 array = [
 /* array */]
@@ -18,49 +28,160 @@ object = {
 /* object */}
 call = call(
 /* call argument */)
-// TODO[@fisker]: fix this
 fn = function(
 /* function parameter */) {}
+arrow = (
+/* arrow parameter */) => {}
 newFoo = new Foo
-(/* new argument */)
+(
+/* new argument */)
+c = class {
+constructor(
+/* class constructor parameter */){}
+method(
+/* class method parameter */){}
+get getter(
+/* class getter parameter */){}
+}
+object = {
+method(
+/* object method parameter */){},
+get getter(
+/* object getter parameter */){}
+}
 
-array = [/* array long long long long long long long long long long long long long long */]
-object = {/* object long long long long long long long long long long long long long long */}
+array = [/* array */
+]
+object = {/* object */
+}
+call = call(/* call argument */
+)
+fn = function(/* function parameter */
+) {}
+arrow = (/* function parameter */
+) => {}
+newFoo = new Foo(/* new argument */
+)
+c = class {
+constructor(/* class constructor parameter */
+){}
+method(/* class method parameter */
+){}
+get getter(/* class getter parameter */
+){}
+}
+object = {
+method(/* object method parameter */
+){},
+get getter(/* object getter parameter */
+){}
+}
+
+array = [/* array long long long long long long long long long long long long long long long long */]
+object = {/* object long long long long long long long long long long long long long long long long */}
 call = call(/* call argument long long long long long long long long long long long long long long */)
-// TODO[@fisker]: fix this
 fn = function (/* function long parameter long long long long long long long long long long long long long */) {}
+arrow = (/* function long parameter long long long long long long long long long long long long long */) => {}
 newFoo = new Foo(/* new argument long long long long long long long long long long long long long long */)
+c = class {
+constructor(/* class constructor parameter long long long long long long long long long long long long long long */){}
+method(/* class method parameter long long long long long long long long long long long long long long */){}
+get getter(/* class getter parameter long long long long long long long long long long long long long long */){}
+}
+object = {
+method(
+/* object method parameter long long long long long long long long long long long long long long */){},
+get getter(
+/* object getter parameter long long long long long long long long long long long long long long */){}
+}
 
 =====================================output=====================================
 array = [/* array */];
 object = {/* object */};
 call = call(/* call argument */);
 fn = function (/* function parameter */) {};
+arrow = (/* function parameter */) => {};
 newFoo = new Foo(/* new argument */);
+c = class {
+  constructor(/* class constructor parameter */) {}
+  method(/* class method parameter */) {}
+  get getter(/* class getter parameter */) {}
+};
+object = {
+  method(/* object method parameter */) {},
+  get getter(/* object getter parameter */) {},
+};
 
 array = [/* array */];
 object = {/* object */};
 call = call(/* call argument */);
-// TODO[@fisker]: fix this
-fn = function () /* function parameter */ {};
+fn = function (/* function parameter */) {};
+arrow = (/* arrow parameter */) => {};
 newFoo = new Foo(/* new argument */);
+c = class {
+  constructor(/* class constructor parameter */) {}
+  method(/* class method parameter */) {}
+  get getter(/* class getter parameter */) {}
+};
+object = {
+  method(/* object method parameter */) {},
+  get getter(/* object getter parameter */) {},
+};
+
+array = [/* array */];
+object = {/* object */};
+call = call(/* call argument */);
+fn = function (/* function parameter */) {};
+arrow = (/* function parameter */) => {};
+newFoo = new Foo(/* new argument */);
+c = class {
+  constructor(/* class constructor parameter */) {}
+  method(/* class method parameter */) {}
+  get getter(/* class getter parameter */) {}
+};
+object = {
+  method(/* object method parameter */) {},
+  get getter(/* object getter parameter */) {},
+};
 
 array = [
-  /* array long long long long long long long long long long long long long long */
+  /* array long long long long long long long long long long long long long long long long */
 ];
 object = {
-  /* object long long long long long long long long long long long long long long */
+  /* object long long long long long long long long long long long long long long long long */
 };
 call =
   call(
     /* call argument long long long long long long long long long long long long long long */
   );
-// TODO[@fisker]: fix this
-fn =
-  function (/* function long parameter long long long long long long long long long long long long long */) {};
+fn = function (
+  /* function long parameter long long long long long long long long long long long long long */
+) {};
+arrow = (
+  /* function long parameter long long long long long long long long long long long long long */
+) => {};
 newFoo = new Foo(
   /* new argument long long long long long long long long long long long long long long */
 );
+c = class {
+  constructor(
+    /* class constructor parameter long long long long long long long long long long long long long long */
+  ) {}
+  method(
+    /* class method parameter long long long long long long long long long long long long long long */
+  ) {}
+  get getter(
+    /* class getter parameter long long long long long long long long long long long long long long */
+  ) {}
+};
+object = {
+  method(
+    /* object method parameter long long long long long long long long long long long long long long */
+  ) {},
+  get getter(
+    /* object getter parameter long long long long long long long long long long long long long long */
+  ) {},
+};
 
 ================================================================================
 `;

tests/format/js/comments/in-list/dangling-comment-in-list.js

@@ -2,7 +2,17 @@ array = [/* array */]
 object = {/* object */}
 call = call(/* call argument */)
 fn = function(/* function parameter */) {}
+arrow = (/* function parameter */) => {}
 newFoo = new Foo(/* new argument */)
+c = class {
+constructor(/* class constructor parameter */){}
+method(/* class method parameter */){}
+get getter(/* class getter parameter */){}
+}
+object = {
+method(/* object method parameter */){},
+get getter(/* object getter parameter */){}
+}
 
 array = [
 /* array */]
@@ -10,15 +20,69 @@ object = {
 /* object */}
 call = call(
 /* call argument */)
-// TODO[@fisker]: fix this
 fn = function(
 /* function parameter */) {}
+arrow = (
+/* arrow parameter */) => {}
 newFoo = new Foo
-(/* new argument */)
+(
+/* new argument */)
+c = class {
+constructor(
+/* class constructor parameter */){}
+method(
+/* class method parameter */){}
+get getter(
+/* class getter parameter */){}
+}
+object = {
+method(
+/* object method parameter */){},
+get getter(
+/* object getter parameter */){}
+}
 
-array = [/* array long long long long long long long long long long long long long long */]
-object = {/* object long long long long long long long long long long long long long long */}
+array = [/* array */
+]
+object = {/* object */
+}
+call = call(/* call argument */
+)
+fn = function(/* function parameter */
+) {}
+arrow = (/* function parameter */
+) => {}
+newFoo = new Foo(/* new argument */
+)
+c = class {
+constructor(/* class constructor parameter */
+){}
+method(/* class method parameter */
+){}
+get getter(/* class getter parameter */
+){}
+}
+object = {
+method(/* object method parameter */
+){},
+get getter(/* object getter parameter */
+){}
+}
+
+array = [/* array long long long long long long long long long long long long long long long long */]
+object = {/* object long long long long long long long long long long long long long long long long */}
 call = call(/* call argument long long long long long long long long long long long long long long */)
-// TODO[@fisker]: fix this
 fn = function (/* function long parameter long long long long long long long long long long long long long */) {}
+arrow = (/* function long parameter long long long long long long long long long long long long long */) => {}
 newFoo = new Foo(/* new argument long long long long long long long long long long long long long long */)
+c = class {
+constructor(/* class constructor parameter long long long long long long long long long long long long long long */){}
+method(/* class method parameter long long long long long long long long long long long long long long */){}
+get getter(/* class getter parameter long long long long long long long long long long long long long long */){}
+}
+object = {
+method(
+/* object method parameter long long long long long long long long long long long long long long */){},
+get getter(
+/* object getter parameter long long long long long long long long long long long long long long */){}
+}

tests/format/js/function/__snapshots__/format.test.js.snap

@@ -1,5 +1,221 @@
 // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
 
+exports[`dangling-comments.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+arrow = (// 101
+) => {}
+arrow = (
+// 102
+) => {}
+fn = function a(// 103
+) {}
+fn = function a(
+// 104
+) {}
+c = class {
+  constructor(// 105
+  ){}
+  method(// 106
+  ) {}
+  method2(
+// 107
+  ) {}
+  get getter(// 108
+) {
+  }
+  get getter2(
+// 109
+) {
+  }
+  property = function a(// 110
+) {}
+  property2 = function a(
+// 111
+) {}
+}
+c = class {
+  constructor(
+// 112
+  ){}
+}
+object = {
+  method(// 113
+  ) {},
+  method2(
+// 114
+  ) {},
+  property: function a(// 115
+) {},
+  property2: function a(
+// 116
+) {},
+}
+
+arrow = (/* 201 */
+) => {}
+arrow = (
+/* 202 */
+) => {}
+arrow = (
+/* 203 */) => {}
+fn = function a(/* 204 */
+) {}
+fn = function a(
+/* 205 */
+) {}
+fn = function a(
+/* 206 */) {}
+c = class {
+  constructor(/* 207 */
+  ){}
+  method(/* 208 */
+  ) {}
+  method2(
+/* 209 */
+  ) {}
+  method3(
+/* 210 */) {}
+  get getter(/* 211 */
+) {
+  }
+  get getter2(
+/* 212 */
+) {
+  }
+  get getter3(
+/* 213 */) {
+  }
+  property = function a(/* 214 */
+) {}
+  property2 = function a(
+/* 215 */
+) {}
+  property3 = function a(
+/* 216 */) {}
+}
+c = class {
+  constructor(
+/* 217 */
+  ){}
+}
+c = class {
+  constructor(
+/* 218 */){}
+}
+object = {
+  method(/* 219 */
+  ) {},
+  method2(
+/* 220 */
+  ) {},
+  method3(
+/* 221 */) {},
+  property: function a(/* 222 */
+) {},
+  property2: function a(
+/* 223 */
+) {},
+  property2: function a(
+/* 224 */) {},
+}
+
+
+=====================================output=====================================
+arrow = (
+  // 101
+) => {};
+arrow = (
+  // 102
+) => {};
+fn = function a(
+  // 103
+) {};
+fn = function a(
+  // 104
+) {};
+c = class {
+  constructor(
+    // 105
+  ) {}
+  method(
+    // 106
+  ) {}
+  method2(
+    // 107
+  ) {}
+  get getter(
+    // 108
+  ) {}
+  get getter2(
+    // 109
+  ) {}
+  property = function a(
+    // 110
+  ) {};
+  property2 = function a(
+    // 111
+  ) {};
+};
+c = class {
+  constructor(
+    // 112
+  ) {}
+};
+object = {
+  method(
+    // 113
+  ) {},
+  method2(
+    // 114
+  ) {},
+  property: function a(
+    // 115
+  ) {},
+  property2: function a(
+    // 116
+  ) {},
+};
+
+arrow = (/* 201 */) => {};
+arrow = (/* 202 */) => {};
+arrow = (/* 203 */) => {};
+fn = function a(/* 204 */) {};
+fn = function a(/* 205 */) {};
+fn = function a(/* 206 */) {};
+c = class {
+  constructor(/* 207 */) {}
+  method(/* 208 */) {}
+  method2(/* 209 */) {}
+  method3(/* 210 */) {}
+  get getter(/* 211 */) {}
+  get getter2(/* 212 */) {}
+  get getter3(/* 213 */) {}
+  property = function a(/* 214 */) {};
+  property2 = function a(/* 215 */) {};
+  property3 = function a(/* 216 */) {};
+};
+c = class {
+  constructor(/* 217 */) {}
+};
+c = class {
+  constructor(/* 218 */) {}
+};
+object = {
+  method(/* 219 */) {},
+  method2(/* 220 */) {},
+  method3(/* 221 */) {},
+  property: function a(/* 222 */) {},
+  property2: function a(/* 223 */) {},
+  property2: function a(/* 224 */) {},
+};
+
+================================================================================
+`;
+
 exports[`function_expression.js format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow", "typescript"]

tests/format/js/function/dangling-comments.js

@@ -0,0 +1,117 @@
+arrow = (// 101
+) => {}
+arrow = (
+// 102
+) => {}
+fn = function a(// 103
+) {}
+fn = function a(
+// 104
+) {}
+c = class {
+  constructor(// 105
+  ){}
+  method(// 106
+  ) {}
+  method2(
+// 107
+  ) {}
+  get getter(// 108
+) {
+  }
+  get getter2(
+// 109
+) {
+  }
+  property = function a(// 110
+) {}
+  property2 = function a(
+// 111
+) {}
+}
+c = class {
+  constructor(
+// 112
+  ){}
+}
+object = {
+  method(// 113
+  ) {},
+  method2(
+// 114
+  ) {},
+  property: function a(// 115
+) {},
+  property2: function a(
+// 116
+) {},
+}
+
+arrow = (/* 201 */
+) => {}
+arrow = (
+/* 202 */
+) => {}
+arrow = (
+/* 203 */) => {}
+fn = function a(/* 204 */
+) {}
+fn = function a(
+/* 205 */
+) {}
+fn = function a(
+/* 206 */) {}
+c = class {
+  constructor(/* 207 */
+  ){}
+  method(/* 208 */
+  ) {}
+  method2(
+/* 209 */
+  ) {}
+  method3(
+/* 210 */) {}
+  get getter(/* 211 */
+) {
+  }
+  get getter2(
+/* 212 */
+) {
+  }
+  get getter3(
+/* 213 */) {
+  }
+  property = function a(/* 214 */
+) {}
+  property2 = function a(
+/* 215 */
+) {}
+  property3 = function a(
+/* 216 */) {}
+}
+c = class {
+  constructor(
+/* 217 */
+  ){}
+}
+c = class {
+  constructor(
+/* 218 */){}
+}
+object = {
+  method(/* 219 */
+  ) {},
+  method2(
+/* 220 */
+  ) {},
+  method3(
+/* 221 */) {},
+  property: function a(/* 222 */
+) {},
+  property2: function a(
+/* 223 */
+) {},
+  property2: function a(
+/* 224 */) {},
+}
+

tests/format/js/last-argument-expansion/__snapshots__/format.test.js.snap

@@ -146,10 +146,9 @@ foo(
 );
 
 =====================================output=====================================
-foo(() =>
+foo((
   // foo
-  {},
-);
+) => {});
 
 ================================================================================
 `;