From 008ac1c4796c9dfbb5504df6af63d7467dfd79d4 Mon Sep 17 00:00:00 2001 From: Davy Duperron Date: Tue, 7 Mar 2017 20:02:45 +0100 Subject: [PATCH] Fix files with comments only (#813) * Add new handleOnlyComments function. * Update tests. * Update test as the printer forces a trailing newline if there were any contents. * Implement a different heuristic. * Update tests. * Add directives checking in handleOnlyComments function. * Add directives checking in handleOnlyComments function (amend to retrigger CI). * Remove duplicate. --- src/comments.js | 41 ++++++++++-- .../comments/__snapshots__/jsfmt.spec.js.snap | 64 +++++++++++++++++++ tests/comments/blank.js | 16 +++++ .../__snapshots__/jsfmt.spec.js.snap | 1 + .../__snapshots__/jsfmt.spec.js.snap | 1 + 5 files changed, 117 insertions(+), 6 deletions(-) diff --git a/src/comments.js b/src/comments.js index 99e256006..f8f6f4ac3 100644 --- a/src/comments.js +++ b/src/comments.js @@ -132,13 +132,15 @@ function attach(comments, ast, text, options) { var tiesToBreak = []; - comments.forEach(function(comment) { + comments.forEach((comment, i) => { decorateComment(ast, comment, text); const precedingNode = comment.precedingNode; const enclosingNode = comment.enclosingNode; const followingNode = comment.followingNode; + const isLastComment = comments.length - 1 === i; + if (util.hasNewline(text, locStart(comment), { backwards: true })) { // If a comment exists on its own line, prefer a leading comment. // We also need to check if it's the first line of the file. @@ -152,14 +154,15 @@ function attach(comments, ast, text, options) { handleMemberExpressionComments(enclosingNode, followingNode, comment) || handleIfStatementComments(enclosingNode, followingNode, comment) || handleTryStatementComments(enclosingNode, followingNode, comment) || - handleImportSpecifierComments(enclosingNode, comment) || handleClassComments(enclosingNode, comment) || + handleImportSpecifierComments(enclosingNode, comment) || handleUnionTypeComments( precedingNode, enclosingNode, followingNode, comment - ) + ) || + handleOnlyComments(enclosingNode, ast, comment, isLastComment) ) { // We're good } else if (followingNode) { @@ -184,10 +187,11 @@ function attach(comments, ast, text, options) { ) || handleImportSpecifierComments(enclosingNode, comment) || handleTemplateLiteralComments(enclosingNode, comment) || - handleCallExpressionComments(precedingNode, enclosingNode, comment) || handleClassComments(enclosingNode, comment) || + handleCallExpressionComments(precedingNode, enclosingNode, comment) || handlePropertyComments(enclosingNode, comment) || - handleExportNamedDeclarationComments(enclosingNode, comment) + handleExportNamedDeclarationComments(enclosingNode, comment) || + handleOnlyComments(enclosingNode, ast, comment, isLastComment) ) { // We're good } else if (precedingNode) { @@ -207,7 +211,8 @@ function attach(comments, ast, text, options) { handleIfStatementComments(enclosingNode, followingNode, comment) || handleObjectProperty(enclosingNode, precedingNode, comment) || handleTemplateLiteralComments(enclosingNode, comment) || - handleFunctionDeclarationComments(enclosingNode, comment) + handleFunctionDeclarationComments(enclosingNode, comment) || + handleOnlyComments(enclosingNode, ast, comment, isLastComment) ) { // We're good } else if (precedingNode && followingNode) { @@ -584,6 +589,30 @@ function handleExportNamedDeclarationComments(enclosingNode, comment) { return false; } +function handleOnlyComments(enclosingNode, ast, comment, isLastComment) { + // With Flow the enclosingNode is undefined so use the AST instead. + if (ast && ast.body && ast.body.length === 0) { + if (isLastComment) { + addDanglingComment(ast, comment); + } else { + addLeadingComment(ast, comment); + } + return true; + } else if ( + enclosingNode && enclosingNode.type === 'Program' && + enclosingNode.body.length === 0 && enclosingNode.directives && + enclosingNode.directives.length === 0 + ) { + if (isLastComment) { + addDanglingComment(enclosingNode, comment); + } else { + addLeadingComment(enclosingNode, comment); + } + return true; + } + return false; +} + function printComment(commentPath) { const comment = commentPath.getValue(); comment.printed = true; diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap index 53348a68b..24e8f0918 100644 --- a/tests/comments/__snapshots__/jsfmt.spec.js.snap +++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap @@ -44,12 +44,44 @@ exports[`blank.js 1`] = ` // should still exist // // when printed. + +/** + * @typedef {DataDrivenMapping|ConstantMapping} Mapping + */ +/** + * @typedef {Object.} ConfigurationMapping + */ + +/** + * @typedef {Function} D3Scale - a D3 scale + * @property {Function} ticks + * @property {Function} tickFormat + */ +// comment + +// comment ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // This file only // has comments. This comment // should still exist // // when printed. + +/** + * @typedef {DataDrivenMapping|ConstantMapping} Mapping + */ +/** + * @typedef {Object.} ConfigurationMapping + */ + +/** + * @typedef {Function} D3Scale - a D3 scale + * @property {Function} ticks + * @property {Function} tickFormat + */ +// comment + +// comment " `; @@ -59,12 +91,44 @@ exports[`blank.js 2`] = ` // should still exist // // when printed. + +/** + * @typedef {DataDrivenMapping|ConstantMapping} Mapping + */ +/** + * @typedef {Object.} ConfigurationMapping + */ + +/** + * @typedef {Function} D3Scale - a D3 scale + * @property {Function} ticks + * @property {Function} tickFormat + */ +// comment + +// comment ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // This file only // has comments. This comment // should still exist // // when printed. + +/** + * @typedef {DataDrivenMapping|ConstantMapping} Mapping + */ +/** + * @typedef {Object.} ConfigurationMapping + */ + +/** + * @typedef {Function} D3Scale - a D3 scale + * @property {Function} ticks + * @property {Function} tickFormat + */ +// comment + +// comment " `; diff --git a/tests/comments/blank.js b/tests/comments/blank.js index b0989b691..e2d551f62 100644 --- a/tests/comments/blank.js +++ b/tests/comments/blank.js @@ -3,3 +3,19 @@ // should still exist // // when printed. + +/** + * @typedef {DataDrivenMapping|ConstantMapping} Mapping + */ +/** + * @typedef {Object.} ConfigurationMapping + */ + +/** + * @typedef {Function} D3Scale - a D3 scale + * @property {Function} ticks + * @property {Function} tickFormat + */ +// comment + +// comment diff --git a/tests/flow/declare_export/__snapshots__/jsfmt.spec.js.snap b/tests/flow/declare_export/__snapshots__/jsfmt.spec.js.snap index 430647065..0eac17d99 100644 --- a/tests/flow/declare_export/__snapshots__/jsfmt.spec.js.snap +++ b/tests/flow/declare_export/__snapshots__/jsfmt.spec.js.snap @@ -586,6 +586,7 @@ export default { * @providesModule ES6Default * @flow */ + /* export default { numberValue: 42, diff --git a/tests/flow/es6modules/__snapshots__/jsfmt.spec.js.snap b/tests/flow/es6modules/__snapshots__/jsfmt.spec.js.snap index 86e6efb5d..867bc7c31 100644 --- a/tests/flow/es6modules/__snapshots__/jsfmt.spec.js.snap +++ b/tests/flow/es6modules/__snapshots__/jsfmt.spec.js.snap @@ -686,6 +686,7 @@ export default { * @providesModule ES6Default * @flow */ + /* export default { numberValue: 42,