Skip to content

Commit d8aacb8

Browse files
fix(rome_js_parser): Better error recover for jsx attribute rome#2944 (rome#2954)
Co-authored-by: Micha Reiser <micha@reiser.io>
1 parent cc95dd5 commit d8aacb8

File tree

10 files changed

+170
-34
lines changed

10 files changed

+170
-34
lines changed

crates/rome_js_analyze/tests/specs/jsx/noImplicitBoolean.jsx

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
<input disabled={0} />;
55
<input disabled={undefined} />;
66
<input disabled="false" />;
7+
// https://github.com/rome/tools/issues/2944
8+
<div className={asdf asdf} />;
79

810
//invalid
911
<input disabled />;

crates/rome_js_analyze/tests/specs/jsx/noImplicitBoolean.jsx.snap

+32-29
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
---
22
source: crates/rome_js_analyze/tests/spec_tests.rs
3+
assertion_line: 89
34
expression: noImplicitBoolean.jsx
45
---
56
# Input
@@ -10,6 +11,8 @@ expression: noImplicitBoolean.jsx
1011
<input disabled={0} />;
1112
<input disabled={undefined} />;
1213
<input disabled="false" />;
14+
// https://github.com/rome/tools/issues/2944
15+
<div className={asdf asdf} />;
1316
1417
//invalid
1518
<input disabled />;
@@ -21,57 +24,57 @@ expression: noImplicitBoolean.jsx
2124
# Diagnostics
2225
```
2326
warning[jsx/noImplicitBoolean]: Use explicit boolean values for boolean JSX props.
24-
┌─ noImplicitBoolean.jsx:9:8
25-
26-
9 │ <input disabled />;
27-
│ --------
27+
┌─ noImplicitBoolean.jsx:11:8
28+
29+
11 │ <input disabled />;
30+
│ --------
2831
2932
Safe fix: Add explicit `true` literal for this attribute
30-
| @@ -6,6 +6,6 @@
31-
5 5 | <input disabled="false" />;
32-
6 6 |
33-
7 7 | //invalid
34-
8 | - <input disabled />;
35-
8 | + <input disabled={true} />;
36-
9 9 | <input accept/** some comment */ />;
37-
10 10 | <input /** some comment */ accept />;
33+
| @@ -8,6 +8,6 @@
34+
7 7 | <div className={asdf asdf} />;
35+
8 8 |
36+
9 9 | //invalid
37+
10 | - <input disabled />;
38+
10 | + <input disabled={true} />;
39+
11 11 | <input accept/** some comment */ />;
40+
12 12 | <input /** some comment */ accept />;
3841
3942
4043
```
4144

4245
```
4346
warning[jsx/noImplicitBoolean]: Use explicit boolean values for boolean JSX props.
44-
┌─ noImplicitBoolean.jsx:10:8
47+
┌─ noImplicitBoolean.jsx:12:8
4548
46-
10 │ <input accept/** some comment */ />;
49+
12 │ <input accept/** some comment */ />;
4750
│ ------
4851
4952
Safe fix: Add explicit `true` literal for this attribute
50-
| @@ -7,5 +7,5 @@
51-
6 6 |
52-
7 7 | //invalid
53-
8 8 | <input disabled />;
54-
9 | - <input accept/** some comment */ />;
55-
9 | + <input accept={true}/** some comment */ />;
56-
10 10 | <input /** some comment */ accept />;
53+
| @@ -9,5 +9,5 @@
54+
8 8 |
55+
9 9 | //invalid
56+
10 10 | <input disabled />;
57+
11 | - <input accept/** some comment */ />;
58+
11 | + <input accept={true}/** some comment */ />;
59+
12 12 | <input /** some comment */ accept />;
5760
5861
5962
```
6063

6164
```
6265
warning[jsx/noImplicitBoolean]: Use explicit boolean values for boolean JSX props.
63-
┌─ noImplicitBoolean.jsx:11:28
66+
┌─ noImplicitBoolean.jsx:13:28
6467
65-
11 │ <input /** some comment */ accept />;
68+
13 │ <input /** some comment */ accept />;
6669
│ ------
6770
6871
Safe fix: Add explicit `true` literal for this attribute
69-
| @@ -8,4 +8,4 @@
70-
7 7 | //invalid
71-
8 8 | <input disabled />;
72-
9 9 | <input accept/** some comment */ />;
73-
10 | - <input /** some comment */ accept />;
74-
10 | + <input /** some comment */ accept={true} />;
72+
| @@ -10,4 +10,4 @@
73+
9 9 | //invalid
74+
10 10 | <input disabled />;
75+
11 11 | <input accept/** some comment */ />;
76+
12 | - <input /** some comment */ accept />;
77+
12 | + <input /** some comment */ accept={true} />;
7578
7679
7780
```

