Preserve parens around expressions in default export declaration that start with function or class (#6133)

CHANGELOG.unreleased.md

@@ -304,6 +304,27 @@ type G = keyof T[];
 type G = (keyof T)[];
 ```
 
+### JavaScript: Keep parenthesis around functions and classes in `export default` declarations ([#6133] by [@duailibe])
+
+Prettier was removing parenthesis from some expressions in `export default`, but if those are complex expressions that start with `function` or `class`, the parenthesis are required.
+
+See below some practical examples, including one affecting TypeScript.
+
+<!-- prettier-ignore -->
+```ts
+// Input
+export default (function log() {}).toString();
+export default (function log() {} as typeof console.log);
+
+// Output (Prettier stable)
+export default function log() {}.toString();
+export default function log() {} as typeof console.log; // syntax error
+
+// Output (Prettier master)
+export default (function log() {}).toString();
+export default (function log() {} as typeof console.log);
+```
+
 ### TypeScript: Keep parentheses around a function called with non-null assertion. ([6136] by [@sosukesuzuki])
 
 Previously, Prettier removes necessary parentheses around a call expression with non-null assertion. It happens when it's return value is called as function.
@@ -336,6 +357,7 @@ const b = new (c()!)();
 [#6129]: https://github.com/prettier/prettier/pull/6129
 [#6130]: https://github.com/prettier/prettier/pull/6130
 [#6131]: https://github.com/prettier/prettier/pull/6131
+[#6133]: https://github.com/prettier/prettier/pull/6133
 [#6136]: https://github.com/prettier/prettier/pull/6136
 [@belochub]: https://github.com/belochub
 [@brainkim]: https://github.com/brainkim

src/language-js/needs-parens.js

@@ -4,7 +4,11 @@ const assert = require("assert");
 
 const util = require("../common/util");
 const comments = require("./comments");
-const { hasFlowShorthandAnnotationComment } = require("./utils");
+const {
+  getLeftSidePathName,
+  hasNakedLeftSide,
+  hasFlowShorthandAnnotationComment
+} = require("./utils");
 
 function hasClosureCompilerTypeCastComment(text, path) {
   // https://github.com/google/closure-compiler/wiki/Annotating-Types#type-casts
@@ -155,6 +159,13 @@ function needsParens(path, options) {
     return true;
   }
 
+  // `export default function` or `export default class` can't be followed by
+  // anything after. So an expression like `export default (function(){}).toString()`
+  // needs to be followed by a parentheses
+  if (parent.type === "ExportDefaultDeclaration") {
+    return shouldWrapFunctionForExportDefault(path, options);
+  }
+
   if (parent.type === "Decorator" && parent.expression === node) {
     let hasCallExpression = false;
     let hasMemberExpression = false;
@@ -311,6 +322,7 @@ function needsParens(path, options) {
         case "ClassExpression":
         case "ClassDeclaration":
           return name === "superClass" && parent.superClass === node;
+
         case "TSTypeAssertion":
         case "TaggedTemplateExpression":
         case "UnaryExpression":
@@ -622,8 +634,6 @@ function needsParens(path, options) {
           return name === "callee"; // Not strictly necessary, but it's clearer to the reader if IIFEs are wrapped in parentheses.
         case "TaggedTemplateExpression":
           return true; // This is basically a kind of IIFE.
-        case "ExportDefaultDeclaration":
-          return true;
         default:
           return false;
       }
@@ -658,8 +668,6 @@ function needsParens(path, options) {
 
     case "ClassExpression":
       switch (parent.type) {
-        case "ExportDefaultDeclaration":
-          return true;
         case "NewExpression":
           return name === "callee" && parent.callee === node;
         default:
@@ -839,4 +847,33 @@ function isFollowedByRightBracket(path) {
   return false;
 }
 
+function shouldWrapFunctionForExportDefault(path, options) {
+  const node = path.getValue();
+  const parent = path.getParentNode();
+
+  if (node.type === "FunctionExpression" || node.type === "ClassExpression") {
+    return (
+      parent.type === "ExportDefaultDeclaration" ||
+      // in some cases the function is already wrapped
+      // (e.g. `export default (function() {})();`)
+      // in this case we don't need to add extra parens
+      !needsParens(path, options)
+    );
+  }
+
+  if (
+    !hasNakedLeftSide(node) ||
+    (parent.type !== "ExportDefaultDeclaration" && needsParens(path, options))
+  ) {
+    return false;
+  }
+
+  return path.call.apply(
+    path,
+    [
+      childPath => shouldWrapFunctionForExportDefault(childPath, options)
+    ].concat(getLeftSidePathName(path, node))
+  );
+}
+
 module.exports = needsParens;

src/language-js/printer-estree.js

@@ -41,6 +41,9 @@ const {
 } = require("./html-binding");
 const preprocess = require("./preprocess");
 const {
+  getLeftSide,
+  getLeftSidePathName,
+  hasNakedLeftSide,
   hasNode,
   hasFlowAnnotationComment,
   hasFlowShorthandAnnotationComment
@@ -6002,74 +6005,12 @@ function hasLeadingOwnLineComment(text, node, options) {
   return res;
 }
 
-function hasNakedLeftSide(node) {
-  return (
-    node.type === "AssignmentExpression" ||
-    node.type === "BinaryExpression" ||
-    node.type === "LogicalExpression" ||
-    node.type === "NGPipeExpression" ||
-    node.type === "ConditionalExpression" ||
-    node.type === "CallExpression" ||
-    node.type === "OptionalCallExpression" ||
-    node.type === "MemberExpression" ||
-    node.type === "OptionalMemberExpression" ||
-    node.type === "SequenceExpression" ||
-    node.type === "TaggedTemplateExpression" ||
-    node.type === "BindExpression" ||
-    (node.type === "UpdateExpression" && !node.prefix) ||
-    node.type === "TSNonNullExpression"
-  );
-}
-
 function isFlowAnnotationComment(text, typeAnnotation, options) {
   const start = options.locStart(typeAnnotation);
   const end = skipWhitespace(text, options.locEnd(typeAnnotation));
   return text.substr(start, 2) === "/*" && text.substr(end, 2) === "*/";
 }
 
-function getLeftSide(node) {
-  if (node.expressions) {
-    return node.expressions[0];
-  }
-  return (
-    node.left ||
-    node.test ||
-    node.callee ||
-    node.object ||
-    node.tag ||
-    node.argument ||
-    node.expression
-  );
-}
-
-function getLeftSidePathName(path, node) {
-  if (node.expressions) {
-    return ["expressions", 0];
-  }
-  if (node.left) {
-    return ["left"];
-  }
-  if (node.test) {
-    return ["test"];
-  }
-  if (node.object) {
-    return ["object"];
-  }
-  if (node.callee) {
-    return ["callee"];
-  }
-  if (node.tag) {
-    return ["tag"];
-  }
-  if (node.argument) {
-    return ["argument"];
-  }
-  if (node.expression) {
-    return ["expression"];
-  }
-  throw new Error("Unexpected node has no left side", node);
-}
-
 function exprNeedsASIProtection(path, options) {
   const node = path.getValue();
 

src/language-js/utils.js

@@ -44,7 +44,73 @@ function hasNode(node, fn) {
     : Object.keys(node).some(key => hasNode(node[key], fn));
 }
 
+function hasNakedLeftSide(node) {
+  return (
+    node.type === "AssignmentExpression" ||
+    node.type === "BinaryExpression" ||
+    node.type === "LogicalExpression" ||
+    node.type === "NGPipeExpression" ||
+    node.type === "ConditionalExpression" ||
+    node.type === "CallExpression" ||
+    node.type === "OptionalCallExpression" ||
+    node.type === "MemberExpression" ||
+    node.type === "OptionalMemberExpression" ||
+    node.type === "SequenceExpression" ||
+    node.type === "TaggedTemplateExpression" ||
+    node.type === "BindExpression" ||
+    (node.type === "UpdateExpression" && !node.prefix) ||
+    node.type === "TSAsExpression" ||
+    node.type === "TSNonNullExpression"
+  );
+}
+
+function getLeftSide(node) {
+  if (node.expressions) {
+    return node.expressions[0];
+  }
+  return (
+    node.left ||
+    node.test ||
+    node.callee ||
+    node.object ||
+    node.tag ||
+    node.argument ||
+    node.expression
+  );
+}
+
+function getLeftSidePathName(path, node) {
+  if (node.expressions) {
+    return ["expressions", 0];
+  }
+  if (node.left) {
+    return ["left"];
+  }
+  if (node.test) {
+    return ["test"];
+  }
+  if (node.object) {
+    return ["object"];
+  }
+  if (node.callee) {
+    return ["callee"];
+  }
+  if (node.tag) {
+    return ["tag"];
+  }
+  if (node.argument) {
+    return ["argument"];
+  }
+  if (node.expression) {
+    return ["expression"];
+  }
+  throw new Error("Unexpected node has no left side", node);
+}
+
 module.exports = {
+  getLeftSide,
+  getLeftSidePathName,
+  hasNakedLeftSide,
   hasNode,
   hasFlowShorthandAnnotationComment,
   hasFlowAnnotationComment

tests/export_default/__snapshots__/jsfmt.spec.js.snap

@@ -1,8 +1,22 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`binary_and_template.js 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+export default (function() {} + foo)\`\`;
+
+=====================================output=====================================
+export default (function() {} + foo)\`\`;
+
+================================================================================
+`;
+
 exports[`body.js 1`] = `
 ====================================options=====================================
-parsers: ["flow", "typescript"]
+parsers: ["babel", "flow", "typescript"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
@@ -13,3 +27,59 @@ export default (class {}[1] = 1);
 
 ================================================================================
 `;
+
+exports[`class_instance.js 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+export default (class {}.getInstance());
+
+=====================================output=====================================
+export default (class {}.getInstance());
+
+================================================================================
+`;
+
+exports[`function_in_template.js 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+export default (function templ() {})\`foo\`;
+
+=====================================output=====================================
+export default (function templ() {})\`foo\`;
+
+================================================================================
+`;
+
+exports[`function_tostring.js 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+export default (function() {}).toString();
+
+=====================================output=====================================
+export default (function() {}.toString());
+
+================================================================================
+`;
+
+exports[`iife.js 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+export default (function foo() {})();
+
+=====================================output=====================================
+export default (function foo() {})();
+
+================================================================================
+`;

tests/export_default/binary_and_template.js

@@ -0,0 +1 @@
+export default (function() {} + foo)``;

tests/export_default/class_instance.js

@@ -0,0 +1 @@
+export default (class {}.getInstance());

tests/export_default/function_in_template.js

@@ -0,0 +1 @@
+export default (function templ() {})`foo`;

tests/export_default/function_tostring.js

@@ -0,0 +1 @@
+export default (function() {}).toString();

tests/export_default/iife.js

@@ -0,0 +1 @@
+export default (function foo() {})();

tests/export_default/jsfmt.spec.js

@@ -1 +1 @@
-run_spec(__dirname, ["flow", "typescript"]);
+run_spec(__dirname, ["babel", "flow", "typescript"]);

tests/export_default_ts/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,15 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`function_as.ts 1`] = `
+====================================options=====================================
+parsers: ["typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+export default (function log(){} as typeof console.log);
+
+=====================================output=====================================
+export default (function log() {} as typeof console.log);
+
+================================================================================
+`;

tests/export_default_ts/function_as.ts

@@ -0,0 +1 @@
+export default (function log(){} as typeof console.log);

tests/export_default_ts/jsfmt.spec.js

@@ -0,0 +1 @@
+run_spec(__dirname, ["typescript"]);

tests/typescript_as/__snapshots__/jsfmt.spec.js.snap

@@ -75,3 +75,17 @@ const state = JSON.stringify({
 
 ================================================================================
 `;
+
+exports[`export_default_as.ts 1`] = `
+====================================options=====================================
+parsers: ["typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+export default (function log() {} as typeof console.log)
+
+=====================================output=====================================
+export default (function log() {} as typeof console.log);
+
+================================================================================
+`;

tests/typescript_as/export_default_as.ts

@@ -0,0 +1 @@
+export default (function log() {} as typeof console.log)