Avoid double parentheses around logical expression (#18397)

changelog_unreleased/javascript/18397.md

@@ -0,0 +1,13 @@
+#### Avoid double parentheses around logical expression (#18397 by @fisker)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+const a = !(x || y /* foo */);
+
+// Prettier stable
+const a = !((x || y) /* foo */);
+
+// Prettier main
+const a = !(x || y /* foo */);
+```

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

@@ -4,6 +4,7 @@ import {
   getFunctionParameters,
   getLeftSidePathName,
   getPrecedence,
+  hasComment,
   hasNakedLeftSide,
   hasNode,
   isArrayExpression,
@@ -380,7 +381,6 @@ function needsParentheses(path, options) {
 
         case "TSTypeAssertion":
         case "TaggedTemplateExpression":
-        case "UnaryExpression":
         case "JSXSpreadAttribute":
         case "SpreadElement":
         case "BindExpression":
@@ -388,6 +388,12 @@ function needsParentheses(path, options) {
         case "TSNonNullExpression":
         case "UpdateExpression":
           return true;
+        case "UnaryExpression":
+          // `UnaryExpression` adds parentheses and indention when argument has comment
+          if (!hasComment(node)) {
+            return true;
+          }
+          break;
 
         case "MemberExpression":
         case "OptionalMemberExpression":
@@ -451,6 +457,7 @@ function needsParentheses(path, options) {
         default:
           return false;
       }
+      break;
 
     case "SequenceExpression":
       // Although parentheses wouldn't hurt around sequence

src/language-js/print/binaryish.js

@@ -85,7 +85,8 @@ function printBinaryishExpression(path, options, print) {
   if (
     (key === "callee" &&
       (isCallExpression(parent) || parent.type === "NewExpression")) ||
-    parent.type === "UnaryExpression" ||
+    // `UnaryExpression` adds parentheses and indention when argument has comment
+    (parent.type === "UnaryExpression" && !hasComment(node)) ||
     (isMemberExpression(parent) && !parent.computed)
   ) {
     return group([indent([softline, ...parts]), softline]);
@@ -112,6 +113,7 @@ function printBinaryishExpression(path, options, print) {
       !isCallExpression(grandparent) &&
       grandparent.type !== "NewExpression") ||
     parent.type === "TemplateLiteral" ||
+    (key === "argument" && parent.type === "UnaryExpression") ||
     isBooleanTypeCoercion(path);
 
   const shouldIndentIfInlining =

src/language-js/print/estree.js

@@ -292,12 +292,14 @@ function printEstree(path, options, print, args) {
         parts.push(" ");
       }
 
+      const argumentDoc = print("argument");
+
       if (hasComment(node.argument)) {
         parts.push(
-          group(["(", indent([softline, print("argument")]), softline, ")"]),
+          group(["(", indent([softline, argumentDoc]), softline, ")"]),
         );
       } else {
-        parts.push(print("argument"));
+        parts.push(argumentDoc);
       }
 
       return parts;

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

@@ -81,11 +81,9 @@ void(
 
 =====================================output=====================================
 !(
-  (
-    x || // foo
-    y || // bar
-    z
-  ) /*
+  x || // foo
+  y || // bar
+  z /*
    * comment
    */
 )
@@ -115,12 +113,10 @@ void(
 )
 
 void (
-  (
-    (p && // first
-      q) || // second
-    (r && // third
-      s)
-  ) // fourth
+  (p && // first
+    q) || // second
+  (r && // third
+    s) // fourth
 )
 
 void (
@@ -241,11 +237,9 @@ void(
 
 =====================================output=====================================
 !(
-  (
-    x || // foo
-    y || // bar
-    z
-  ) /*
+  x || // foo
+  y || // bar
+  z /*
    * comment
    */
 );
@@ -275,12 +269,10 @@ void(
 );
 
 void (
-  (
-    (p && // first
-      q) || // second
-    (r && // third
-      s)
-  ) // fourth
+  (p && // first
+    q) || // second
+  (r && // third
+    s) // fourth
 );
 
 void (

tests/format/js/logical-expressions/__snapshots__/format.test.js.snap

@@ -1,5 +1,183 @@
 // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
 
+exports[`in-unary-expression.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+
+
+// https://github.com/estree/estree/blob/96fee942ecc2b3b9d3c34163ec142b75daf4cca1/es5.md#unaryoperator
+[
+!(
+  //
+  octahedralAnteaterGenerator ||
+  //
+  stellatedFossilProcessor
+),
++(
+  //
+  octahedralAnteaterGenerator &&
+  //
+  stellatedFossilProcessor
+),
+typeof (
+  //
+  octahedralAnteaterGenerator ??
+  //
+  stellatedFossilProcessor
+),
+];
+
+// Realworld case from Prettier repo
+if (
+  !(
+    // \`import("foo")\`
+    (
+      node.type === "ImportExpression" ||
+      // \`type foo = import("foo")\`
+      node.type === "TSImportType" ||
+      // \`import type A = require("foo")\`
+      node.type === "TSExternalModuleReference" ||
+      // \`require("foo")\`
+      // \`require.resolve("foo")\`
+      // \`require.resolve.paths("foo")\`
+      // \`import.meta.resolve("foo")\`
+      (node.type === "CallExpression" &&
+        !node.optional &&
+        isNodeMatches(node.callee, moduleImportCallees))
+    )
+  )
+) {
+}
+
+// Realworld case from Babel repo
+const argsOptEligible =
+  !state.deopted &&
+  !(
+    // ex: \`args[0] = "whatever"\`
+    (
+      (grandparentPath.isAssignmentExpression() &&
+        parentPath.node === grandparentPath.node.left) ||
+      // ex: \`[args[0]] = ["whatever"]\`
+      grandparentPath.isLVal() ||
+      // ex: \`for (rest[0] in this)\`
+      // ex: \`for (rest[0] of this)\`
+      grandparentPath.isForXStatement() ||
+      // ex: \`++args[0]\`
+      // ex: \`args[0]--\`
+      grandparentPath.isUpdateExpression() ||
+      // ex: \`delete args[0]\`
+      grandparentPath.isUnaryExpression({ operator: "delete" }) ||
+      // ex: \`args[0]()\`
+      // ex: \`new args[0]()\`
+      // ex: \`new args[0]\`
+      ((grandparentPath.isCallExpression() ||
+        grandparentPath.isNewExpression()) &&
+        parentPath.node === grandparentPath.node.callee)
+    )
+  );
+
+// Realworld case from excalidraw repo
+const foo = () =>
+  !!(
+    // versions are required integers
+    (
+      Number.isInteger(deleted.version) &&
+      Number.isInteger(inserted.version) &&
+      // versions should be positive, zero included
+      deleted.version >= 0 &&
+      inserted.version >= 0 &&
+      // versions should never be the same
+      deleted.version !== inserted.version
+    )
+  );
+
+=====================================output=====================================
+// https://github.com/estree/estree/blob/96fee942ecc2b3b9d3c34163ec142b75daf4cca1/es5.md#unaryoperator
+[
+  !(
+    //
+    octahedralAnteaterGenerator ||
+    //
+    stellatedFossilProcessor
+  ),
+  +(
+    //
+    octahedralAnteaterGenerator &&
+    //
+    stellatedFossilProcessor
+  ),
+  typeof (
+    //
+    octahedralAnteaterGenerator ??
+    //
+    stellatedFossilProcessor
+  ),
+];
+
+// Realworld case from Prettier repo
+if (
+  !(
+    // \`import("foo")\`
+    node.type === "ImportExpression" ||
+    // \`type foo = import("foo")\`
+    node.type === "TSImportType" ||
+    // \`import type A = require("foo")\`
+    node.type === "TSExternalModuleReference" ||
+    // \`require("foo")\`
+    // \`require.resolve("foo")\`
+    // \`require.resolve.paths("foo")\`
+    // \`import.meta.resolve("foo")\`
+    (node.type === "CallExpression" &&
+      !node.optional &&
+      isNodeMatches(node.callee, moduleImportCallees))
+  )
+) {
+}
+
+// Realworld case from Babel repo
+const argsOptEligible =
+  !state.deopted &&
+  !(
+    // ex: \`args[0] = "whatever"\`
+    (grandparentPath.isAssignmentExpression() &&
+      parentPath.node === grandparentPath.node.left) ||
+    // ex: \`[args[0]] = ["whatever"]\`
+    grandparentPath.isLVal() ||
+    // ex: \`for (rest[0] in this)\`
+    // ex: \`for (rest[0] of this)\`
+    grandparentPath.isForXStatement() ||
+    // ex: \`++args[0]\`
+    // ex: \`args[0]--\`
+    grandparentPath.isUpdateExpression() ||
+    // ex: \`delete args[0]\`
+    grandparentPath.isUnaryExpression({ operator: "delete" }) ||
+    // ex: \`args[0]()\`
+    // ex: \`new args[0]()\`
+    // ex: \`new args[0]\`
+    ((grandparentPath.isCallExpression() ||
+      grandparentPath.isNewExpression()) &&
+      parentPath.node === grandparentPath.node.callee)
+  );
+
+// Realworld case from excalidraw repo
+const foo = () =>
+  !!(
+    // versions are required integers
+    Number.isInteger(deleted.version) &&
+    Number.isInteger(inserted.version) &&
+    // versions should be positive, zero included
+    deleted.version >= 0 &&
+    inserted.version >= 0 &&
+    // versions should never be the same
+    deleted.version !== inserted.version
+  );
+
+================================================================================
+`;
+
 exports[`issue-7024.js format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow", "typescript"]

tests/format/js/logical-expressions/in-unary-expression.js

@@ -0,0 +1,87 @@
+
+
+// https://github.com/estree/estree/blob/96fee942ecc2b3b9d3c34163ec142b75daf4cca1/es5.md#unaryoperator
+[
+!(
+  //
+  octahedralAnteaterGenerator ||
+  //
+  stellatedFossilProcessor
+),
++(
+  //
+  octahedralAnteaterGenerator &&
+  //
+  stellatedFossilProcessor
+),
+typeof (
+  //
+  octahedralAnteaterGenerator ??
+  //
+  stellatedFossilProcessor
+),
+];
+
+// Realworld case from Prettier repo
+if (
+  !(
+    // `import("foo")`
+    (
+      node.type === "ImportExpression" ||
+      // `type foo = import("foo")`
+      node.type === "TSImportType" ||
+      // `import type A = require("foo")`
+      node.type === "TSExternalModuleReference" ||
+      // `require("foo")`
+      // `require.resolve("foo")`
+      // `require.resolve.paths("foo")`
+      // `import.meta.resolve("foo")`
+      (node.type === "CallExpression" &&
+        !node.optional &&
+        isNodeMatches(node.callee, moduleImportCallees))
+    )
+  )
+) {
+}
+
+// Realworld case from Babel repo
+const argsOptEligible =
+  !state.deopted &&
+  !(
+    // ex: `args[0] = "whatever"`
+    (
+      (grandparentPath.isAssignmentExpression() &&
+        parentPath.node === grandparentPath.node.left) ||
+      // ex: `[args[0]] = ["whatever"]`
+      grandparentPath.isLVal() ||
+      // ex: `for (rest[0] in this)`
+      // ex: `for (rest[0] of this)`
+      grandparentPath.isForXStatement() ||
+      // ex: `++args[0]`
+      // ex: `args[0]--`
+      grandparentPath.isUpdateExpression() ||
+      // ex: `delete args[0]`
+      grandparentPath.isUnaryExpression({ operator: "delete" }) ||
+      // ex: `args[0]()`
+      // ex: `new args[0]()`
+      // ex: `new args[0]`
+      ((grandparentPath.isCallExpression() ||
+        grandparentPath.isNewExpression()) &&
+        parentPath.node === grandparentPath.node.callee)
+    )
+  );
+
+// Realworld case from excalidraw repo
+const foo = () =>
+  !!(
+    // versions are required integers
+    (
+      Number.isInteger(deleted.version) &&
+      Number.isInteger(inserted.version) &&
+      // versions should be positive, zero included
+      deleted.version >= 0 &&
+      inserted.version >= 0 &&
+      // versions should never be the same
+      deleted.version !== inserted.version
+    )
+  );

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

@@ -309,33 +309,33 @@ async function bar2() {
 );
 
 !(x + y);
-!((x + y) /* foo */);
-!(/* foo */ (x + y));
+!(x + y /* foo */);
+!(/* foo */ x + y);
 !(
   /* foo */
-  (x + y)
+  x + y
 );
 !(
-  (x + y)
+  x + y
   /* foo */
 );
 !(
-  (x + y) // foo
+  x + y // foo
 );
 
 !(x || y);
-!(/* foo */ (x || y));
-!((x || y) /* foo */);
+!(/* foo */ x || y);
+!(x || y /* foo */);
 !(
   /* foo */
-  (x || y)
+  x || y
 );
 !(
-  (x || y)
+  x || y
   /* foo */
 );
 !(
-  (x || y) // foo
+  x || y // foo
 );
 
 ![1, 2, 3];