diff --git a/changelog_unreleased/javascript/9672.md b/changelog_unreleased/javascript/9672.md new file mode 100644 index 000000000..58fc5c2b7 --- /dev/null +++ b/changelog_unreleased/javascript/9672.md @@ -0,0 +1,27 @@ +#### Improve comment format (#9672 by @fisker) + +When multiple comments on same line, the result became unstable. + + +```jsx +// Input +a; +/*1*//*2*/ +/*3*/ +b; + +// Prettier stable +a; /*2*/ +/*1*/ /*3*/ +b; + +// Prettier stable (second format) +a; /*2*/ /*3*/ +/*1*/ b; + +// Prettier master +a; +/*1*/ /*2*/ +/*3*/ +b; +``` diff --git a/src/main/comments.js b/src/main/comments.js index c5c0931e5..57db1e932 100644 --- a/src/main/comments.js +++ b/src/main/comments.js @@ -187,7 +187,27 @@ function attach(comments, ast, text, options) { remaining: handleRemainingComment = returnFalse, } = handleComments; - comments.forEach((comment, i) => { + const decoratedComments = comments.map((comment, index) => ({ + ...decorateComment(ast, comment, options), + comment, + text, + options, + ast, + isLastComment: comments.length - 1 === index, + })); + + decoratedComments.forEach((context, index) => { + const { + comment, + precedingNode, + enclosingNode, + followingNode, + text, + options, + ast, + isLastComment, + } = context; + if ( options.parser === "json" || options.parser === "json5" || @@ -204,20 +224,6 @@ function attach(comments, ast, text, options) { } } - const isLastComment = comments.length - 1 === i; - const decorated = decorateComment(ast, comment, options); - const { precedingNode, enclosingNode, followingNode } = decorated; - const context = { - comment, - precedingNode, - enclosingNode, - followingNode, - text, - options, - ast, - isLastComment, - }; - let args; if (avoidAstMutation) { args = [context]; @@ -228,7 +234,7 @@ function attach(comments, ast, text, options) { args = [comment, text, options, ast, isLastComment]; } - if (hasNewline(text, locStart(comment), { backwards: true })) { + if (isOwnLineComment(text, options, decoratedComments, index)) { // 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. if (handleOwnLineComment(...args)) { @@ -245,7 +251,7 @@ function attach(comments, ast, text, options) { /* istanbul ignore next */ addDanglingComment(ast, comment); } - } else if (hasNewline(text, locEnd(comment))) { + } else if (isEndOfLineComment(text, options, decoratedComments, index)) { if (handleEndOfLineComment(...args)) { // We're good } else if (precedingNode) { @@ -306,6 +312,61 @@ function attach(comments, ast, text, options) { } } +const isAllEmptyAndNoLineBreak = (text) => !/[\S\n\u2028\u2029]/.test(text); +function isOwnLineComment(text, options, decoratedComments, commentIndex) { + const { comment, precedingNode } = decoratedComments[commentIndex]; + const { locStart, locEnd } = options; + let start = locStart(comment); + + if (precedingNode) { + // Find first comment on the same line + for (let index = commentIndex - 1; index >= 0; index--) { + const { + comment, + precedingNode: currentCommentPrecedingNode, + } = decoratedComments[index]; + if ( + currentCommentPrecedingNode !== precedingNode || + !isAllEmptyAndNoLineBreak(text.slice(locEnd(comment), start)) + ) { + break; + } + start = locStart(comment); + } + } + + return hasNewline(text, start, { backwards: true }); +} + +function isEndOfLineComment(text, options, decoratedComments, commentIndex) { + const { comment, followingNode } = decoratedComments[commentIndex]; + const { locStart, locEnd } = options; + let end = locEnd(comment); + + if (followingNode) { + // Find last comment on the same line + for ( + let index = commentIndex + 1; + index < decoratedComments.length; + index++ + ) { + const { + comment, + followingNode: currentCommentFollowingNode, + } = decoratedComments[index]; + if ( + currentCommentFollowingNode !== followingNode || + !isAllEmptyAndNoLineBreak(text.slice(end, locStart(comment))) + ) { + break; + } + end = locEnd(comment); + } + } + + return hasNewline(text, end); +} + function breakTies(tiesToBreak, text, options) { const tieCount = tiesToBreak.length; if (tieCount === 0) { diff --git a/tests/js/comments/__snapshots__/jsfmt.spec.js.snap b/tests/js/comments/__snapshots__/jsfmt.spec.js.snap index aec6bbbce..3aebe8723 100644 --- a/tests/js/comments/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/comments/__snapshots__/jsfmt.spec.js.snap @@ -2152,6 +2152,282 @@ if (14) { ================================================================================ `; +exports[`issue-3532.js - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +import React from 'react'; + +/* +import styled from 'react-emotion'; + +const AspectRatioBox = styled.div\` + &::before { + content: ''; + width: 1px; + margin-left: -1px; + float: left; + height: 0; + padding-top: \${props => 100 / props.aspectRatio}%; + } + + &::after { + /* To clear float *//* + content: ''; + display: table; + clear: both; + } +\`; +*/ + +const AspectRatioBox = ({ + aspectRatio, + children, + ...props +}) => ( +
100 / props.aspectRatio}%; + background: white; + position: relative;\`} + > +
{children}
+
+); + +export default AspectRatioBox; + +=====================================output===================================== +import React from "react" + +/* +import styled from 'react-emotion'; + +const AspectRatioBox = styled.div\` + &::before { + content: ''; + width: 1px; + margin-left: -1px; + float: left; + height: 0; + padding-top: \${props => 100 / props.aspectRatio}%; + } + + &::after { + /* To clear float */ /* + content: ''; + display: table; + clear: both; + } +\`; +*/ + +const AspectRatioBox = ({ aspectRatio, children, ...props }) => ( +
100 / props.aspectRatio}%; + background: white; + position: relative;\`} + > +
{children}
+
+) + +export default AspectRatioBox + +================================================================================ +`; + +exports[`issue-3532.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +import React from 'react'; + +/* +import styled from 'react-emotion'; + +const AspectRatioBox = styled.div\` + &::before { + content: ''; + width: 1px; + margin-left: -1px; + float: left; + height: 0; + padding-top: \${props => 100 / props.aspectRatio}%; + } + + &::after { + /* To clear float *//* + content: ''; + display: table; + clear: both; + } +\`; +*/ + +const AspectRatioBox = ({ + aspectRatio, + children, + ...props +}) => ( +
100 / props.aspectRatio}%; + background: white; + position: relative;\`} + > +
{children}
+
+); + +export default AspectRatioBox; + +=====================================output===================================== +import React from "react"; + +/* +import styled from 'react-emotion'; + +const AspectRatioBox = styled.div\` + &::before { + content: ''; + width: 1px; + margin-left: -1px; + float: left; + height: 0; + padding-top: \${props => 100 / props.aspectRatio}%; + } + + &::after { + /* To clear float */ /* + content: ''; + display: table; + clear: both; + } +\`; +*/ + +const AspectRatioBox = ({ aspectRatio, children, ...props }) => ( +
100 / props.aspectRatio}%; + background: white; + position: relative;\`} + > +
{children}
+
+); + +export default AspectRatioBox; + +================================================================================ +`; + +exports[`issue-7724.js - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +const foo = "Bar"; + +/** + * @template T + * @param {Type} type + * @param {T} value + * @return {Value} + *//** + * @param {Type} type + * @return {Value} + */ +function value(type, value) { + if (arguments.length === 2) { + return new ConcreteValue(type, value); + } else { + return new Value(type); + } +} +=====================================output===================================== +const foo = "Bar" + +/** + * @template T + * @param {Type} type + * @param {T} value + * @return {Value} + */ /** + * @param {Type} type + * @return {Value} + */ +function value(type, value) { + if (arguments.length === 2) { + return new ConcreteValue(type, value) + } else { + return new Value(type) + } +} + +================================================================================ +`; + +exports[`issue-7724.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const foo = "Bar"; + +/** + * @template T + * @param {Type} type + * @param {T} value + * @return {Value} + *//** + * @param {Type} type + * @return {Value} + */ +function value(type, value) { + if (arguments.length === 2) { + return new ConcreteValue(type, value); + } else { + return new Value(type); + } +} +=====================================output===================================== +const foo = "Bar"; + +/** + * @template T + * @param {Type} type + * @param {T} value + * @return {Value} + */ /** + * @param {Type} type + * @return {Value} + */ +function value(type, value) { + if (arguments.length === 2) { + return new ConcreteValue(type, value); + } else { + return new Value(type); + } +} + +================================================================================ +`; + exports[`issues.js - {"semi":false} format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] @@ -3341,6 +3617,424 @@ y; ================================================================================ `; +exports[`multi-comments-on-same-line.js - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +/*========= All on same line =========*/ +a; +/*1*//*2*//*3*/ +b; + +a;/*1*//*2*//*3*/ +b; + +a; +/*1*//*2*//*3*/b; + +a; +/* +1*//*2*//*3 +*/ +b; + +a;/* +1*//*2*//*3 +*/ +b; + +a;/* +1*//*2*//*3 +*/b; + +/*========= First two on same line =========*/ +a; +/*1*//*2*/ +/*3*/ +b; + +a;/*1*//*2*/ +/*3*/ +b; + +a; +/*1*//*2*/ +/*3*/b; + +a; +/* +1*//*2*/ +/*3 +*/ +b; + +a;/* +1*//*2*/ +/*3 +*/ +b; + +a;/* +1*//*2*/ +/*3 +*/b; + +/*========= Last two on same line =========*/ +a; +/*1*/ +/*2*//*3*/ +b; + +a;/*1*/ +/*2*//*3*/ +b; + +a; +/*1*/ +/*2*//*3*/b; + +a; +/* +1*/ +/*2*//*3 +*/ +b; + +a;/* +1*/ +/*2*//*3 +*/ +b; + +a;/* +1*/ +/*2*//*3 +*/b; + +=====================================output===================================== +/*========= All on same line =========*/ +a +/*1*/ /*2*/ /*3*/ +b + +a /*1*/ /*2*/ /*3*/ +b + +a +/*1*/ /*2*/ /*3*/ b + +a +/* +1*/ /*2*/ /*3 + */ +b + +a /* +1*/ /*2*/ +/*3 + */ +b + +a +/* +1*/ /*2*/ /*3 + */ b + +/*========= First two on same line =========*/ +a +/*1*/ /*2*/ +/*3*/ +b + +a /*1*/ /*2*/ +/*3*/ +b + +a +/*1*/ /*2*/ +/*3*/ b + +a +/* +1*/ /*2*/ +/*3 + */ +b + +a /* +1*/ /*2*/ +/*3 + */ +b + +a /* +1*/ /*2*/ +/*3 + */ b + +/*========= Last two on same line =========*/ +a +/*1*/ +/*2*/ /*3*/ +b + +a /*1*/ +/*2*/ /*3*/ +b + +a +/*1*/ +/*2*/ /*3*/ b + +a +/* +1*/ +/*2*/ /*3 + */ +b + +a /* +1*/ +/*2*/ /*3 + */ +b + +a /* +1*/ +/*2*/ /*3 + */ b + +================================================================================ +`; + +exports[`multi-comments-on-same-line.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +/*========= All on same line =========*/ +a; +/*1*//*2*//*3*/ +b; + +a;/*1*//*2*//*3*/ +b; + +a; +/*1*//*2*//*3*/b; + +a; +/* +1*//*2*//*3 +*/ +b; + +a;/* +1*//*2*//*3 +*/ +b; + +a;/* +1*//*2*//*3 +*/b; + +/*========= First two on same line =========*/ +a; +/*1*//*2*/ +/*3*/ +b; + +a;/*1*//*2*/ +/*3*/ +b; + +a; +/*1*//*2*/ +/*3*/b; + +a; +/* +1*//*2*/ +/*3 +*/ +b; + +a;/* +1*//*2*/ +/*3 +*/ +b; + +a;/* +1*//*2*/ +/*3 +*/b; + +/*========= Last two on same line =========*/ +a; +/*1*/ +/*2*//*3*/ +b; + +a;/*1*/ +/*2*//*3*/ +b; + +a; +/*1*/ +/*2*//*3*/b; + +a; +/* +1*/ +/*2*//*3 +*/ +b; + +a;/* +1*/ +/*2*//*3 +*/ +b; + +a;/* +1*/ +/*2*//*3 +*/b; + +=====================================output===================================== +/*========= All on same line =========*/ +a; +/*1*/ /*2*/ /*3*/ +b; + +a; /*1*/ /*2*/ /*3*/ +b; + +a; +/*1*/ /*2*/ /*3*/ b; + +a; +/* +1*/ /*2*/ /*3 + */ +b; + +a; /* +1*/ /*2*/ +/*3 + */ +b; + +a; +/* +1*/ /*2*/ /*3 + */ b; + +/*========= First two on same line =========*/ +a; +/*1*/ /*2*/ +/*3*/ +b; + +a; /*1*/ /*2*/ +/*3*/ +b; + +a; +/*1*/ /*2*/ +/*3*/ b; + +a; +/* +1*/ /*2*/ +/*3 + */ +b; + +a; /* +1*/ /*2*/ +/*3 + */ +b; + +a; /* +1*/ /*2*/ +/*3 + */ b; + +/*========= Last two on same line =========*/ +a; +/*1*/ +/*2*/ /*3*/ +b; + +a; /*1*/ +/*2*/ /*3*/ +b; + +a; +/*1*/ +/*2*/ /*3*/ b; + +a; +/* +1*/ +/*2*/ /*3 + */ +b; + +a; /* +1*/ +/*2*/ /*3 + */ +b; + +a; /* +1*/ +/*2*/ /*3 + */ b; + +================================================================================ +`; + +exports[`multi-comments-on-same-line-2.js - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +/* 1 */ /* 2 */ /* 3 */ a; +a; /* 4 */ /* 5 */ /* 6 */ + +=====================================output===================================== +/* 1 */ /* 2 */ /* 3 */ a +a /* 4 */ /* 5 */ /* 6 */ + +================================================================================ +`; + +exports[`multi-comments-on-same-line-2.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +/* 1 */ /* 2 */ /* 3 */ a; +a; /* 4 */ /* 5 */ /* 6 */ + +=====================================output===================================== +/* 1 */ /* 2 */ /* 3 */ a; +a; /* 4 */ /* 5 */ /* 6 */ + +================================================================================ +`; + exports[`preserve-new-line-last.js - {"semi":false} format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] diff --git a/tests/js/comments/issue-3532.js b/tests/js/comments/issue-3532.js new file mode 100644 index 000000000..87c660e44 --- /dev/null +++ b/tests/js/comments/issue-3532.js @@ -0,0 +1,41 @@ +import React from 'react'; + +/* +import styled from 'react-emotion'; + +const AspectRatioBox = styled.div` + &::before { + content: ''; + width: 1px; + margin-left: -1px; + float: left; + height: 0; + padding-top: ${props => 100 / props.aspectRatio}%; + } + + &::after { + /* To clear float *//* + content: ''; + display: table; + clear: both; + } +`; +*/ + +const AspectRatioBox = ({ + aspectRatio, + children, + ...props +}) => ( +
100 / props.aspectRatio}%; + background: white; + position: relative;`} + > +
{children}
+
+); + +export default AspectRatioBox; diff --git a/tests/js/comments/issue-7724.js b/tests/js/comments/issue-7724.js new file mode 100644 index 000000000..50b7c53c9 --- /dev/null +++ b/tests/js/comments/issue-7724.js @@ -0,0 +1,18 @@ +const foo = "Bar"; + +/** + * @template T + * @param {Type} type + * @param {T} value + * @return {Value} + *//** + * @param {Type} type + * @return {Value} + */ +function value(type, value) { + if (arguments.length === 2) { + return new ConcreteValue(type, value); + } else { + return new Value(type); + } +} \ No newline at end of file diff --git a/tests/js/comments/multi-comments-on-same-line-2.js b/tests/js/comments/multi-comments-on-same-line-2.js new file mode 100644 index 000000000..129f6c026 --- /dev/null +++ b/tests/js/comments/multi-comments-on-same-line-2.js @@ -0,0 +1,2 @@ +/* 1 */ /* 2 */ /* 3 */ a; +a; /* 4 */ /* 5 */ /* 6 */ diff --git a/tests/js/comments/multi-comments-on-same-line.js b/tests/js/comments/multi-comments-on-same-line.js new file mode 100644 index 000000000..98b583664 --- /dev/null +++ b/tests/js/comments/multi-comments-on-same-line.js @@ -0,0 +1,89 @@ +/*========= All on same line =========*/ +a; +/*1*//*2*//*3*/ +b; + +a;/*1*//*2*//*3*/ +b; + +a; +/*1*//*2*//*3*/b; + +a; +/* +1*//*2*//*3 +*/ +b; + +a;/* +1*//*2*//*3 +*/ +b; + +a;/* +1*//*2*//*3 +*/b; + +/*========= First two on same line =========*/ +a; +/*1*//*2*/ +/*3*/ +b; + +a;/*1*//*2*/ +/*3*/ +b; + +a; +/*1*//*2*/ +/*3*/b; + +a; +/* +1*//*2*/ +/*3 +*/ +b; + +a;/* +1*//*2*/ +/*3 +*/ +b; + +a;/* +1*//*2*/ +/*3 +*/b; + +/*========= Last two on same line =========*/ +a; +/*1*/ +/*2*//*3*/ +b; + +a;/*1*/ +/*2*//*3*/ +b; + +a; +/*1*/ +/*2*//*3*/b; + +a; +/* +1*/ +/*2*//*3 +*/ +b; + +a;/* +1*/ +/*2*//*3 +*/ +b; + +a;/* +1*/ +/*2*//*3 +*/b;