Review ChainExpression check (#18659)

changelog_unreleased/typescript/18652.md

@@ -1,4 +1,4 @@
-#### Fix inconsistent formatting of class superClass (#18652 by @fisker)
+#### Fix inconsistent formatting of class superClass (#18652, #18659 by @fisker)
 
 <!-- prettier-ignore -->
 ```tsx
@@ -18,5 +18,5 @@ class ALongLongLongLongLongLongLongLongLongLongLongLongClassName extends (foo
 class ALongLongLongLongLongLongLongLongLongLongLongLongClassName
   extends foo.bar.Baz {}
 class ALongLongLongLongLongLongLongLongLongLongLongLongClassName
-  extends (foo.bar.Baz!) {}
+  extends foo.bar.Baz! {}
 ```

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

@@ -22,6 +22,7 @@ import {
   isUnionType,
   shouldFlatten,
   startsWithNoLookaheadToken,
+  stripChainElementWrappers,
 } from "../utilities/index.js";
 import { returnArgumentHasLeadingComment } from "../utilities/return-statement-has-leading-comment.js";
 import { shouldAddParenthesesToIdentifier } from "./identifier.js";
@@ -111,25 +112,27 @@ function needsParentheses(path, options) {
     case "ClassExpression":
       // Add parens around the extends clause of a class. It is needed for almost
       // all expressions.
-      if (
-        key === "superClass" &&
-        (node.type === "ArrowFunctionExpression" ||
-          node.type === "AssignmentExpression" ||
-          node.type === "AwaitExpression" ||
-          node.type === "BinaryExpression" ||
-          node.type === "ConditionalExpression" ||
-          node.type === "LogicalExpression" ||
-          node.type === "NewExpression" ||
-          node.type === "ObjectExpression" ||
-          node.type === "SequenceExpression" ||
-          node.type === "TaggedTemplateExpression" ||
-          node.type === "UnaryExpression" ||
-          node.type === "UpdateExpression" ||
-          node.type === "YieldExpression" ||
-          node.type === "TSNonNullExpression" ||
-          (node.type === "ClassExpression" && isNonEmptyArray(node.decorators)))
-      ) {
-        return true;
+      if (key === "superClass") {
+        const superClass = stripChainElementWrappers(node);
+        if (
+          superClass.type === "ArrowFunctionExpression" ||
+          superClass.type === "AssignmentExpression" ||
+          superClass.type === "AwaitExpression" ||
+          superClass.type === "BinaryExpression" ||
+          superClass.type === "ConditionalExpression" ||
+          superClass.type === "LogicalExpression" ||
+          superClass.type === "NewExpression" ||
+          superClass.type === "ObjectExpression" ||
+          superClass.type === "SequenceExpression" ||
+          superClass.type === "TaggedTemplateExpression" ||
+          superClass.type === "UnaryExpression" ||
+          superClass.type === "UpdateExpression" ||
+          superClass.type === "YieldExpression" ||
+          (superClass.type === "ClassExpression" &&
+            isNonEmptyArray(superClass.decorators))
+        ) {
+          return true;
+        }
       }
       break;
 

src/language-js/print/binaryish.js

@@ -243,7 +243,6 @@ 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/class.js

@@ -18,8 +18,8 @@ import {
   CommentCheckFlags,
   createTypeCheckFunction,
   hasComment,
-  isChainElementWrapper,
   isMemberExpression,
+  stripChainElementWrappers,
 } from "../utilities/index.js";
 import { printAssignment } from "./assignment.js";
 import { printClassMemberDecorators } from "./decorators.js";
@@ -169,15 +169,6 @@ function hasMultipleHeritage(node) {
   return count > 1;
 }
 
-function isMemberish(node) {
-  // TODO[@fisker]: Should use `stripChainElementWrappers` instead, since it can be in multiple wrappers
-  if (isChainElementWrapper(node)) {
-    return isMemberish(node.expression);
-  }
-
-  return isMemberExpression(node);
-}
-
 /**
 @returns {boolean}
 */
@@ -197,7 +188,10 @@ function shouldPrintClassInGroupModeWithoutCache(path) {
       return false;
     }
 
-    return !node.superTypeArguments && isMemberish(node.superClass);
+    return (
+      !node.superTypeArguments &&
+      isMemberExpression(stripChainElementWrappers(node.superClass))
+    );
   }
 
   const heritage =

src/language-js/print/jsx.js

@@ -34,6 +34,7 @@ import {
   isJsxElement,
   isObjectExpression,
   isStringLiteral,
+  stripChainElementWrappers,
 } from "../utilities/index.js";
 
 /**
@@ -587,10 +588,7 @@ function printJsxExpressionContainer(path, options, print) {
         (node.type === "AwaitExpression" &&
           (shouldInline(node.argument, node) ||
             node.argument.type === "JSXElement")) ||
-        isCallExpression(node) ||
-        // TODO[@fisker]: Use `stripChainElementWrappers`
-        (node.type === "ChainExpression" &&
-          isCallExpression(node.expression)) ||
+        isCallExpression(stripChainElementWrappers(node)) ||
         node.type === "FunctionExpression" ||
         node.type === "TemplateLiteral" ||
         node.type === "TaggedTemplateExpression" ||

src/language-js/print/member.js

@@ -11,16 +11,12 @@ import {
   isChainElementWrapper,
   isMemberExpression,
   isNumericLiteral,
+  stripChainElementWrappers,
 } from "../utilities/index.js";
 import { printOptionalToken } from "./miscellaneous.js";
 
-const isCallExpressionWithArguments = (node) => {
-  // TODO[@fisker]: Use `stripChainElementWrappers`
-  if (isChainElementWrapper(node)) {
-    node = node.expression;
-  }
-  return isCallExpression(node) && getCallArguments(node).length > 0;
-};
+const isCallExpressionWithArguments = (node) =>
+  isCallExpression(node) && getCallArguments(node).length > 0;
 
 function shouldInlineNewExpressionCallee(path) {
   let { node: child, ancestors } = path;
@@ -64,7 +60,7 @@ function printMemberExpression(path, options, print) {
       !isMemberExpression(firstNonChainElementWrapperParent)) ||
     ((firstNonChainElementWrapperParent.type === "AssignmentExpression" ||
       firstNonChainElementWrapperParent.type === "VariableDeclarator") &&
-      (isCallExpressionWithArguments(node.object) ||
+      (isCallExpressionWithArguments(stripChainElementWrappers(node.object)) ||
         objectDoc.label?.memberChain));
 
   return label(objectDoc.label, [

tests/format/typescript/assignment/__snapshots__/format.test.js.snap

@@ -500,7 +500,16 @@ printWidth: 80
 =====================================input======================================
 if (true) {
   if (condition) {
-    const secondType = sourceCode.getNodeByRangeIndex1234(second.range[0])!
+    const secondType1 = sourceCode.getNodeByRangeIndex1234(second.range[0])!
+      .type;
+    const secondType2 = (sourceCode?.getNodeByRangeIndex1234(second.range[0]))
+      .type;
+    const secondType3 = (sourceCode?.getNodeByRangeIndex1234(second.range[0])!)
+      .type;
+    // TODO[@fisker]: Fix
+    // const secondType4 = (sourceCode?.getNodeByRangeIndex1234(second.range[0])!)!
+    //   .type;
+    const secondType5 = sourceCode.getNodeByRangeIndex1234(second.range[0])!!
       .type;
   }
 }
@@ -508,9 +517,21 @@ if (true) {
 =====================================output=====================================
 if (true) {
   if (condition) {
-    const secondType = sourceCode.getNodeByRangeIndex1234(
+    const secondType1 = sourceCode.getNodeByRangeIndex1234(
       second.range[0],
     )!.type;
+    const secondType2 = (sourceCode?.getNodeByRangeIndex1234(
+      second.range[0],
+    )).type;
+    const secondType3 = (sourceCode?.getNodeByRangeIndex1234(
+      second.range[0],
+    ))!.type;
+    // TODO[@fisker]: Fix
+    // const secondType4 = (sourceCode?.getNodeByRangeIndex1234(second.range[0])!)!
+    //   .type;
+    const secondType5 = sourceCode.getNodeByRangeIndex1234(
+      second.range[0],
+    )!!.type;
   }
 }
 

tests/format/typescript/assignment/lone-arg.ts

@@ -1,6 +1,15 @@
 if (true) {
   if (condition) {
-    const secondType = sourceCode.getNodeByRangeIndex1234(second.range[0])!
+    const secondType1 = sourceCode.getNodeByRangeIndex1234(second.range[0])!
+      .type;
+    const secondType2 = (sourceCode?.getNodeByRangeIndex1234(second.range[0]))
+      .type;
+    const secondType3 = (sourceCode?.getNodeByRangeIndex1234(second.range[0])!)
+      .type;
+    // TODO[@fisker]: Fix
+    // const secondType4 = (sourceCode?.getNodeByRangeIndex1234(second.range[0])!)!
+    //   .type;
+    const secondType5 = sourceCode.getNodeByRangeIndex1234(second.range[0])!!
       .type;
   }
 }

tests/format/typescript/binary-expressions/__snapshots__/format.test.js.snap

@@ -0,0 +1,47 @@
+// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
+
+exports[`chain-expression.ts format 1`] = `
+====================================options=====================================
+parsers: ["typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+if (
+  true
+  // This is a really complicated part of the condition, so we need a big ol'
+  // comment here to explain it.
+  && flibble.blibble.blobble?.bloo
+) {
+  doThings();
+}
+
+if (
+  true
+  // This is a really complicated part of the condition, so we need a big ol'
+  // comment here to explain it.
+  && flibble.blibble.blobble?.bloo!
+) {
+  doThings();
+}
+
+=====================================output=====================================
+if (
+  true &&
+  // This is a really complicated part of the condition, so we need a big ol'
+  // comment here to explain it.
+  flibble.blibble.blobble?.bloo
+) {
+  doThings();
+}
+
+if (
+  true &&
+  // This is a really complicated part of the condition, so we need a big ol'
+  // comment here to explain it.
+  flibble.blibble.blobble?.bloo!
+) {
+  doThings();
+}
+
+================================================================================
+`;

tests/format/typescript/binary-expressions/chain-expression.ts

@@ -0,0 +1,17 @@
+if (
+  true
+  // This is a really complicated part of the condition, so we need a big ol'
+  // comment here to explain it.
+  && flibble.blibble.blobble?.bloo
+) {
+  doThings();
+}
+
+if (
+  true
+  // This is a really complicated part of the condition, so we need a big ol'
+  // comment here to explain it.
+  && flibble.blibble.blobble?.bloo!
+) {
+  doThings();
+}

tests/format/typescript/binary-expressions/format.test.js

@@ -0,0 +1 @@
+runFormatTest(import.meta, ["typescript"]);

tests/format/typescript/class-and-interface/heritage-break/__snapshots__/format.test.js.snap

@@ -17,6 +17,10 @@ class A_long_long_long_long_long_long_long_long_name12
 
 class A_long_long_long_long_long_long_long_long_name12
   extends eslint.Rule.RuleModule! {}
+class A_long_long_long_long_long_long_long_long_name12
+  extends eslint?.Rule.RuleModule! {}
+class A_long_long_long_long_long_long_long_long_name12
+  extends eslint.Rule.RuleModule!! {}
 
 interface A_long_long_long_long_long_long_long_long_name2
   extends eslint.Rule.RuleModule {}
@@ -38,7 +42,11 @@ class A_long_long_long_long_long_long_long_long_name12
   extends eslint.Rule?.RuleModule {}
 
 class A_long_long_long_long_long_long_long_long_name12
-  extends (eslint.Rule.RuleModule!) {}
+  extends eslint.Rule.RuleModule! {}
+class A_long_long_long_long_long_long_long_long_name12
+  extends eslint?.Rule.RuleModule! {}
+class A_long_long_long_long_long_long_long_long_name12
+  extends eslint.Rule.RuleModule!! {}
 
 interface A_long_long_long_long_long_long_long_long_name2
   extends eslint.Rule.RuleModule {}

tests/format/typescript/class-and-interface/heritage-break/member-expression-like.ts

@@ -9,6 +9,10 @@ class A_long_long_long_long_long_long_long_long_name12
 
 class A_long_long_long_long_long_long_long_long_name12
   extends eslint.Rule.RuleModule! {}
+class A_long_long_long_long_long_long_long_long_name12
+  extends eslint?.Rule.RuleModule! {}
+class A_long_long_long_long_long_long_long_long_name12
+  extends eslint.Rule.RuleModule!! {}
 
 interface A_long_long_long_long_long_long_long_long_name2
   extends eslint.Rule.RuleModule {}

tests/format/typescript/tsx/__snapshots__/format.test.js.snap

@@ -126,6 +126,74 @@ const footer = "<footer></footer>";
 ================================================================================
 `;
 
+exports[`optional-chainning.tsx format 1`] = `
+====================================options=====================================
+parsers: ["typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa?.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )}
+    </div>
+  );
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa?.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )!}
+    </div>
+  );
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )}
+    </div>
+  );
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )!}
+    </div>
+  );
+
+=====================================output=====================================
+a = (
+  <div>
+    {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa?.bbbbbbbbbbbbbbbbbbbbbbbbb.map((p, i) => (
+      <p key={i}>{p}</p>
+    ))}
+  </div>
+);
+a = (
+  <div>
+    {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa?.bbbbbbbbbbbbbbbbbbbbbbbbb.map((p, i) => (
+      <p key={i}>{p}</p>
+    ))!}
+  </div>
+);
+a = (
+  <div>
+    {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbb.map((p, i) => (
+      <p key={i}>{p}</p>
+    ))}
+  </div>
+);
+a = (
+  <div>
+    {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbb.map((p, i) => (
+      <p key={i}>{p}</p>
+    ))!}
+  </div>
+);
+
+================================================================================
+`;
+
 exports[`react.tsx format 1`] = `
 ====================================options=====================================
 parsers: ["typescript"]

tests/format/typescript/tsx/optional-chainning.tsx

@@ -0,0 +1,28 @@
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa?.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )}
+    </div>
+  );
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa?.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )!}
+    </div>
+  );
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )}
+    </div>
+  );
+a = (
+    <div>
+      {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbb.map(
+        (p, i) => <p key={i}>{p}</p>
+      )!}
+    </div>
+  );