Remove redundant parentheses around SequenceExpression and AssignmentExpression (#18142)

changelog_unreleased/javascript/18142.md

@@ -0,0 +1,27 @@
+#### Remove redundant parentheses in "return statement" (#18142 by @fisker)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+function sequenceExpressionInside() {
+  return ( // Reason for a
+    a, b
+  );
+}
+
+// Prettier stable
+function sequenceExpressionInside() {
+  return (
+    // Reason for a
+    (a, b)
+  );
+}
+
+// Prettier main
+function sequenceExpressionInside() {
+  return (
+    // Reason for a
+    a, b
+  );
+}
+```

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

@@ -18,10 +18,12 @@ import {
   isNumericLiteral,
   isObjectExpression,
   isObjectProperty,
+  isReturnOrThrowStatement,
   isUnionType,
   shouldFlatten,
   startsWithNoLookaheadToken,
 } from "../utilities/index.js";
+import { returnArgumentHasLeadingComment } from "../utilities/return-statement-has-leading-comment.js";
 
 /**
  * @import AstPath from "../../common/ast-path.js"
@@ -190,6 +192,13 @@ function needsParentheses(path, options) {
   }
 
   switch (parent.type) {
+    case "ReturnStatement":
+    case "ThrowStatement":
+      if (willReturnOrThrowStatementBreak(path, options)) {
+        return false;
+      }
+      break;
+
     case "ParenthesizedExpression":
       return false;
     case "ClassDeclaration":
@@ -1350,4 +1359,29 @@ function canDecoratorExpressionUnparenthesized(node) {
   );
 }
 
+function willReturnOrThrowStatementBreak(path, options) {
+  const { key, parent } = path;
+  if (!(key === "argument" && isReturnOrThrowStatement(parent))) {
+    return false;
+  }
+
+  /*
+  When `ReturnStatement` or `ThrowStatement` breaks, parentheses will be added around it's argument.
+  So don't need add parentheses again.
+  But we can't know how the argument printed, so only matches cases that will break for sure
+  */
+
+  const { node } = path;
+
+  if (
+    (node.type === "SequenceExpression" ||
+      node.type === "AssignmentExpression") &&
+    returnArgumentHasLeadingComment(node, options)
+  ) {
+    return true;
+  }
+
+  return false;
+}
+
 export default needsParentheses;

src/language-js/print/sequence-expression.js

@@ -6,6 +6,24 @@ import {
   line,
   softline,
 } from "../../document/index.js";
+import needsParentheses from "../parentheses/needs-parentheses.js";
+
+function shouldIndentSequenceExpression(path, options) {
+  const { key, parent } = path;
+  if (
+    key === "argument" &&
+    (parent.type === "ReturnStatement" || parent.type === "ThrowStatement") &&
+    needsParentheses(path, options)
+  ) {
+    return true;
+  }
+
+  if (key === "body" && parent.type === "ArrowFunctionExpression") {
+    return true;
+  }
+
+  return false;
+}
 
 function printSequenceExpression(path, options, print) {
   const { parent } = path;
@@ -26,11 +44,7 @@ function printSequenceExpression(path, options, print) {
 
   const parts = join([",", line], path.map(print, "expressions"));
 
-  if (
-    ((parent.type === "ReturnStatement" || parent.type === "ThrowStatement") &&
-      path.key === "argument") ||
-    (parent.type === "ArrowFunctionExpression" && path.key === "body")
-  ) {
+  if (shouldIndentSequenceExpression(path, options)) {
     return group(ifBreak([indent([softline, parts]), softline], parts));
   }
 

src/language-js/utilities/index.js

@@ -1111,6 +1111,11 @@ function isBooleanTypeCoercion(node) {
   );
 }
 
+const isReturnOrThrowStatement = createTypeCheckFunction([
+  "ReturnStatement",
+  "ThrowStatement",
+]);
+
 export {
   CommentCheckFlags,
   createTypeCheckFunction,
@@ -1156,6 +1161,7 @@ export {
   isObjectType,
   isPrettierIgnoreComment,
   isRegExpLiteral,
+  isReturnOrThrowStatement,
   isSignedNumericLiteral,
   isSimpleCallArgument,
   isSimpleExpressionByNodeCount,

src/language-js/utilities/return-statement-has-leading-comment.js

@@ -12,7 +12,7 @@ import {
 // This recurses the return argument, looking for the first token
 // (the leftmost leaf node) and, if it (or its parents) has any
 // leadingComments, returns true (so it can be wrapped in parens).
-function returnArgumentHasLeadingComment(node, options) {
+function returnArgumentHasLeadingCommentWithoutCache(node, options) {
   if (
     hasLeadingOwnLineComment(options.originalText, node) ||
     (hasComment(node, CommentCheckFlags.Leading, (comment) =>
@@ -42,4 +42,13 @@ function returnArgumentHasLeadingComment(node, options) {
   return false;
 }
 
+const cache = new WeakMap();
+function returnArgumentHasLeadingComment(node, options) {
+  if (!cache.has(node)) {
+    cache.set(node, returnArgumentHasLeadingCommentWithoutCache(node, options));
+  }
+
+  return cache.get(node);
+}
+
 export { returnArgumentHasLeadingComment };

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

@@ -5137,6 +5137,12 @@ function sequenceExpressionInside() {
   return ( // Reason for a
     a, b
   );
+  throw ( // Reason for a
+    a, b
+  );
+  return ( // Reason for a
+    a = 1
+  );
 }
 
 function taggedTemplate() {
@@ -5164,7 +5170,7 @@ function multilineBlockNextLine() {
   return (
     /**
     * @type {string}
-    */ 
+    */
     'result'
   )
 }
@@ -5194,7 +5200,7 @@ function singleLineBlockSameLine() {
 
 function singleLineBlockNextLine() {
   return (
-    /** Result below */ 
+    /** Result below */
     'result'
   )
 }
@@ -5315,7 +5321,15 @@ function excessiveEverything() {
 function sequenceExpressionInside() {
   return (
     // Reason for a
-    (a, b)
+    a, b
+  )
+  throw (
+    // Reason for a
+    a, b
+  )
+  return (
+    // Reason for a
+    a = 1
   )
 }
 
@@ -5491,6 +5505,12 @@ function sequenceExpressionInside() {
   return ( // Reason for a
     a, b
   );
+  throw ( // Reason for a
+    a, b
+  );
+  return ( // Reason for a
+    a = 1
+  );
 }
 
 function taggedTemplate() {
@@ -5518,7 +5538,7 @@ function multilineBlockNextLine() {
   return (
     /**
     * @type {string}
-    */ 
+    */
     'result'
   )
 }
@@ -5548,7 +5568,7 @@ function singleLineBlockSameLine() {
 
 function singleLineBlockNextLine() {
   return (
-    /** Result below */ 
+    /** Result below */
     'result'
   )
 }
@@ -5669,7 +5689,15 @@ function excessiveEverything() {
 function sequenceExpressionInside() {
   return (
     // Reason for a
-    (a, b)
+    a, b
+  );
+  throw (
+    // Reason for a
+    a, b
+  );
+  return (
+    // Reason for a
+    a = 1
   );
 }
 
@@ -5732,6 +5760,181 @@ function singleLineBlockNextLine() {
 ================================================================================
 `;
 
