diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 827b5ad5c..c904f9130 100644 --- a/CHANGELOG.unreleased.md +++ b/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. + + +```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 diff --git a/src/language-js/needs-parens.js b/src/language-js/needs-parens.js index 015cba134..1aade8973 100644 --- a/src/language-js/needs-parens.js +++ b/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; diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 63ab8a8c8..09cc3a338 100644 --- a/src/language-js/printer-estree.js +++ b/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(); diff --git a/src/language-js/utils.js b/src/language-js/utils.js index 5721041ba..5f60e04ae 100644 --- a/src/language-js/utils.js +++ b/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 diff --git a/tests/export_default/__snapshots__/jsfmt.spec.js.snap b/tests/export_default/__snapshots__/jsfmt.spec.js.snap index 3eed46afb..c085ccc7b 100644 --- a/tests/export_default/__snapshots__/jsfmt.spec.js.snap +++ b/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() {})(); + +================================================================================ +`; diff --git a/tests/export_default/binary_and_template.js b/tests/export_default/binary_and_template.js new file mode 100644 index 000000000..c8b7fddf4 --- /dev/null +++ b/tests/export_default/binary_and_template.js @@ -0,0 +1 @@ +export default (function() {} + foo)``; diff --git a/tests/export_default/class_instance.js b/tests/export_default/class_instance.js new file mode 100644 index 000000000..44ab61f2f --- /dev/null +++ b/tests/export_default/class_instance.js @@ -0,0 +1 @@ +export default (class {}.getInstance()); diff --git a/tests/export_default/function_in_template.js b/tests/export_default/function_in_template.js new file mode 100644 index 000000000..cd069356f --- /dev/null +++ b/tests/export_default/function_in_template.js @@ -0,0 +1 @@ +export default (function templ() {})`foo`; diff --git a/tests/export_default/function_tostring.js b/tests/export_default/function_tostring.js new file mode 100644 index 000000000..5803e8b8d --- /dev/null +++ b/tests/export_default/function_tostring.js @@ -0,0 +1 @@ +export default (function() {}).toString(); diff --git a/tests/export_default/iife.js b/tests/export_default/iife.js new file mode 100644 index 000000000..f37f9797c --- /dev/null +++ b/tests/export_default/iife.js @@ -0,0 +1 @@ +export default (function foo() {})(); diff --git a/tests/export_default/jsfmt.spec.js b/tests/export_default/jsfmt.spec.js index a842e1348..eb85eda6b 100644 --- a/tests/export_default/jsfmt.spec.js +++ b/tests/export_default/jsfmt.spec.js @@ -1 +1 @@ -run_spec(__dirname, ["flow", "typescript"]); +run_spec(__dirname, ["babel", "flow", "typescript"]); diff --git a/tests/export_default_ts/__snapshots__/jsfmt.spec.js.snap b/tests/export_default_ts/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..e3afc0ae8 --- /dev/null +++ b/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); + +================================================================================ +`; diff --git a/tests/export_default_ts/function_as.ts b/tests/export_default_ts/function_as.ts new file mode 100644 index 000000000..45c86bf6d --- /dev/null +++ b/tests/export_default_ts/function_as.ts @@ -0,0 +1 @@ +export default (function log(){} as typeof console.log); diff --git a/tests/export_default_ts/jsfmt.spec.js b/tests/export_default_ts/jsfmt.spec.js new file mode 100644 index 000000000..2ea3bb6eb --- /dev/null +++ b/tests/export_default_ts/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(__dirname, ["typescript"]); diff --git a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap index 77601cd55..e1fbf2921 100644 --- a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap +++ b/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); + +================================================================================ +`; diff --git a/tests/typescript_as/export_default_as.ts b/tests/typescript_as/export_default_as.ts new file mode 100644 index 000000000..5704a4073 --- /dev/null +++ b/tests/typescript_as/export_default_as.ts @@ -0,0 +1 @@ +export default (function log() {} as typeof console.log)