Skip to content

Commit cea7a7b

Browse files
dsherretdsherret
committed
fix: JSX element in attribute should not add parens
Closes #294
1 parent 2586e71 commit cea7a7b

File tree

8 files changed

+295
-112
lines changed

8 files changed

+295
-112
lines changed

‎src/parsing/parser.rs

Lines changed: 83 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2270,9 +2270,7 @@ fn parse_object_lit<'a>(node: &'a ObjectLit, context: &mut Context<'a>) -> Print
22702270
}
22712271

22722272
fn parse_paren_expr<'a>(node: &'a ParenExpr, context: &mut Context<'a>) -> PrintItems {
2273-
if is_jsx_paren_expr_handled_node(&node.expr.into(), context) {
2274-
// skip explicitly parsing this as a paren expr as that will be handled
2275-
// in the JSX element/fragment and it might collapse back to not having a paren expr
2273+
if skip_paren_expr(node, context) {
22762274
return parse_node(node.expr.into(), context);
22772275
}
22782276

@@ -2301,6 +2299,35 @@ fn parse_paren_expr<'a>(node: &'a ParenExpr, context: &mut Context<'a>) -> Print
23012299
true
23022300
}
23032301
}
2302+
2303+
fn skip_paren_expr(node: &ParenExpr, context: &Context) -> bool {
2304+
if has_surrounding_comments(&node.expr.into(), context) {
2305+
return false;
2306+
}
2307+
2308+
// skip over any paren exprs within paren exprs and needless paren exprs
2309+
let parent = node.parent();
2310+
if matches!(
2311+
parent.kind(),
2312+
NodeKind::ParenExpr | NodeKind::ExprStmt | NodeKind::JSXElement | NodeKind::JSXFragment | NodeKind::JSXExprContainer
2313+
) {
2314+
return true;
2315+
}
2316+
2317+
// skip over an expr or spread if not a spread
2318+
if let Some(expr_or_spread) = parent.to::<ExprOrSpread>() {
2319+
// these should only appear in these nodes
2320+
let is_known_parent = matches!(expr_or_spread.parent().kind(), NodeKind::NewExpr | NodeKind::ArrayLit | NodeKind::CallExpr);
2321+
debug_assert!(is_known_parent);
2322+
if is_known_parent && expr_or_spread.spread().is_none() {
2323+
return true;
2324+
}
2325+
}
2326+
2327+
// skip explicitly parsing this as a paren expr as that will be handled
2328+
// in the JSX element/fragment and it might collapse back to not having a paren expr
2329+
is_jsx_paren_expr_handled_node(&node.expr.into(), context)
2330+
}
23042331
}
23052332

23062333
fn parse_sequence_expr<'a>(node: &'a SeqExpr, context: &mut Context<'a>) -> PrintItems {
@@ -2908,8 +2935,16 @@ fn parse_jsx_closing_fragment<'a>(_: &'a JSXClosingFragment, _: &mut Context<'a>
29082935
}
29092936

