From 2eca6193d71b022502cb4c1aa112390b39fa6731 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Sun, 1 Dec 2019 20:59:15 +0900 Subject: [PATCH] JavaScript: Format ThrowStatement like ReturnStatement (#7070) --- changelog_unreleased/javascript/pr-7070.md | 24 +++ src/language-js/needs-parens.js | 1 + src/language-js/printer-estree.js | 100 +++++----- .../__snapshots__/jsfmt.spec.js.snap | 173 ++++++++++++++++++ tests/throw_statement/binaryish.js | 23 +++ tests/throw_statement/comment.js | 21 +++ tests/throw_statement/jsfmt.spec.js | 1 + tests/throw_statement/jsx.js | 23 +++ 8 files changed, 323 insertions(+), 43 deletions(-) create mode 100644 changelog_unreleased/javascript/pr-7070.md create mode 100644 tests/throw_statement/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/throw_statement/binaryish.js create mode 100644 tests/throw_statement/comment.js create mode 100644 tests/throw_statement/jsfmt.spec.js create mode 100644 tests/throw_statement/jsx.js diff --git a/changelog_unreleased/javascript/pr-7070.md b/changelog_unreleased/javascript/pr-7070.md new file mode 100644 index 000000000..e105b65f2 --- /dev/null +++ b/changelog_unreleased/javascript/pr-7070.md @@ -0,0 +1,24 @@ +# Format `throw` like `return` ([#7070](https://github.com/prettier/prettier/pull/7070) by [@sosukesuzuki](https://github.com/sosukesuzuki)) + + +```js +// Input +function foo() { + throw this.hasPlugin("dynamicImports") && this.lookahead().type === tt.parenLeft.right; +} + +// Prettier stable +function foo() { + throw this.hasPlugin("dynamicImports") && + this.lookahead().type === tt.parenLeft.right; +} + + +// Prettier master +function foo() { + throw ( + this.hasPlugin("dynamicImports") && + this.lookahead().type === tt.parenLeft.right + ); +} +``` diff --git a/src/language-js/needs-parens.js b/src/language-js/needs-parens.js index 9ff9bba87..207103988 100644 --- a/src/language-js/needs-parens.js +++ b/src/language-js/needs-parens.js @@ -759,6 +759,7 @@ function needsParens(path, options) { parent.type !== "OptionalCallExpression" && parent.type !== "Property" && parent.type !== "ReturnStatement" && + parent.type !== "ThrowStatement" && parent.type !== "TypeCastExpression" && parent.type !== "VariableDeclarator") ); diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 49b4552e3..784786a7c 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -597,6 +597,7 @@ function printPathNoParens(path, options, print, args) { // indented accordingly. We should indent sub-expressions where the first case isn't indented. const shouldNotIndent = parent.type === "ReturnStatement" || + parent.type === "ThrowStatement" || (parent.type === "JSXExpressionContainer" && parentParent.type === "JSXAttribute") || (n.type !== "NGPipeExpression" && @@ -608,6 +609,7 @@ function printPathNoParens(path, options, print, args) { (n !== parent.body && parent.type === "ForStatement") || (parent.type === "ConditionalExpression" && parentParent.type !== "ReturnStatement" && + parentParent.type !== "ThrowStatement" && parentParent.type !== "CallExpression" && parentParent.type !== "OptionalCallExpression") || parent.type === "TemplateLiteral"; @@ -1121,48 +1123,10 @@ function printPathNoParens(path, options, print, args) { return concat(parts); } case "ReturnStatement": - parts.push("return"); - - if (n.argument) { - if (returnArgumentHasLeadingComment(options, n.argument)) { - parts.push( - concat([ - " (", - indent(concat([hardline, path.call(print, "argument")])), - hardline, - ")" - ]) - ); - } else if ( - n.argument.type === "LogicalExpression" || - n.argument.type === "BinaryExpression" || - n.argument.type === "SequenceExpression" - ) { - parts.push( - group( - concat([ - ifBreak(" (", " "), - indent(concat([softline, path.call(print, "argument")])), - softline, - ifBreak(")") - ]) - ) - ); - } else { - parts.push(" ", path.call(print, "argument")); - } - } - - if (hasDanglingComments(n)) { - parts.push( - " ", - comments.printDanglingComments(path, options, /* sameIndent */ true) - ); - } - - parts.push(semi); - - return concat(parts); + return concat([ + "return", + printReturnAndThrowArgument(path, options, print) + ]); case "NewExpression": case "OptionalCallExpression": case "CallExpression": { @@ -2030,7 +1994,10 @@ function printPathNoParens(path, options, print, args) { return concat(["catch ", path.call(print, "body")]); case "ThrowStatement": - return concat(["throw ", path.call(print, "argument"), semi]); + return concat([ + "throw", + printReturnAndThrowArgument(path, options, print) + ]); // Note: ignoring n.lexical because it has no printing consequences. case "SwitchStatement": return concat([ @@ -5957,6 +5924,53 @@ function printArrayItems(path, options, printPath, print) { return concat(printedElements); } +function printReturnAndThrowArgument(path, options, print) { + const node = path.getValue(); + const semi = options.semi ? ";" : ""; + const parts = []; + + if (node.argument) { + if (returnArgumentHasLeadingComment(options, node.argument)) { + parts.push( + concat([ + " (", + indent(concat([hardline, path.call(print, "argument")])), + hardline, + ")" + ]) + ); + } else if ( + node.argument.type === "LogicalExpression" || + node.argument.type === "BinaryExpression" || + node.argument.type === "SequenceExpression" + ) { + parts.push( + group( + concat([ + ifBreak(" (", " "), + indent(concat([softline, path.call(print, "argument")])), + softline, + ifBreak(")") + ]) + ) + ); + } else { + parts.push(" ", path.call(print, "argument")); + } + } + + if (hasDanglingComments(node)) { + parts.push( + " ", + comments.printDanglingComments(path, options, /* sameIndent */ true) + ); + } + + parts.push(semi); + + return concat(parts); +} + function willPrintOwnComments(path /*, options */) { const node = path.getValue(); const parent = path.getParentNode(); diff --git a/tests/throw_statement/__snapshots__/jsfmt.spec.js.snap b/tests/throw_statement/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..5d7e4ff57 --- /dev/null +++ b/tests/throw_statement/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,173 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`binaryish.js 1`] = ` +====================================options===================================== +parsers: ["flow", "typescript", "babel"] +printWidth: 80 + | printWidth +=====================================input====================================== +function f() { + throw ( + property.isIdentifier() && + FUNCTIONS[property.node.name] && + (object.isIdentifier(JEST_GLOBAL) || + (callee.isMemberExpression() && shouldHoistExpression(object))) && + FUNCTIONS[property.node.name](expr.get('arguments')) + ); + + throw ( + chalk.bold( + 'No tests found related to files changed since last commit.\\n', + ) + + chalk.dim( + patternInfo.watch ? + 'Press \`a\` to run all tests, or run Jest with \`--watchAll\`.' : + 'Run Jest without \`-o\` to run all tests.', + ) + ); + + throw !filePath.includes(coverageDirectory) && + !filePath.endsWith(\`.\${SNAPSHOT_EXTENSION}\`); +} + +=====================================output===================================== +function f() { + throw ( + property.isIdentifier() && + FUNCTIONS[property.node.name] && + (object.isIdentifier(JEST_GLOBAL) || + (callee.isMemberExpression() && shouldHoistExpression(object))) && + FUNCTIONS[property.node.name](expr.get("arguments")) + ); + + throw ( + chalk.bold("No tests found related to files changed since last commit.\\n") + + chalk.dim( + patternInfo.watch + ? "Press \`a\` to run all tests, or run Jest with \`--watchAll\`." + : "Run Jest without \`-o\` to run all tests." + ) + ); + + throw ( + !filePath.includes(coverageDirectory) && + !filePath.endsWith(\`.\${SNAPSHOT_EXTENSION}\`) + ); +} + +================================================================================ +`; + +exports[`comment.js 1`] = ` +====================================options===================================== +parsers: ["flow", "typescript", "babel"] +printWidth: 80 + | printWidth +=====================================input====================================== +function x() { + throw func2 + //comment + .bar(); +} + +function f() { + throw ( + foo + // comment + .bar() + ); +} + +fn(function f() { + throw ( + foo + // comment + .bar() + ); +}); + +=====================================output===================================== +function x() { + throw ( + func2 + //comment + .bar() + ); +} + +function f() { + throw ( + foo + // comment + .bar() + ); +} + +fn(function f() { + throw ( + foo + // comment + .bar() + ); +}); + +================================================================================ +`; + +exports[`jsx.js 1`] = ` +====================================options===================================== +parsers: ["flow", "typescript", "babel"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + throw ; +} + +function foo() { + throw baz; +} + +function foo() { + throw ; +} + +function foo() { + throw foo; +} + +function foo() { + throw <>; +} + +function foo() { + throw <>foo; +} + +=====================================output===================================== +function foo() { + throw ; +} + +function foo() { + throw baz; +} + +function foo() { + throw ; +} + +function foo() { + throw foo; +} + +function foo() { + throw <>; +} + +function foo() { + throw <>foo; +} + +================================================================================ +`; diff --git a/tests/throw_statement/binaryish.js b/tests/throw_statement/binaryish.js new file mode 100644 index 000000000..a56877573 --- /dev/null +++ b/tests/throw_statement/binaryish.js @@ -0,0 +1,23 @@ +function f() { + throw ( + property.isIdentifier() && + FUNCTIONS[property.node.name] && + (object.isIdentifier(JEST_GLOBAL) || + (callee.isMemberExpression() && shouldHoistExpression(object))) && + FUNCTIONS[property.node.name](expr.get('arguments')) + ); + + throw ( + chalk.bold( + 'No tests found related to files changed since last commit.\n', + ) + + chalk.dim( + patternInfo.watch ? + 'Press `a` to run all tests, or run Jest with `--watchAll`.' : + 'Run Jest without `-o` to run all tests.', + ) + ); + + throw !filePath.includes(coverageDirectory) && + !filePath.endsWith(`.${SNAPSHOT_EXTENSION}`); +} diff --git a/tests/throw_statement/comment.js b/tests/throw_statement/comment.js new file mode 100644 index 000000000..fd74f067c --- /dev/null +++ b/tests/throw_statement/comment.js @@ -0,0 +1,21 @@ +function x() { + throw func2 + //comment + .bar(); +} + +function f() { + throw ( + foo + // comment + .bar() + ); +} + +fn(function f() { + throw ( + foo + // comment + .bar() + ); +}); diff --git a/tests/throw_statement/jsfmt.spec.js b/tests/throw_statement/jsfmt.spec.js new file mode 100644 index 000000000..f268e6d0f --- /dev/null +++ b/tests/throw_statement/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(__dirname, ["flow", "typescript", "babel"]); diff --git a/tests/throw_statement/jsx.js b/tests/throw_statement/jsx.js new file mode 100644 index 000000000..2275b17ae --- /dev/null +++ b/tests/throw_statement/jsx.js @@ -0,0 +1,23 @@ +function foo() { + throw ; +} + +function foo() { + throw baz; +} + +function foo() { + throw ; +} + +function foo() { + throw foo; +} + +function foo() { + throw <>; +} + +function foo() { + throw <>foo; +}