Refactor, optimize and fix call args expansion

changelog_unreleased/javascript/13341.md

@@ -0,0 +1,68 @@
+#### Fix edge cases of the first call argument expansion (#13341 by @thorn0)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+export default whatever(function (a: {
+  aaaaaaaaa: string;  
+  bbbbbbbbb: string;
+  ccccccccc: string;
+}) {
+  return null;
+}, "xyz");
+
+call(
+    function() {
+        return 1;
+    },
+    $var ?? $var ?? $var ?? $var ?? $var ?? $var ?? $var ?? $var ?? $var ?? 'test'
+);
+
+// Prettier stable
+export default whatever(function (a: {
+  aaaaaaaaa: string;
+  bbbbbbbbb: string;
+  ccccccccc: string;
+}) {
+  return null;
+},
+"xyz");
+
+call(function () {
+  return 1;
+}, $var ??
+  $var ??
+  $var ??
+  $var ??
+  $var ??
+  $var ??
+  $var ??
+  $var ??
+  $var ??
+  "test");
+
+// Prettier main
+export default whatever(function (a: {
+  aaaaaaaaa: string,
+  bbbbbbbbb: string,
+  ccccccccc: string,
+}) {
+  return null;
+}, "xyz");
+
+call(
+  function () {
+    return 1;
+  },
+  $var ??
+    $var ??
+    $var ??
+    $var ??
+    $var ??
+    $var ??
+    $var ??
+    $var ??
+    $var ??
+    "test",
+);
+```

src/language-js/print/call-arguments.js

@@ -14,6 +14,12 @@ import {
   isCallExpression,
   isStringLiteral,
   isObjectProperty,
+  getCallArgumentSelector,
+  isSimpleCallArgument,
+  isBinaryish,
+  isRegExpLiteral,
+  isSimpleType,
+  isCallLikeExpression,
 } from "../utils/index.js";
 
 import {
@@ -33,7 +39,6 @@ import { isConciselyPrintedArray } from "./array.js";
 
 function printCallArguments(path, options, print) {
   const node = path.getValue();
-  const isDynamicImport = node.type === "ImportExpression";
 
   const args = getCallArguments(node);
   if (args.length === 0) {
@@ -50,7 +55,6 @@ function printCallArguments(path, options, print) {
   }
 
   let anyArgEmptyLine = false;
-  let hasEmptyLineFollowingFirstArg = false;
   const lastArgIndex = args.length - 1;
   const printedArguments = [];
   iterateCallArgumentsPath(path, (argPath, index) => {
@@ -60,10 +64,6 @@ function printCallArguments(path, options, print) {
     if (index === lastArgIndex) {
       // do nothing
     } else if (isNextLineEmpty(arg, options)) {
-      if (index === 0) {
-        hasEmptyLineFollowingFirstArg = true;
-      }
-
       anyArgEmptyLine = true;
       parts.push(",", hardline, hardline);
     } else {
@@ -73,12 +73,11 @@ function printCallArguments(path, options, print) {
     printedArguments.push(parts);
   });
 
+  // Dynamic imports cannot have trailing commas
+  const isDynamicImport =
+    node.type === "ImportExpression" || node.callee.type === "Import";
   const maybeTrailingComma =
-    // Dynamic imports cannot have trailing commas
-    !(isDynamicImport || (node.callee && node.callee.type === "Import")) &&
-    shouldPrintComma(options, "all")
-      ? ","
-      : "";
+    !isDynamicImport && shouldPrintComma(options, "all") ? "," : "";
 
   function allArgsBrokenOut() {
     return group(
@@ -95,39 +94,50 @@ function printCallArguments(path, options, print) {
     return allArgsBrokenOut();
   }
 
-  const shouldGroupFirst = shouldGroupFirstArg(args);
-  const shouldGroupLast = shouldGroupLastArg(args, options);
-  if (shouldGroupFirst || shouldGroupLast) {
-    if (
-      shouldGroupFirst
-        ? printedArguments.slice(1).some(willBreak)
-        : printedArguments.slice(0, -1).some(willBreak)
-    ) {
+  if (shouldExpandFirstArg(args)) {
+    const tailArgs = printedArguments.slice(1);
+    if (tailArgs.some(willBreak)) {
       return allArgsBrokenOut();
     }
+    let firstArg;
+    try {
+      firstArg = print(getCallArgumentSelector(node, 0), {
+        expandFirstArg: true,
+      });
+    } catch (caught) {
+      if (caught instanceof ArgExpansionBailout) {
+        return allArgsBrokenOut();
+      }
+      /* istanbul ignore next */
+      throw caught;
+    }
+
+    if (willBreak(firstArg)) {
+      return [
+        breakParent,
+        conditionalGroup([
+          ["(", group(firstArg, { shouldBreak: true }), ", ", ...tailArgs, ")"],
+          allArgsBrokenOut(),
+        ]),
+      ];
+    }
 
-    // We want to print the last argument with a special flag
-    let printedExpanded = [];
+    return conditionalGroup([
+      ["(", firstArg, ", ", ...tailArgs, ")"],
+      ["(", group(firstArg, { shouldBreak: true }), ", ", ...tailArgs, ")"],
+      allArgsBrokenOut(),
+    ]);
+  }
 
+  if (shouldExpandLastArg(args, options)) {
+    const headArgs = printedArguments.slice(0, -1);
+    if (headArgs.some(willBreak)) {
+      return allArgsBrokenOut();
+    }
+    let lastArg;
     try {
-      iterateCallArgumentsPath(path, (argPath, i) => {
-        if (shouldGroupFirst && i === 0) {
-          printedExpanded = [
-            [
-              print([], { expandFirstArg: true }),
-              printedArguments.length > 1 ? "," : "",
-              hasEmptyLineFollowingFirstArg ? hardline : line,
-              hasEmptyLineFollowingFirstArg ? hardline : "",
-            ],
-            ...printedArguments.slice(1),
-          ];
-        }
-        if (shouldGroupLast && i === lastArgIndex) {
-          printedExpanded = [
-            ...printedArguments.slice(0, -1),
-            print([], { expandLastArg: true }),
-          ];
-        }
+      lastArg = print(getCallArgumentSelector(node, -1), {
+        expandLastArg: true,
       });
     } catch (caught) {
       if (caught instanceof ArgExpansionBailout) {
@@ -137,26 +147,21 @@ function printCallArguments(path, options, print) {
       throw caught;
     }
 
-    return [
-      printedArguments.some(willBreak) ? breakParent : "",
-      conditionalGroup([
-        ["(", ...printedExpanded, ")"],
-        shouldGroupFirst
-          ? [
-              "(",
-              group(printedExpanded[0], { shouldBreak: true }),
-              ...printedExpanded.slice(1),
-              ")",
-            ]
-          : [
-              "(",
-              ...printedArguments.slice(0, -1),
-              group(getLast(printedExpanded), { shouldBreak: true }),
-              ")",
-            ],
-        allArgsBrokenOut(),
-      ]),
-    ];
+    if (willBreak(lastArg)) {
+      return [
+        breakParent,
+        conditionalGroup([
+          ["(", ...headArgs, group(lastArg, { shouldBreak: true }), ")"],
+          allArgsBrokenOut(),
+        ]),
+      ];
+    }
+
+    return conditionalGroup([
+      ["(", ...headArgs, lastArg, ")"],
+      ["(", ...headArgs, group(lastArg, { shouldBreak: true }), ")"],
+      allArgsBrokenOut(),
+    ]);
   }
 
   const contents = [
@@ -177,14 +182,14 @@ function printCallArguments(path, options, print) {
   });
 }
 
-function couldGroupArg(arg, arrowChainRecursion = false) {
+function couldExpandArg(arg, arrowChainRecursion = false) {
   return (
     (arg.type === "ObjectExpression" &&
       (arg.properties.length > 0 || hasComment(arg))) ||
     (arg.type === "ArrayExpression" &&
       (arg.elements.length > 0 || hasComment(arg))) ||
-    (arg.type === "TSTypeAssertion" && couldGroupArg(arg.expression)) ||
-    (arg.type === "TSAsExpression" && couldGroupArg(arg.expression)) ||
+    (arg.type === "TSTypeAssertion" && couldExpandArg(arg.expression)) ||
+    (arg.type === "TSAsExpression" && couldExpandArg(arg.expression)) ||
     arg.type === "FunctionExpression" ||
     (arg.type === "ArrowFunctionExpression" &&
       // we want to avoid breaking inside composite return types but not simple keywords
@@ -205,7 +210,7 @@ function couldGroupArg(arg, arrowChainRecursion = false) {
         isNonEmptyBlockStatement(arg.body)) &&
       (arg.body.type === "BlockStatement" ||
         (arg.body.type === "ArrowFunctionExpression" &&
-          couldGroupArg(arg.body, true)) ||
+          couldExpandArg(arg.body, true)) ||
         arg.body.type === "ObjectExpression" ||
         arg.body.type === "ArrayExpression" ||
         (!arrowChainRecursion &&
@@ -217,13 +222,13 @@ function couldGroupArg(arg, arrowChainRecursion = false) {
   );
 }
 
-function shouldGroupLastArg(args, options) {
+function shouldExpandLastArg(args, options) {
   const lastArg = getLast(args);
   const penultimateArg = getPenultimate(args);
   return (
     !hasComment(lastArg, CommentCheckFlags.Leading) &&
     !hasComment(lastArg, CommentCheckFlags.Trailing) &&
-    couldGroupArg(lastArg) &&
+    couldExpandArg(lastArg) &&
     // If the last two arguments are of the same type,
     // disable last element expansion.
     (!penultimateArg || penultimateArg.type !== lastArg.type) &&
@@ -239,7 +244,7 @@ function shouldGroupLastArg(args, options) {
   );
 }
 
-function shouldGroupFirstArg(args) {
+function shouldExpandFirstArg(args) {
   if (args.length !== 2) {
     return false;
   }
@@ -261,10 +266,54 @@ function shouldGroupFirstArg(args) {
     secondArg.type !== "FunctionExpression" &&
     secondArg.type !== "ArrowFunctionExpression" &&
     secondArg.type !== "ConditionalExpression" &&
-    !couldGroupArg(secondArg)
+    isHopefullyShortCallArgument(secondArg) &&
+    !couldExpandArg(secondArg)
   );
 }
 
+// A hack to fix most manifestations of
+// https://github.com/prettier/prettier/issues/2456
+// https://github.com/prettier/prettier/issues/5172
+// https://github.com/prettier/prettier/issues/12892
+// A proper (printWidth-aware) fix for those would require a complex change in the doc printer.
+function isHopefullyShortCallArgument(node) {
+  if (node.type === "ParenthesizedExpression") {
+    return isHopefullyShortCallArgument(node.expression);
+  }
+
+  if (node.type === "TSAsExpression") {
+    let { typeAnnotation } = node;
+    if (typeAnnotation.type === "TSArrayType") {
+      typeAnnotation = typeAnnotation.elementType;
+      if (typeAnnotation.type === "TSArrayType") {
+        typeAnnotation = typeAnnotation.elementType;
+      }
+    }
+    if (
+      (typeAnnotation.type === "GenericTypeAnnotation" ||
+        typeAnnotation.type === "TSTypeReference") &&
+      typeAnnotation.typeParameters?.params.length === 1
+    ) {
+      typeAnnotation = typeAnnotation.typeParameters.params[0];
+    }
+    return (
+      isSimpleType(typeAnnotation) && isSimpleCallArgument(node.expression, 1)
+    );
+  }
+
+  if (isCallLikeExpression(node) && getCallArguments(node).length > 1) {
+    return false;
+  }
+
+  if (isBinaryish(node)) {
+    return (
+      isSimpleCallArgument(node.left, 1) && isSimpleCallArgument(node.right, 1)
+    );
+  }
+
+  return isRegExpLiteral(node) || isSimpleCallArgument(node);
+}
+
 function isReactHookCallWithDepsArray(args) {
   return (
     args.length === 2 &&

src/language-js/print/function.js

@@ -53,8 +53,7 @@ function printFunction(path, print, options, args) {
   if (
     (node.type === "FunctionDeclaration" ||
       node.type === "FunctionExpression") &&
-    args &&
-    args.expandLastArg
+    args?.expandLastArg
   ) {
     const parent = path.getParentNode();
     if (isCallExpression(parent) && getCallArguments(parent).length > 1) {
@@ -301,10 +300,7 @@ function printArrowFunction(path, options, print, args) {
       node.typeParameters ||
       getFunctionParameters(node).some((param) => param.type !== "Identifier");
 
-    if (
-      node.body.type !== "ArrowFunctionExpression" ||
-      (args && args.expandLastArg)
-    ) {
+    if (node.body.type !== "ArrowFunctionExpression" || args?.expandLastArg) {
       body.unshift(print("body", args));
     } else {
       node = node.body;
@@ -355,12 +351,12 @@ function printArrowFunction(path, options, print, args) {
   // with the opening (, or if it's inside a JSXExpression (e.g. an attribute)
   // we should align the expression's closing } with the line with the opening {.
   const shouldAddSoftLine =
-    ((args && args.expandLastArg) ||
+    (args?.expandLastArg ||
       path.getParentNode().type === "JSXExpressionContainer") &&
     !hasComment(node);
 
   const printTrailingComma =
-    args && args.expandLastArg && shouldPrintComma(options, "all");
+    args?.expandLastArg && shouldPrintComma(options, "all");
 
   // In order to avoid confusion between
   // a => a ? a : a