Don't skip ChainExpression in isCallExpression (#18654)

changelog_unreleased/typescript/18654.md

@@ -0,0 +1,53 @@
+#### Consistent print non-null assertions (#18654 by @fisker)
+
+<!-- prettier-ignore -->
+```tsx
+// Input
+const corners = baseCorners.map((corner) =>
+  curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset))!,
+);
+const corners = baseCorners.map((corner) =>
+  curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset)),
+);
+
+// Prettier stable
+const corners = baseCorners.map(
+  (corner) =>
+    curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset))!,
+);
+const corners = baseCorners.map((corner) =>
+  curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset)),
+);
+
+// Prettier main
+const corners = baseCorners.map((corner) =>
+  curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset))!,
+);
+const corners = baseCorners.map((corner) =>
+  curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset)),
+);
+```
+
+<!-- prettier-ignore -->
+```tsx
+// Input
+isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id));
+isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!);
+
+// Prettier stable
+isEqual(
+  a?.map(([t, _]) => t?.id)!,
+  b?.map(([t, _]) => t?.id),
+);
+isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!);
+
+// Prettier main
+isEqual(
+  a?.map(([t, _]) => t?.id)!,
+  b?.map(([t, _]) => t?.id),
+);
+isEqual(
+  a?.map(([t, _]) => t?.id)!,
+  b?.map(([t, _]) => t?.id)!,
+);
+```

src/language-js/parentheses/needs-parentheses.js