+exports[`return-statement-2.js - {"semi":false} format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+semi: false
+                                                                                | printWidth
+=====================================input======================================
+function foo() {
+  return ( // Comment
+    a, b
+  );
+  return (
+    // Comment
+    a, b
+  );
+  return /* Comment*/ (
+    a, b
+  );
+  return (/* Comment*/
+    a, b
+  );
+  return (
+    /* Comment*/
+    a, b
+  );
+
+  return ( // Comment
+    a = b
+  );
+  return (
+    // Comment
+    a = b
+  );
+
+  throw (
+    // Comment
+    a, b
+  );
+
+  throw (
+    // Comment
+    a = b
+  );
+}
+
+=====================================output=====================================
+function foo() {
+  return (
+    // Comment
+    a, b
+  )
+  return (
+    // Comment
+    a, b
+  )
+  return /* Comment*/ (a, b)
+  return (
+    /* Comment*/
+    a, b
+  )
+  return (
+    /* Comment*/
+    a, b
+  )
+
+  return (
+    // Comment
+    a = b
+  )
+  return (
+    // Comment
+    a = b
+  )
+
+  throw (
+    // Comment
+    a, b
+  )
+
+  throw (
+    // Comment
+    a = b
+  )
+}
+
+================================================================================
+`;
+
+exports[`return-statement-2.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+function foo() {
+  return ( // Comment
+    a, b
+  );
+  return (
+    // Comment
+    a, b
+  );
+  return /* Comment*/ (
+    a, b
+  );
+  return (/* Comment*/
+    a, b
+  );
+  return (
+    /* Comment*/
+    a, b
+  );
+
+  return ( // Comment
+    a = b
+  );
+  return (
+    // Comment
+    a = b
+  );
+
+  throw (
+    // Comment
+    a, b
+  );
+
+  throw (
+    // Comment
+    a = b
+  );
+}
+
+=====================================output=====================================
+function foo() {
+  return (
+    // Comment
+    a, b
+  );
+  return (
+    // Comment
+    a, b
+  );
+  return /* Comment*/ (a, b);
+  return (
+    /* Comment*/
+    a, b
+  );
+  return (
+    /* Comment*/
+    a, b
+  );
+
+  return (
+    // Comment
+    a = b
+  );
+  return (
+    // Comment
+    a = b
+  );
+
+  throw (
+    // Comment
+    a, b
+  );
+
+  throw (
+    // Comment
+    a = b
+  );
+}
+
+================================================================================
+`;
+
 exports[`single-star-jsdoc.js - {"semi":false} format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow", "typescript"]

tests/format/js/comments/return-statement-2.js

@@ -0,0 +1,37 @@
+function foo() {
+  return ( // Comment
+    a, b
+  );
+  return (
+    // Comment
+    a, b
+  );
+  return /* Comment*/ (
+    a, b
+  );
+  return (/* Comment*/
+    a, b
+  );
+  return (
+    /* Comment*/
+    a, b
+  );
+
+  return ( // Comment
+    a = b
+  );
+  return (
+    // Comment
+    a = b
+  );
+
+  throw (
+    // Comment
+    a, b
+  );
+
+  throw (
+    // Comment
+    a = b
+  );
+}

tests/format/js/comments/return-statement.js

@@ -105,6 +105,12 @@ function sequenceExpressionInside() {
   return ( // Reason for a
     a, b
   );
+  throw ( // Reason for a
+    a, b
+  );
+  return ( // Reason for a
+    a = 1
+  );
 }
 
 function taggedTemplate() {
@@ -132,7 +138,7 @@ function multilineBlockNextLine() {
   return (
     /**
     * @type {string}
-    */ 
+    */
     'result'
   )
 }
@@ -162,7 +168,7 @@ function singleLineBlockSameLine() {
 
 function singleLineBlockNextLine() {
   return (
-    /** Result below */ 
+    /** Result below */
     'result'
   )
 }