crates/rome_js_formatter/src/jsx/attribute/expression_attribute_value.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::prelude::*;
33
use rome_formatter::write;
44
use rome_js_syntax::{
55
JsAnyExpression, JsxExpressionAttributeValue, JsxExpressionAttributeValueFields,
6+
TriviaPieceKind,
67
};
78

89
#[derive(Debug, Clone, Default)]
@@ -19,7 +20,6 @@ impl FormatNodeRule<JsxExpressionAttributeValue> for FormatJsxExpressionAttribut
1920
expression,
2021
r_curly_token,
2122
} = node.as_fields();
22-
2323
write!(
2424
f,
2525
[group(&format_with(|f| {
@@ -70,7 +70,19 @@ impl FormatNodeRule<JsxExpressionAttributeValue> for FormatJsxExpressionAttribut
7070
write!(f, [soft_block_indent(&expression.format())])?;
7171
};
7272

73-
write!(f, [line_suffix_boundary(), r_curly_token.format()])
73+
write!(f, [line_suffix_boundary(),])?;
74+
75+
// format if `}` has a `Skipped` leading trivia
76+
// <div className={asdf asdf} />;
77+
let r_curly_token_ref = r_curly_token.as_ref()?;
78+
if matches!(
79+
r_curly_token_ref.leading_trivia().first().map(|t| t.kind()),
80+
Some(TriviaPieceKind::Skipped)
81+
) {
82+
write!(f, [space(), r_curly_token.format()])
83+
} else {
84+
write!(f, [r_curly_token.format()])
85+
}
7486
}))]
7587
)
7688
}

crates/rome_js_formatter/tests/specs/jsx/attributes.jsx

+6
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,9 @@
4343

4444
<test {...WithAVeryLongFunctionthat_exceeds_the_line_width_what_happens_with_ithis()} />;
4545
<div {...["Chungking Express", "Fallen Angels", "In the Mood for Love", "Days of Living Wild", "Happy Together"]}/>;
46+
47+
48+
// https://github.com/rome/tools/issues/2944
49+
<div className={asdf asdf} />;
50+
<div className={asdf
51+
/* comment */ asdf } />

crates/rome_js_formatter/tests/specs/jsx/attributes.jsx.snap

+14-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
source: crates/rome_js_formatter/tests/spec_test.rs
3-
assertion_line: 257
3+
assertion_line: 259
44
expression: attributes.jsx
55
---
66
# Input
@@ -50,6 +50,11 @@ expression: attributes.jsx
5050
<test {...WithAVeryLongFunctionthat_exceeds_the_line_width_what_happens_with_ithis()} />;
5151
<div {...["Chungking Express", "Fallen Angels", "In the Mood for Love", "Days of Living Wild", "Happy Together"]}/>;
5252

53+
54+
// https://github.com/rome/tools/issues/2944
55+
<div className={asdf asdf} />;
56+
<div className={asdf
57+
/* comment */ asdf } />
5358
=============================
5459
# Outputs
5560
## Output 1
@@ -112,6 +117,14 @@ let a = (
112117
]}
113118
/>;
114119

120+
// https://github.com/rome/tools/issues/2944
121+
<div className={asdf asdf} />;
122+
<div
123+
className={
124+
asdf
125+
/* comment */asdf }
126+
/>;
127+
115128

116129
## Lines exceeding width of 80 characters
117130