29102937
fn handle_jsx_surrounding_parens<'a>(inner_items: PrintItems, context: &mut Context<'a>) -> PrintItems {
2911-
if is_jsx_paren_expr_handled_node(&context.parent(), context) || !is_jsx_paren_expr_handled_node(&context.current_node, context) {
2912-
return inner_items;
2938+
if !is_jsx_paren_expr_handled_node(&context.current_node, context) {
2939+
if should_jsx_surround_newlines(&context.current_node, context) {
2940+
return surround_with_newlines_indented_if_multi_line(inner_items, context.config.indent_width);
2941+
} else {
2942+
return inner_items;
2943+
}
2944+
}
2945+
2946+
if context.parent().is::<JSXExprContainer>() {
2947+
return surround_with_newlines_indented_if_multi_line(inner_items, context.config.indent_width);
29132948
}
29142949

29152950
let start_info = Info::new("conditionalParenStartInfo");
@@ -2919,7 +2954,7 @@ fn handle_jsx_surrounding_parens<'a>(inner_items: PrintItems, context: &mut Cont
29192954

29202955
items.push_info(start_info);
29212956
items.push_condition(if_true_or(
2922-
"isMultipleLines",
2957+
"parensOrNewlinesIfMultipleLines",
29232958
move |context| {
29242959
// clear the end info when the start info changes
29252960
if context.has_info_moved(&start_info)? {
@@ -2943,19 +2978,51 @@ fn handle_jsx_surrounding_parens<'a>(inner_items: PrintItems, context: &mut Cont
29432978
));
29442979

29452980
items.push_info(end_info);
2946-
items
2981+
return items;
2982+
2983+
fn should_jsx_surround_newlines(node: &Node, context: &Context) -> bool {
2984+
let mut parent = node.parent().unwrap();
2985+
while let Some(paren_expr) = parent.to::<ParenExpr>() {
2986+
if has_surrounding_comments(&paren_expr.expr.into(), context) {
2987+
return false;
2988+
}
2989+
parent = parent.parent().unwrap();
2990+
}
2991+
2992+
parent.is::<JSXExprContainer>()
2993+
}
29472994
}
29482995

29492996
fn is_jsx_paren_expr_handled_node(node: &Node, context: &Context) -> bool {
2950-
context.config.jsx_multi_line_parens
2951-
&& matches!(node.kind(), NodeKind::JSXElement | NodeKind::JSXFragment)
2952-
// do not allow in expr statement or argument
2953-
&& !matches!(node.parent().unwrap().kind(), NodeKind::ExprStmt
2954-
| NodeKind::ExprOrSpread
2955-
| NodeKind::JSXElement
2956-
| NodeKind::JSXFragment)
2957-
&& node.leading_comments_fast(context.module).is_empty()
2958-
&& node.trailing_comments_fast(context.module).is_empty()
2997+
if !context.config.jsx_multi_line_parens {
2998+
return false;
2999+
}
3000+
3001+
if !matches!(node.kind(), NodeKind::JSXElement | NodeKind::JSXFragment) {
3002+
return false;
3003+
}
3004+
3005+
if has_surrounding_comments(node, context) {
3006+
return false;
3007+
}
3008+
3009+
let mut parent = node.parent().unwrap();
3010+
while parent.is::<ParenExpr>() {
3011+
if has_surrounding_comments(&parent, context) {
3012+
return false;
3013+
}
3014+
parent = parent.parent().unwrap();
3015+
}
3016+
3017+
// do not allow in expr statement, argument, attributes, or jsx exprs
3018+
!matches!(
3019+
parent.kind(),
3020+
NodeKind::ExprStmt | NodeKind::ExprOrSpread | NodeKind::JSXElement | NodeKind::JSXFragment | NodeKind::JSXExprContainer
3021+
)
3022+
}
3023+
3024+
fn has_surrounding_comments(node: &Node, context: &Context) -> bool {
3025+
!node.leading_comments_fast(context.module).is_empty() || !node.trailing_comments_fast(context.module).is_empty()
29593026
}
29603027

29613028
fn parse_jsx_element<'a>(node: &'a JSXElement, context: &mut Context<'a>) -> PrintItems {

‎tests/specs/expressions/CallExpression/CallExpression_PreferSingleLine_False.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ callExpr(
66

77
[expect]
88
callExpr(
9-
(5),
9+
5,
1010
6,
1111
);

‎tests/specs/expressions/CallExpression/CallExpression_PreferSingleLine_True.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ callExpr(
55
6);
66

77
[expect]
8-
callExpr((5), 6);
8+
callExpr(5, 6);

‎tests/specs/expressions/ParenExpr/ParenExpr_All.txt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,17 @@ const test = (
2525

2626
[expect]
2727
(x.test as unknown) = 6;
28+
29+
== should ignore paren exprs within paren exprs ==
30+
(((test)));
31+
(
32+
// test
33+
((test))
34+
);
35+
36+
[expect]
37+
test;
38+
(
39+
// test
40+
test
41+
);

‎tests/specs/general/Parentheses_All.txt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
== should format with parens ==
2-
(1 + 2);
2+
1 + (1 + 2);
33

44
[expect]
5-
(1 + 2);
5+
1 + (1 + 2);
66

77
== should format on a new line when the opening paren is on a different line ==
8-
(
8+
1 + (
99
1 + 2);
1010

1111
[expect]
12-
(
12+
1 + (
1313
1 + 2
1414
);
1515

1616
== should format logical expressions in parens without hanging indentation ==
17-
(
17+
1 + (
1818
test
1919
&& test
2020
&& {
@@ -28,7 +28,7 @@
2828
);
2929

3030
[expect]
31-
(
31+
1 + (
3232
test
3333
&& test
3434
&& {
@@ -72,7 +72,7 @@ if (
7272
}
7373

7474
== should format binary expressions without hanging indentation ==
75-
(
75+
1 + (
7676
1543652546
7777
+ 2
7878
* 5 * {
@@ -87,7 +87,7 @@ if (
8787
);
8888

8989
[expect]
90-
(
90+
1 + (
9191
1543652546
9292
+ 2
9393
* 5 * {

‎tests/specs/jsx/JsxElement/JsxElement_All.txt

Lines changed: 0 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -81,46 +81,6 @@ const u = (
8181
<TestingThisOutTest></TestingThisOutTest>
8282
);
8383

84-
== should remove paren expression when the element isn't multi-line ==
85-
const t = (
86-
<Test></Test>
87-
);
88-
89-
[expect]
90-
const t = <Test></Test>;
91-
92-
== should not remove paren expression when the element isn't multi-line, but has surrounding comments ==
93-
const t = ( // test
94-
<Test></Test>
95-
);
96-
const u = (
97-
// test
98-
<Test></Test>
99-
);
100-
const v = (
101-
<Test></Test> // test
102-
);
103-
const w = (
104-
<Test></Test>
105-
// test
106-
);
107-
108-
[expect]
109-
const t = ( // test
110-
<Test></Test>
111-
);
112-
const u = (
113-
// test
114-
<Test></Test>
115-
);
116-
const v = (
117-
<Test></Test> // test
118-
);
119-
const w = (
120-
<Test></Test>
121-
// test
122-
);
123-
12484
== should make the children multi-line when they exceed the line width ==
12585
const t = <Test><Test /><Test /><Test /><Test /><Test /></Test>;
12686

@@ -260,47 +220,3 @@ return <E>Test: {type}</E>;
260220
return <E>Test: {type}</E>;
261221
return <E>{type} test</E>;
262222
return <E>{type} test</E>;
263-
264-
== should not add parens around multi-line element in an expr stmt ==
265-
<Test><Test /></Test>;
266-
267-
function test() {
268-
<Test><Test /></Test>;
269-
}
270-
271-
[expect]
272-
<Test>
273-
<Test />
274-
</Test>;
275-
276-
function test() {
277-
<Test>
278-
<Test />
279-
</Test>;
280-
}
281-
282-
== should not add parens around an element in an argument ==
283-
ReactDOM.render(
284-
<React.StrictMode>
285-
<App />
286-
</React.StrictMode>,
287-
document.getElementById("root"),
288-
);
289-
new MyClass(
290-
<React.StrictMode>
291-
<App />
292-
</React.StrictMode>,
293-
);
294-
295-
[expect]
296-
ReactDOM.render(
297-
<React.StrictMode>
298-
<App />
299-
</React.StrictMode>,
300-
document.getElementById("root"),
301-
);
302-
new MyClass(
303-
<React.StrictMode>
304-
<App />
305-
</React.StrictMode>,
306-
);

0 commit comments

Comments
 (0)