Skip to content

Commit

Permalink
Fix merged line comments (prettier#13438)
Browse files Browse the repository at this point in the history
  • Loading branch information
thorn0 authored and medikoo committed Feb 1, 2024
1 parent e7e82c1 commit 5ad2cb5
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 48 deletions.
16 changes: 16 additions & 0 deletions changelog_unreleased/javascript/13438.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#### Fix interleaved comments (#13438 by @thorn0)

<!-- prettier-ignore -->
```js
// Input
function x() {
} // first
; // second

// Prettier stable
function x() {} // first // second

// Prettier main
function x() {} // first
// second
```
15 changes: 1 addition & 14 deletions src/language-js/print/comment.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { hasNewline } from "../../common/util.js";
import { join, hardline } from "../../document/builders.js";
import { replaceEndOfLine } from "../../document/utils.js";

Expand All @@ -18,19 +17,7 @@ function printComment(commentPath, options) {

if (isBlockComment(comment)) {
if (isIndentableBlockComment(comment)) {
const printed = printIndentableBlockComment(comment);
// We need to prevent an edge case of a previous trailing comment
// printed as a `lineSuffix` which causes the comments to be
// interleaved. See https://github.com/prettier/prettier/issues/4412
if (
comment.trailing &&
!hasNewline(options.originalText, locStart(comment), {
backwards: true,
})
) {
return [hardline, printed];
}
return printed;
return printIndentableBlockComment(comment);
}

const commentEnd = locEnd(comment);
Expand Down
36 changes: 26 additions & 10 deletions src/main/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,14 +463,17 @@ function printLeadingComment(path, options) {
return parts;
}

function printTrailingComment(path, options) {
function printTrailingComment(path, options, previousComment) {
const comment = path.getValue();
const printed = printComment(path, options);

const { printer, originalText, locStart } = options;
const isBlock = printer.isBlockComment && printer.isBlockComment(comment);
const isBlock = printer.isBlockComment?.(comment);

if (hasNewline(originalText, locStart(comment), { backwards: true })) {
if (
(previousComment?.hasLineSuffix && !previousComment?.isBlock) ||
hasNewline(originalText, locStart(comment), { backwards: true })
) {
// This allows comments at the end of nested structures:
// {
// x: 1,
Expand All @@ -489,20 +492,27 @@ function printTrailingComment(path, options) {
locStart
);

return lineSuffix([hardline, isLineBeforeEmpty ? hardline : "", printed]);
return {
doc: lineSuffix([hardline, isLineBeforeEmpty ? hardline : "", printed]),
isBlock,
hasLineSuffix: true,
};
}

let parts = [
const parts = [
isBlock && tokensToString(printed).startsWith("/*, ") ? "" : " ",
printed,
];

// Trailing block comments never need a newline
if (!isBlock) {
parts = [lineSuffix(parts), breakParent];
if (!isBlock || previousComment?.hasLineSuffix) {
return {
doc: [lineSuffix(parts), breakParent],
isBlock,
hasLineSuffix: true,
};
}

return parts;
return { doc: parts, isBlock, hasLineSuffix: false };
}

const commentBlockTypes = new Set(["CommentBlock", "Block", "MultiLine"]);
Expand Down Expand Up @@ -562,6 +572,7 @@ function printCommentsSeparately(path, options, ignored) {

const leadingParts = [];
const trailingParts = [];
let printedTrailingComment;
path.each(() => {
const comment = path.getValue();
if (ignored && ignored.has(comment)) {
Expand All @@ -572,7 +583,12 @@ function printCommentsSeparately(path, options, ignored) {
if (leading) {
leadingParts.push(printLeadingComment(path, options));
} else if (trailing) {
trailingParts.push(printTrailingComment(path, options));
printedTrailingComment = printTrailingComment(
path,
options,
printedTrailingComment
);
trailingParts.push(printedTrailingComment.doc);
}
}, "comments");

Expand Down
131 changes: 107 additions & 24 deletions tests/format/js/comments/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1434,6 +1434,101 @@ const test = "💖";
================================================================================
`;

exports[`empty-statements.js - {"semi":false} format 1`] = `
====================================options=====================================
parsers: ["babel", "flow", "typescript"]
printWidth: 80
semi: false
| printWidth
=====================================input======================================
a; /* a */ // b
; /* c */
foo; // first
;// second
;// third
function x() {
} // first
; // second
a = (
b // 1
+ // 2
c // 3
+ // 4
d // 5
+ /* 6 */
e // 7
);
=====================================output=====================================
a /* a */ // b
/* c */
foo // first
// second
// third
function x() {} // first
// second
a =
b + // 1
// 2
c + // 3
// 4
d + // 5
/* 6 */
e // 7
================================================================================
`;

exports[`empty-statements.js format 1`] = `
====================================options=====================================
parsers: ["babel", "flow", "typescript"]
printWidth: 80
| printWidth
=====================================input======================================
a; /* a */ // b
; /* c */
foo; // first
;// second
;// third
function x() {
} // first
; // second
a = (
b // 1
+ // 2
c // 3
+ // 4
d // 5
+ /* 6 */
e // 7
);
=====================================output=====================================
a; /* a */ // b
/* c */
foo; // first
// second
// third
function x() {} // first
// second
a =
b + // 1
// 2
c + // 3
// 4
d + // 5
/* 6 */
e; // 7
================================================================================
`;

exports[`export.js - {"semi":false} format 1`] = `
====================================options=====================================
parsers: ["babel", "flow", "typescript"]
Expand Down Expand Up @@ -3179,8 +3274,7 @@ onClick={() => {}}>
;<div>
{
a
/* comment
a /* comment
*/
}
</div>
Expand Down Expand Up @@ -3420,8 +3514,7 @@ onClick={() => {}}>
<div>
{
a
/* comment
a /* comment
*/
}
</div>;
Expand Down Expand Up @@ -3963,8 +4056,7 @@ a
b
a /*
1*/ /*2*/
/*3
1*/ /*2*/ /*3
*/
b
Expand Down Expand Up @@ -4155,8 +4247,7 @@ a;
b;
a; /*
1*/ /*2*/
/*3
1*/ /*2*/ /*3
*/
b;
Expand Down Expand Up @@ -5516,24 +5607,20 @@ const CONNECTION_STATUS = (exports.CONNECTION_STATUS = {
NOT_CONNECTED: Object.freeze({ kind: "NOT_CONNECTED" }),
})
/* A comment */
/**
/* A comment */ /**
* A type that can be written to a buffer.
*/
/**
*/ /**
* Describes the connection status of a ReactiveSocket/DuplexConnection.
* - NOT_CONNECTED: no connection established or pending.
* - CONNECTING: when \`connect()\` has been called but a connection is not yet
* established.
* - CONNECTED: when a connection is established.
* - CLOSED: when the connection has been explicitly closed via \`close()\`.
* - ERROR: when the connection has been closed for any other reason.
*/
/**
*/ /**
* A contract providing different interaction models per the [ReactiveSocket protocol]
* (https://github.com/ReactiveSocket/reactivesocket/blob/master/Protocol.md).
*/
/**
*/ /**
* A single unit of data exchanged between the peers of a \`ReactiveSocket\`.
*/
Expand Down Expand Up @@ -5577,24 +5664,20 @@ const CONNECTION_STATUS = (exports.CONNECTION_STATUS = {
NOT_CONNECTED: Object.freeze({ kind: "NOT_CONNECTED" }),
});
/* A comment */
/**
/* A comment */ /**
* A type that can be written to a buffer.
*/
/**
*/ /**
* Describes the connection status of a ReactiveSocket/DuplexConnection.
* - NOT_CONNECTED: no connection established or pending.
* - CONNECTING: when \`connect()\` has been called but a connection is not yet
* established.
* - CONNECTED: when a connection is established.
* - CLOSED: when the connection has been explicitly closed via \`close()\`.
* - ERROR: when the connection has been closed for any other reason.
*/
/**
*/ /**
* A contract providing different interaction models per the [ReactiveSocket protocol]
* (https://github.com/ReactiveSocket/reactivesocket/blob/master/Protocol.md).
*/
/**
*/ /**
* A single unit of data exchanged between the peers of a \`ReactiveSocket\`.
*/
Expand Down
20 changes: 20 additions & 0 deletions tests/format/js/comments/empty-statements.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
a; /* a */ // b
; /* c */

foo; // first
;// second
;// third

function x() {
} // first
; // second

a = (
b // 1
+ // 2
c // 3
+ // 4
d // 5
+ /* 6 */
e // 7
);

0 comments on commit 5ad2cb5

Please sign in to comment.