From ce952fc8c1c0b8574588c5840ce4cd92e76f7cbd Mon Sep 17 00:00:00 2001 From: Lucas Duailibe Date: Mon, 8 Oct 2018 14:18:30 -0300 Subject: [PATCH] Fix printing of catch clause with a comment (#5202) --- src/language-js/comments.js | 22 +++++++-- src/language-js/printer-estree.js | 30 ++++++++++--- tests/try/__snapshots__/jsfmt.spec.js.snap | 52 ++++++++++++++++++++++ tests/try/catch.js | 25 +++++++++++ 4 files changed, 121 insertions(+), 8 deletions(-) create mode 100644 tests/try/catch.js diff --git a/src/language-js/comments.js b/src/language-js/comments.js index dd0fcfc0..c865f6c2 100644 --- a/src/language-js/comments.js +++ b/src/language-js/comments.js @@ -28,7 +28,12 @@ function handleOwnLineComment(comment, text, options, ast, isLastComment) { comment, options ) || - handleTryStatementComments(enclosingNode, followingNode, comment) || + handleTryStatementComments( + enclosingNode, + precedingNode, + followingNode, + comment + ) || handleClassComments(enclosingNode, precedingNode, followingNode, comment) || handleImportSpecifierComments(enclosingNode, comment) || handleForComments(enclosingNode, precedingNode, comment) || @@ -248,15 +253,26 @@ function handleIfStatementComments( } // Same as IfStatement but for TryStatement -function handleTryStatementComments(enclosingNode, followingNode, comment) { +function handleTryStatementComments( + enclosingNode, + precedingNode, + followingNode, + comment +) { if ( !enclosingNode || - enclosingNode.type !== "TryStatement" || + (enclosingNode.type !== "TryStatement" && + enclosingNode.type !== "CatchClause") || !followingNode ) { return false; } + if (enclosingNode.type === "CatchClause" && precedingNode) { + addTrailingComment(precedingNode, comment); + return true; + } + if (followingNode.type === "BlockStatement") { addBlockStatementFirstComment(followingNode, comment); return true; diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 3c7713dc..802df73f 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -1737,11 +1737,31 @@ function printPathNoParens(path, options, print, args) { n.finalizer ? concat([" finally ", path.call(print, "finalizer")]) : "" ]); case "CatchClause": - return concat([ - "catch ", - n.param ? concat(["(", path.call(print, "param"), ") "]) : "", - path.call(print, "body") - ]); + if (n.param) { + const hasComments = + n.param.comments && + n.param.comments.some( + comment => + !handleComments.isBlockComment(comment) || + (comment.leading && + hasNewline(options.originalText, options.locEnd(comment))) || + (comment.trailing && + hasNewline(options.originalText, options.locStart(comment), { + backwards: true + })) + ); + const param = path.call(print, "param"); + + return concat([ + "catch ", + hasComments + ? concat(["(", indent(concat([softline, param])), softline, ") "]) + : concat(["(", param, ") "]), + path.call(print, "body") + ]); + } + + return concat(["catch ", path.call(print, "body")]); case "ThrowStatement": return concat(["throw ", path.call(print, "argument"), semi]); // Note: ignoring n.lexical because it has no printing consequences. diff --git a/tests/try/__snapshots__/jsfmt.spec.js.snap b/tests/try/__snapshots__/jsfmt.spec.js.snap index 01c9b2eb..25f1db0d 100644 --- a/tests/try/__snapshots__/jsfmt.spec.js.snap +++ b/tests/try/__snapshots__/jsfmt.spec.js.snap @@ -1,5 +1,57 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`catch.js - flow-verify 1`] = ` +try {} +catch( + // comment + foo +) {} + +try {} +catch(foo //comment +) {} + +try {} +catch( + /* comment */ foo +) {} + +try {} +catch( + foo /* comment */ +) {} + +try {} +catch( + foo + /* comment */ +) {} +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +try { +} catch ( + // comment + foo +) {} + +try { +} catch ( + foo //comment +) {} + +try { +} catch (/* comment */ foo) {} + +try { +} catch (foo /* comment */) {} + +try { +} catch ( + foo + /* comment */ +) {} + +`; + exports[`empty.js - flow-verify 1`] = ` try { } catch (e) { diff --git a/tests/try/catch.js b/tests/try/catch.js new file mode 100644 index 00000000..e4d28634 --- /dev/null +++ b/tests/try/catch.js @@ -0,0 +1,25 @@ +try {} +catch( + // comment + foo +) {} + +try {} +catch(foo //comment +) {} + +try {} +catch( + /* comment */ foo +) {} + +try {} +catch( + foo /* comment */ +) {} + +try {} +catch( + foo + /* comment */ +) {}