@@ -1263,6 +1263,7 @@ function canDecoratorExpressionUnparenthesized(node) {
   return (
     isDecoratorMemberExpression(node) ||
     (isCallExpression(node) &&
+      // @ts-expect-error -- doesn't exists on `CallExpression`
       !node.optional &&
       isDecoratorMemberExpression(node.callee))
   );

src/language-js/print/assignment.js

@@ -16,6 +16,7 @@ import {
   isBinaryish,
   isBooleanLiteral,
   isCallExpression,
+  isChainElementWrapper,
   isIntersectionType,
   isLoneShortArgument,
   isMemberExpression,
@@ -365,7 +366,7 @@ function isPoorlyBreakableMemberOrCallChain(
   const goDeeper = () =>
     isPoorlyBreakableMemberOrCallChain(path, options, print, true);
 
-  if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") {
+  if (isChainElementWrapper(node)) {
     return path.call(goDeeper, "expression");
   }
 

src/language-js/print/binaryish.js

@@ -243,6 +243,7 @@ function printBinaryishExpressions(
 
   const shouldInline = shouldInlineLogicalExpression(node);
   const rightNodeToCheckComments =
+    // TODO[@fisker]: Use `stripChainElementWrappers`
     node.right.type === "ChainExpression" ? node.right.expression : node.right;
   const lineBeforeOperator =
     (node.type === "NGPipeExpression" ||

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

@@ -33,6 +33,7 @@ import {
   isStringLiteral,
   iterateCallArgumentsPath,
   shouldPrintComma,
+  stripChainElementWrappers,
 } from "../utilities/index.js";
 import { isConciselyPrintedArray } from "./array.js";
 import { printDanglingCommentsInList } from "./miscellaneous.js";
@@ -201,26 +202,60 @@ function printCallArguments(path, options, print) {
 }
 
 function couldExpandArg(arg, arrowChainRecursion = false) {
-  return (
-    (isObjectExpression(arg) &&
-      (arg.properties.length > 0 || hasComment(arg))) ||
-    (isArrayExpression(arg) && (arg.elements.length > 0 || hasComment(arg))) ||
-    (arg.type === "TSTypeAssertion" && couldExpandArg(arg.expression)) ||
-    (isBinaryCastExpression(arg) && couldExpandArg(arg.expression)) ||
+  if (
+    isObjectExpression(arg) &&
+    (arg.properties.length > 0 || hasComment(arg))
+  ) {
+    return true;
+  }
+
+  if (isArrayExpression(arg) && (arg.elements.length > 0 || hasComment(arg))) {
+    return true;
+  }
+
+  if (
+    (isBinaryCastExpression(arg) || arg.type === "TSTypeAssertion") &&
+    couldExpandArg(arg.expression)
+  ) {
+    return true;
+  }
+
+  if (
     arg.type === "FunctionExpression" ||
-    (arg.type === "ArrowFunctionExpression" &&
-      (arg.body.type === "BlockStatement" ||
-        (arg.body.type === "ArrowFunctionExpression" &&
-          couldExpandArg(arg.body, true)) ||
-        isObjectExpression(arg.body) ||
-        isArrayExpression(arg.body) ||
-        (!arrowChainRecursion &&
-          (isCallExpression(arg.body) ||
-            arg.body.type === "ConditionalExpression")) ||
-        isJsxElement(arg.body))) ||
     arg.type === "DoExpression" ||
     arg.type === "ModuleExpression"
-  );
+  ) {
+    return true;
+  }
+
+  if (arg.type === "ArrowFunctionExpression") {
+    const { body } = arg;
+
+    if (
+      body.type === "BlockStatement" ||
+      isJsxElement(body) ||
+      isObjectExpression(body) ||
+      isArrayExpression(body)
+    ) {
+      return true;
+    }
+
+    if (body.type === "ArrowFunctionExpression" && couldExpandArg(body, true)) {
+      return true;
+    }
+
+    if (!arrowChainRecursion) {
+      if (body.type === "ConditionalExpression") {
+        return true;
+      }
+
+      if (isCallExpression(stripChainElementWrappers(body))) {
+        return true;
+      }
+    }
+  }
+
+  return false;
 }
 
 function shouldExpandLastArg(args, argDocs, options) {

src/language-js/print/class.js

@@ -18,6 +18,7 @@ import {
   CommentCheckFlags,
   createTypeCheckFunction,
   hasComment,
+  isChainElementWrapper,
   isMemberExpression,
 } from "../utilities/index.js";
 import { printAssignment } from "./assignment.js";
@@ -169,10 +170,8 @@ function hasMultipleHeritage(node) {
 }
 
 function isMemberish(node) {
-  if (
-    node?.type === "ChainExpression" ||
-    node?.type === "TSNonNullExpression"
-  ) {
+  // TODO[@fisker]: Should use `stripChainElementWrappers` instead, since it can be in multiple wrappers
+  if (isChainElementWrapper(node)) {
     return isMemberish(node.expression);
   }
 

src/language-js/print/jsx.js

@@ -588,6 +588,7 @@ function printJsxExpressionContainer(path, options, print) {
           (shouldInline(node.argument, node) ||
             node.argument.type === "JSXElement")) ||
         isCallExpression(node) ||
+        // TODO[@fisker]: Use `stripChainElementWrappers`
         (node.type === "ChainExpression" &&
           isCallExpression(node.expression)) ||
         node.type === "FunctionExpression" ||

src/language-js/print/member-chain.js

@@ -47,6 +47,7 @@ import { printOptionalToken } from "./miscellaneous.js";
 // MemberExpression and CallExpression. We need to traverse the AST
 // and make groups out of it to print it in the desired way.
 function printMemberChain(path, options, print) {
+  // TODO[@fisker]: skip `TSNonNullExpression`
   /* c8 ignore next 6 */
   if (path.node.type === "ChainExpression") {
     return path.call(

src/language-js/print/member.js

@@ -8,13 +8,15 @@ import {
 import {
   getCallArguments,
   isCallExpression,
+  isChainElementWrapper,
   isMemberExpression,
   isNumericLiteral,
 } from "../utilities/index.js";
 import { printOptionalToken } from "./miscellaneous.js";
 
 const isCallExpressionWithArguments = (node) => {
-  if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") {
+  // TODO[@fisker]: Use `stripChainElementWrappers`
+  if (isChainElementWrapper(node)) {
     node = node.expression;
   }
   return isCallExpression(node) && getCallArguments(node).length > 0;
@@ -48,8 +50,7 @@ function printMemberExpression(path, options, print) {
       !(isMemberExpression(node) || node.type === "TSNonNullExpression"),
   );
   const firstNonChainElementWrapperParent = path.findAncestor(
-    (node) =>
-      !(node.type === "ChainExpression" || node.type === "TSNonNullExpression"),
+    (node) => !isChainElementWrapper(node),
   );
 
   const shouldInline =

src/language-js/print/ternary-old.js

@@ -10,6 +10,7 @@ import {
 import {
   isBinaryCastExpression,
   isCallExpression,
+  isChainElementWrapper,
   isJsxElement,
   isMemberExpression,
 } from "../utilities/index.js";
@@ -146,9 +147,7 @@ function shouldExtraIndentForConditionalExpression(path) {
     const node = path.getParentNode(ancestorCount);
 
     if (
-      ((node.type === "ChainExpression" ||
-        node.type === "TSNonNullExpression") &&
-        node.expression === child) ||
+      (isChainElementWrapper(node) && node.expression === child) ||
       (isCallExpression(node) && node.callee === child) ||
       (isMemberExpression(node) && node.object === child)
     ) {

src/language-js/print/ternary.js

@@ -18,6 +18,7 @@ import {
   hasComment,
   isBinaryCastExpression,
   isCallExpression,
+  isChainElementWrapper,
   isConditionalType,
   isJsxElement,
   isLoneShortArgument,
@@ -103,9 +104,7 @@ function shouldExtraIndentForConditionalExpression(path) {
     const node = path.getParentNode(ancestorCount);
 
     if (
-      ((node.type === "ChainExpression" ||
-        node.type === "TSNonNullExpression") &&
-        node.expression === child) ||
+      (isChainElementWrapper(node) && node.expression === child) ||
       (isCallExpression(node) && node.callee === child) ||
       (isMemberExpression(node) && node.object === child)
     ) {

src/language-js/utilities/index.js

@@ -64,8 +64,7 @@ function hasNakedLeftSide(node) {
     node.type === "BindExpression" ||
     (node.type === "UpdateExpression" && !node.prefix) ||
     isBinaryCastExpression(node) ||
-    node.type === "TSNonNullExpression" ||
-    node.type === "ChainExpression"
+    isChainElementWrapper(node)
   );
 }
 
@@ -364,21 +363,10 @@ function isTestCall(node, parent) {
   return false;
 }
 
-/**
-@template {ReturnType<createTypeCheckFunction>} TypeCheckFunction
-@param {TypeCheckFunction} fn
-@return {(node: Node) => boolean} */
-const skipChainExpression = (fn) => (node) => {
-  if (node?.type === "ChainExpression") {
-    node = node.expression;
-  }
-
-  return fn(node);
-};
-
-const isCallExpression = skipChainExpression(
-  createTypeCheckFunction(["CallExpression", "OptionalCallExpression"]),
-);
+const isCallExpression = createTypeCheckFunction([
+  "CallExpression",
+  "OptionalCallExpression",
+]);
 
 const isMemberExpression = createTypeCheckFunction([
   "MemberExpression",
@@ -524,16 +512,19 @@ function isFunctionCompositionArgs(args) {
     return false;
   }
   let count = 0;
-  for (const arg of args) {
+  for (let arg of args) {
     if (isFunctionOrArrowExpression(arg)) {
       count += 1;
       if (count > 1) {
         return true;
       }
-    } else if (isCallExpression(arg)) {
-      for (const childArg of getCallArguments(arg)) {
-        if (isFunctionOrArrowExpression(childArg)) {
-          return true;
+    } else {
+      arg = stripChainElementWrappers(arg);
+      if (isCallExpression(arg)) {
+        for (const childArg of getCallArguments(arg)) {
+          if (isFunctionOrArrowExpression(childArg)) {
+            return true;
+          }
         }
       }
     }
@@ -574,12 +565,10 @@ function isSimpleCallArgument(node, depth = 2) {
     return false;
   }
 
-  if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") {
-    return isSimpleCallArgument(node.expression, depth);
-  }
-
   const isChildSimple = (child) => isSimpleCallArgument(child, depth - 1);
 
+  node = stripChainElementWrappers(node);
+
   if (isRegExpLiteral(node)) {
     return getStringWidth(node.pattern ?? node.regex.pattern) <= 5;
   }
@@ -705,9 +694,9 @@ function startsWithNoLookaheadToken(node, predicate) {
     case "SequenceExpression":
       return startsWithNoLookaheadToken(node.expressions[0], predicate);
     case "ChainExpression":
+    case "TSNonNullExpression":
     case "TSSatisfiesExpression":
     case "TSAsExpression":
-    case "TSNonNullExpression":
     case "AsExpression":
     case "AsConstExpression":
     case "SatisfiesExpression":
@@ -856,10 +845,6 @@ function getCallArguments(node) {
     return callArgumentsCache.get(node);
   }
 
-  if (node.type === "ChainExpression") {
-    return getCallArguments(node.expression);
-  }
-
   let args;
   if (node.type === "ImportExpression" || node.type === "TSImportType") {
     args = [node.source];
@@ -880,13 +865,6 @@ function getCallArguments(node) {
 function iterateCallArgumentsPath(path, iteratee) {
   const { node } = path;
 
-  if (node.type === "ChainExpression") {
-    return path.call(
-      () => iterateCallArgumentsPath(path, iteratee),
-      "expression",
-    );
-  }
-
   if (node.type === "ImportExpression" || node.type === "TSImportType") {
     path.call(() => iteratee(path, 0), "source");
 
@@ -901,30 +879,24 @@ function iterateCallArgumentsPath(path, iteratee) {
 }
 
 function getCallArgumentSelector(node, index) {
-  const selectors = [];
-  if (node.type === "ChainExpression") {
-    node = node.expression;
-    selectors.push("expression");
-  }
-
   if (node.type === "ImportExpression" || node.type === "TSImportType") {
     if (index === 0 || index === (node.options ? -2 : -1)) {
-      return [...selectors, "source"];
+      return ["source"];
     }
     if (node.options && (index === 1 || index === -1)) {
-      return [...selectors, "options"];
+      return ["options"];
     }
     throw new RangeError("Invalid argument index");
   } else if (node.type === "TSExternalModuleReference") {
     if (index === 0 || index === -1) {
-      return [...selectors, "expression"];
+      return ["expression"];
     }
   } else {
     if (index < 0) {
       index = node.arguments.length + index;
     }
     if (index >= 0 && index < node.arguments.length) {
-      return [...selectors, "arguments", index];
+      return ["arguments", index];
     }
   }
 
@@ -1114,6 +1086,23 @@ function isIifeCalleeOrTaggedTemplateExpressionTag(path) {
   );
 }
 
+const isChainElementWrapper = createTypeCheckFunction([
+  "ChainExpression",
+  "TSNonNullExpression",
+]);
+
+/**
+@param {Node} node
+@returns {Exclude<Node, NodeMap["ChainExpression"] | NodeMap["TSNonNullExpression"]>}
+*/
+function stripChainElementWrappers(node) {
+  while (isChainElementWrapper(node)) {
+    node = node.expression;
+  }
+
+  return node;
+}
+
 export {
   CommentCheckFlags,
   createTypeCheckFunction,
@@ -1138,6 +1127,7 @@ export {
   isBooleanTypeCoercion,
   isCallExpression,
   isCallLikeExpression,
+  isChainElementWrapper,
   isConditionalType,
   isExportDeclaration,
   isFlowObjectTypePropertyAFunction,
@@ -1181,6 +1171,7 @@ export {
   shouldPrintComma,
   shouldUnionTypePrintOwnComments,
   startsWithNoLookaheadToken,
+  stripChainElementWrappers,
 };
 export { default as isMeaningfulEmptyStatement } from "./is-meaningful-empty-statement.js";
 export { default as isNodeMatches } from "./is-node-matches.js";

tests/format/js/chain-expression/__snapshots__/format.test.js.snap

@@ -61,19 +61,13 @@ printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
 function someFunctionName(
-  someLongBreakingParameterName,
-  anotherLongParameterName,
 ) {
   return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id));
   return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id));
 }
 
-
 =====================================output=====================================
-function someFunctionName(
-  someLongBreakingParameterName,
-  anotherLongParameterName,
-) {
+function someFunctionName() {
   return isEqual(
     a?.map(([t, _]) => t?.id),
     b?.map(([t, _]) => t?.id),

tests/format/js/chain-expression/issue-15785-1.js

@@ -1,8 +1,5 @@
 function someFunctionName(
-  someLongBreakingParameterName,
-  anotherLongParameterName,
 ) {
   return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id));
   return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id));
 }
-

tests/format/typescript/chain-expression/__snapshots__/format.test.js.snap

@@ -140,6 +140,73 @@ foo(a?.b!);
 ================================================================================
 `;
 
+exports[`issue-15785-1.ts format 1`] = `
+====================================options=====================================
+parsers: ["typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+function someFunctionName(
+) {
+  return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id));
+  return isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!);
+  return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id));
+  return isEqual(a.map(([t, _]) => t?.id)!, b.map(([t, _]) => t?.id)!);
+}
+
+=====================================output=====================================
+function someFunctionName() {
+  return isEqual(
+    a?.map(([t, _]) => t?.id),
+    b?.map(([t, _]) => t?.id),
+  );
+  return isEqual(
+    a?.map(([t, _]) => t?.id)!,
+    b?.map(([t, _]) => t?.id)!,
+  );
+  return isEqual(
+    a.map(([t, _]) => t?.id),
+    b.map(([t, _]) => t?.id),
+  );
+  return isEqual(
+    a.map(([t, _]) => t?.id)!,
+    b.map(([t, _]) => t?.id)!,
+  );
+}
+
+================================================================================
+`;
+
+exports[`issue-15785-2.ts format 1`] = `
+====================================options=====================================
+parsers: ["typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+const theValue = Object.entries(someLongObjectName).filter(
+  ([listingId]) => someListToCompareToHere?.includes(listingId),
+);
+const theValue2 = Object.entries(someLongObjectName).filter(
+  ([listingId]) => someListToCompareToHere.includes(listingId),
+);
+const theValue3 = Object.entries(someLongObjectName).filter(
+  ([listingId]) => someListToCompareToHere.includes(listingId)!,
+);
+
+=====================================output=====================================
+const theValue = Object.entries(someLongObjectName).filter(([listingId]) =>
+  someListToCompareToHere?.includes(listingId),
+);
+const theValue2 = Object.entries(someLongObjectName).filter(([listingId]) =>
+  someListToCompareToHere.includes(listingId),
+);
+const theValue3 = Object.entries(someLongObjectName).filter(([listingId]) =>
+  someListToCompareToHere.includes(listingId)!,
+);
+
+================================================================================
+`;
+
 exports[`member-expression.ts format 1`] = `
 ====================================options=====================================
 parsers: ["typescript"]

tests/format/typescript/chain-expression/issue-15785-1.ts

@@ -0,0 +1,7 @@
+function someFunctionName(
+) {
+  return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id));
+  return isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!);
+  return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id));
+  return isEqual(a.map(([t, _]) => t?.id)!, b.map(([t, _]) => t?.id)!);
+}

tests/format/typescript/chain-expression/issue-15785-2.ts

@@ -0,0 +1,9 @@
+const theValue = Object.entries(someLongObjectName).filter(
+  ([listingId]) => someListToCompareToHere?.includes(listingId),
+);
+const theValue2 = Object.entries(someLongObjectName).filter(
+  ([listingId]) => someListToCompareToHere.includes(listingId),
+);
+const theValue3 = Object.entries(someLongObjectName).filter(
+  ([listingId]) => someListToCompareToHere.includes(listingId)!,
+);

tests/format/typescript/non-null/__snapshots__/format.test.js.snap

@@ -58,6 +58,11 @@ this.foo.get("bar")!.doThings().more();
 
 foo!.bar().baz().what();
 
+xyz.a(b).a(b).a(b)
+xyz.a(b!).a(b!).a(b!)
+xyz.a(/a/).a(/a/).a(/a/)
+xyz.a(/a/!).a(/a/!).a(/a/!)
+
 =====================================output=====================================
 const { somePropThatHasAReallyLongName, anotherPropThatHasALongName } =
   this.props.imReallySureAboutThis!;
@@ -69,6 +74,11 @@ this.foo.get("bar")!.doThings().more();
 
 foo!.bar().baz().what();
 
+xyz.a(b).a(b).a(b);
+xyz.a(b!).a(b!).a(b!);
+xyz.a(/a/).a(/a/).a(/a/);
+xyz.a(/a/!).a(/a/!).a(/a/!);
+
 ================================================================================
 `;
 

tests/format/typescript/non-null/member-chain.ts

@@ -5,3 +5,8 @@ const { somePropThatHasAReallyLongName2, anotherPropThatHasALongName2 } = this.p
 this.foo.get("bar")!.doThings().more();
 
 foo!.bar().baz().what();
+
+xyz.a(b).a(b).a(b)
+xyz.a(b!).a(b!).a(b!)
+xyz.a(/a/).a(/a/).a(/a/)
+xyz.a(/a/!).a(/a/!).a(/a/!)