crates/rome_js_parser/src/syntax/jsx/mod.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,8 @@ fn parse_jsx_attribute_value(p: &mut Parser) -> ParsedSyntax {
605605
_ => ParsedSyntax::Absent,
606606
}
607607
}
608-
608+
// test_err jsx jsx_element_attribute_expression_error
609+
// <div className={asdf asdf} />;
609610
fn parse_jsx_expression_attribute_value(p: &mut Parser) -> ParsedSyntax {
610611
if !p.at(T!['{']) {
611612
return ParsedSyntax::Absent;
@@ -614,7 +615,12 @@ fn parse_jsx_expression_attribute_value(p: &mut Parser) -> ParsedSyntax {
614615
let m = p.start();
615616
p.bump(T!['{']);
616617
parse_jsx_assignment_expression(p, false).or_add_diagnostic(p, expected_expression);
617-
p.expect(T!['}']);
618+
if !p.expect(T!['}']) && p.nth_at(1, T!['}']) {
619+
p.parse_as_skipped_trivia_tokens(|p| {
620+
p.bump_any();
621+
});
622+
p.expect(T!['}']);
623+
}
618624

619625
ParsedSyntax::Present(m.complete(p, JSX_EXPRESSION_ATTRIBUTE_VALUE))
620626
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<div className={asdf asdf} />;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
JsModule {
2+
interpreter_token: missing (optional),
3+
directives: JsDirectiveList [],
4+
items: JsModuleItemList [
5+
JsExpressionStatement {
6+
expression: JsxTagExpression {
7+
tag: JsxSelfClosingElement {
8+
l_angle_token: L_ANGLE@0..1 "<" [] [],
9+
name: JsxName {
10+
value_token: JSX_IDENT@1..5 "div" [] [Whitespace(" ")],
11+
},
12+
type_arguments: missing (optional),
13+
attributes: JsxAttributeList [
14+
JsxAttribute {
15+
name: JsxName {
16+
value_token: JSX_IDENT@5..14 "className" [] [],
17+
},
18+
initializer: JsxAttributeInitializerClause {
19+
eq_token: EQ@14..15 "=" [] [],
20+
value: JsxExpressionAttributeValue {
21+
l_curly_token: L_CURLY@15..16 "{" [] [],
22+
expression: JsIdentifierExpression {
23+
name: JsReferenceIdentifier {
24+
value_token: IDENT@16..21 "asdf" [] [Whitespace(" ")],
25+
},
26+
},
27+
r_curly_token: R_CURLY@21..27 "}" [Skipped("asdf")] [Whitespace(" ")],
28+
},
29+
},
30+
},
31+
],
32+
slash_token: SLASH@27..28 "/" [] [],
33+
r_angle_token: R_ANGLE@28..29 ">" [] [],
34+
},
35+
},
36+
semicolon_token: SEMICOLON@29..30 ";" [] [],
37+
},
38+
],
39+
eof_token: EOF@30..31 "" [Newline("\n")] [],
40+
}
41+
42+
0: JS_MODULE@0..31
43+
0: (empty)
44+
1: JS_DIRECTIVE_LIST@0..0
45+
2: JS_MODULE_ITEM_LIST@0..30
46+
0: JS_EXPRESSION_STATEMENT@0..30
47+
0: JSX_TAG_EXPRESSION@0..29
48+
0: JSX_SELF_CLOSING_ELEMENT@0..29
49+
0: L_ANGLE@0..1 "<" [] []
50+
1: JSX_NAME@1..5
51+
0: JSX_IDENT@1..5 "div" [] [Whitespace(" ")]
52+
2: (empty)
53+
3: JSX_ATTRIBUTE_LIST@5..27
54+
0: JSX_ATTRIBUTE@5..27
55+
0: JSX_NAME@5..14
56+
0: JSX_IDENT@5..14 "className" [] []
57+
1: JSX_ATTRIBUTE_INITIALIZER_CLAUSE@14..27
58+
0: EQ@14..15 "=" [] []
59+
1: JSX_EXPRESSION_ATTRIBUTE_VALUE@15..27
60+
0: L_CURLY@15..16 "{" [] []
61+
1: JS_IDENTIFIER_EXPRESSION@16..21
62+
0: JS_REFERENCE_IDENTIFIER@16..21
63+
0: IDENT@16..21 "asdf" [] [Whitespace(" ")]
64+
2: R_CURLY@21..27 "}" [Skipped("asdf")] [Whitespace(" ")]
65+
4: SLASH@27..28 "/" [] []
66+
5: R_ANGLE@28..29 ">" [] []
67+
1: SEMICOLON@29..30 ";" [] []
68+
3: EOF@30..31 "" [Newline("\n")] []
69+
--
70+
error[SyntaxError]: expected `}` but instead found `asdf`
71+
┌─ jsx_element_attribute_expression_error.jsx:1:22
72+
73+
1 │ <div className={asdf asdf} />;
74+
│ ^^^^ unexpected
75+
76+
--
77+
<div className={asdf asdf} />;

crates/rome_rowan/src/cursor/trivia.rs

+5
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ impl SyntaxTrivia {
7474
self.green_trivia().pieces().last()
7575
}
7676

77+
/// Returns the first trivia piece element
78+
pub(crate) fn first(&self) -> Option<&TriviaPiece> {
79+
self.green_trivia().pieces().first()
80+
}
81+
7782
/// Iterate over all pieces of the trivia. The iterator returns the offset
7883
/// of the trivia as [TextSize] and its data as [Trivia], which contains its length.
7984
/// See [SyntaxTriviaPiece].

crates/rome_rowan/src/syntax/trivia.rs

+11
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,17 @@ impl<L: Language> SyntaxTrivia<L> {
608608
})
609609
}
610610

611+
pub fn first(&self) -> Option<SyntaxTriviaPiece<L>> {
612+
let piece = self.raw.first()?;
613+
614+
Some(SyntaxTriviaPiece {
615+
raw: self.raw.clone(),
616+
offset: self.raw.text_range().end() - piece.length,
617+
trivia: *piece,
618+
_p: Default::default(),
619+
})
620+
}
621+
611622
pub fn text(&self) -> &str {
612623
self.raw.text()
613624
}

0 commit comments

Comments
 (0)