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 empty line check between array elements #14736

Merged
merged 7 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
30 changes: 30 additions & 0 deletions changelog_unreleased/javascript/14736.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#### Fix trailing parens tripping up skipToLineEnd (#14736 by @solarized-fox)

<!-- prettier-ignore -->
```jsx
// Input
[
1
,

2,

3
]

// Prettier stable
[
1, 2,

3,
];

// Prettier main
[
1,

2,

3,
];
```
23 changes: 18 additions & 5 deletions src/language-js/print/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,18 @@ import {
fill,
} from "../../document/builders.js";
import hasNewline from "../../utils/has-newline.js";
import isNextLineEmptyAfterIndex from "../../utils/is-next-line-empty.js";
import { skipInlineComment, skipTrailingComment } from "../../utils/public.js";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this on previous review. Do not import from public.js, please import from the actual file directly.

import {
shouldPrintComma,
hasComment,
CommentCheckFlags,
isNextLineEmpty,
isNumericLiteral,
isSignedNumericLiteral,
isArrayOrTupleExpression,
isObjectOrRecordExpression,
} from "../utils/index.js";
import { locStart } from "../loc.js";
import { locStart, locEnd } from "../loc.js";

import { printOptionalToken } from "./misc.js";
import { printTypeAnnotationProperty } from "./type-annotation.js";
Expand Down Expand Up @@ -170,6 +171,18 @@ function isConciselyPrintedArray(node, options) {
);
}

function isLineAfterElementEmpty(path, { originalText: text }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use destructuring assignment here?

Suggested change
function isLineAfterElementEmpty(path, { originalText: text }) {
function isLineAfterElementEmpty({ node }, { originalText: text }) {

const skipComment = (idx) =>
skipInlineComment(text, skipTrailingComment(text, idx));

const skipToComma = (currentIdx) =>
text[currentIdx] === ","
? currentIdx
: skipToComma(skipComment(currentIdx + 1));
fisker marked this conversation as resolved.
Show resolved Hide resolved

return isNextLineEmptyAfterIndex(text, skipToComma(locEnd(path.node)));
}

function printArrayElements(path, options, elementsProperty, print) {
const parts = [];

Expand All @@ -180,7 +193,7 @@ function printArrayElements(path, options, elementsProperty, print) {
parts.push([
",",
line,
node && isNextLineEmpty(node, options) ? softline : "",
node && isLineAfterElementEmpty(path, options) ? softline : "",
]);
}
}, elementsProperty);
Expand All @@ -191,12 +204,12 @@ function printArrayElements(path, options, elementsProperty, print) {
function printArrayElementsConcisely(path, options, print, trailingComma) {
const parts = [];

path.each(({ node, isLast, next }) => {
path.each(({ isLast, next }) => {
parts.push([print(), isLast ? trailingComma : ","]);

if (!isLast) {
parts.push(
isNextLineEmpty(node, options)
isLineAfterElementEmpty(path, options)
? [hardline, hardline]
: hasComment(next, CommentCheckFlags.Leading | CommentCheckFlags.Line)
? hardline
Expand Down
148 changes: 148 additions & 0 deletions tests/format/js/arrays/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,87 @@ a = [

]

b = [
100,

(200)
,

300
,

1
,
2, 3
]

c = [
"apple",
"banana",
"blueberry",

"red",
"blue"
,
"yellow",

"broccoli",
"celery",
"lettuce"
,

"green"
,
"green",
"green",

//an egg
"egg",
//a bigger egg
"big egg"
//the biggest egg
,
"huge egg"
,

//not an egg
"lasagna"

]

_ = [
a,

b //
]

_ = [
(a),

b, //
];

_ = [
(((((
a = b/* comment */))/* comment */))),

c //
]

_ = [
(((((
(a = b)/* comment */))/* comment */))),

c //
]

_ = [
(a=b

),
b, //
];

=====================================output=====================================
a = [
1, 2,
Expand All @@ -853,6 +934,73 @@ a = [
4,
];

b = [
100,

200,

300,

1, 2, 3,
];

c = [
"apple",
"banana",
"blueberry",

"red",
"blue",
"yellow",

"broccoli",
"celery",
"lettuce",

"green",
"green",
"green",

//an egg
"egg",
//a bigger egg
"big egg",
//the biggest egg
"huge egg",

//not an egg
"lasagna",
];

_ = [
a,

b, //
];

_ = [
a,

b, //
];

_ = [
(a = b) /* comment */ /* comment */,

c, //
];

_ = [
(a = b) /* comment */ /* comment */,

c, //
];

_ = [
(a = b),
b, //
];

================================================================================
`;

Expand Down
81 changes: 81 additions & 0 deletions tests/format/js/arrays/preserve_empty_lines.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,84 @@ a = [


]

b = [
100,

(200)
,

300
,

1
,
2, 3
]

c = [
"apple",
"banana",
"blueberry",

"red",
"blue"
,
"yellow",

"broccoli",
"celery",
"lettuce"
,

"green"
,
"green",
"green",

//an egg
"egg",
//a bigger egg
"big egg"
//the biggest egg
,
"huge egg"
,

//not an egg
"lasagna"

]

_ = [
a,

b //
]

_ = [
(a),

b, //
];

_ = [
(((((
a = b/* comment */))/* comment */))),

c //
]

_ = [
(((((
(a = b)/* comment */))/* comment */))),

c //
]

_ = [
(a=b

),
b, //
];
4 changes: 4 additions & 0 deletions tests/format/json/json/__snapshots__/jsfmt.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,7 @@ trailingComma: "all"
0.5,
98.6,
99.44,

1066,
1e1,
0.1e1,
Expand Down Expand Up @@ -1695,6 +1696,7 @@ trailingComma: "all"
0.5,
98.6,
99.44,

1066,
1e1,
0.1e1,
Expand Down Expand Up @@ -1825,6 +1827,7 @@ trailingComma: "es5"
0.5,
98.6,
99.44,

1066,
1e1,
0.1e1,
Expand Down Expand Up @@ -1955,6 +1958,7 @@ trailingComma: "es5"
0.5,
98.6,
99.44,

1066,
1e1,
0.1e1,
Expand Down