Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix merged line comments #13438

Merged
merged 8 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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: 25 additions & 11 deletions src/main/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,14 +461,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 @@ -487,17 +490,22 @@ function printTrailingComment(path, options) {
locStart
);

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

let parts = [" ", printed];

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

return parts;
return { doc: [" ", printed], isBlock, hasLineSuffix: false };
}

function printDanglingComments(path, options, sameIndent, filter) {
Expand Down Expand Up @@ -544,6 +552,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 @@ -554,7 +563,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 @@ -1444,6 +1444,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 @@ -3189,8 +3284,7 @@ onClick={() => {}}>

;<div>
{
a
/* comment
a /* comment
*/
}
</div>
Expand Down Expand Up @@ -3430,8 +3524,7 @@ onClick={() => {}}>

<div>
{
a
/* comment
a /* comment
*/
}
</div>;
Expand Down Expand Up @@ -3924,8 +4017,7 @@ a
b

a /*
1*/ /*2*/
/*3
1*/ /*2*/ /*3
*/
b

Expand Down Expand Up @@ -4116,8 +4208,7 @@ a;
b;

a; /*
1*/ /*2*/
/*3
1*/ /*2*/ /*3
*/
b;

Expand Down Expand Up @@ -5489,24 +5580,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 @@ -5550,24 +5637,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
);