Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_parser): Better error recover for jsx attribute #2944 (#2954
Browse files Browse the repository at this point in the history
)

Co-authored-by: Micha Reiser <micha@reiser.io>
  • Loading branch information
IWANABETHATGUY and MichaReiser authored Aug 2, 2022
1 parent 08b49db commit ec92885
Show file tree
Hide file tree
Showing 10 changed files with 170 additions and 34 deletions.
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/tests/specs/jsx/noImplicitBoolean.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
<input disabled={0} />;
<input disabled={undefined} />;
<input disabled="false" />;
// https://github.com/rome/tools/issues/2944
<div className={asdf asdf} />;

//invalid
<input disabled />;
Expand Down
61 changes: 32 additions & 29 deletions crates/rome_js_analyze/tests/specs/jsx/noImplicitBoolean.jsx.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
assertion_line: 89
expression: noImplicitBoolean.jsx
---
# Input
Expand All @@ -10,6 +11,8 @@ expression: noImplicitBoolean.jsx
<input disabled={0} />;
<input disabled={undefined} />;
<input disabled="false" />;
// https://github.com/rome/tools/issues/2944
<div className={asdf asdf} />;
//invalid
<input disabled />;
Expand All @@ -21,57 +24,57 @@ expression: noImplicitBoolean.jsx
# Diagnostics
```
warning[jsx/noImplicitBoolean]: Use explicit boolean values for boolean JSX props.
┌─ noImplicitBoolean.jsx:9:8
9 │ <input disabled />;
│ --------
┌─ noImplicitBoolean.jsx:11:8
11 │ <input disabled />;
│ --------
Safe fix: Add explicit `true` literal for this attribute
| @@ -6,6 +6,6 @@
5 5 | <input disabled="false" />;
6 6 |
7 7 | //invalid
8 | - <input disabled />;
8 | + <input disabled={true} />;
9 9 | <input accept/** some comment */ />;
10 10 | <input /** some comment */ accept />;
| @@ -8,6 +8,6 @@
7 7 | <div className={asdf asdf} />;
8 8 |
9 9 | //invalid
10 | - <input disabled />;
10 | + <input disabled={true} />;
11 11 | <input accept/** some comment */ />;
12 12 | <input /** some comment */ accept />;
```

```
warning[jsx/noImplicitBoolean]: Use explicit boolean values for boolean JSX props.
┌─ noImplicitBoolean.jsx:10:8
┌─ noImplicitBoolean.jsx:12:8
10 │ <input accept/** some comment */ />;
12 │ <input accept/** some comment */ />;
│ ------
Safe fix: Add explicit `true` literal for this attribute
| @@ -7,5 +7,5 @@
6 6 |
7 7 | //invalid
8 8 | <input disabled />;
9 | - <input accept/** some comment */ />;
9 | + <input accept={true}/** some comment */ />;
10 10 | <input /** some comment */ accept />;
| @@ -9,5 +9,5 @@
8 8 |
9 9 | //invalid
10 10 | <input disabled />;
11 | - <input accept/** some comment */ />;
11 | + <input accept={true}/** some comment */ />;
12 12 | <input /** some comment */ accept />;
```

```
warning[jsx/noImplicitBoolean]: Use explicit boolean values for boolean JSX props.
┌─ noImplicitBoolean.jsx:11:28
┌─ noImplicitBoolean.jsx:13:28
11 │ <input /** some comment */ accept />;
13 │ <input /** some comment */ accept />;
│ ------
Safe fix: Add explicit `true` literal for this attribute
| @@ -8,4 +8,4 @@
7 7 | //invalid
8 8 | <input disabled />;
9 9 | <input accept/** some comment */ />;
10 | - <input /** some comment */ accept />;
10 | + <input /** some comment */ accept={true} />;
| @@ -10,4 +10,4 @@
9 9 | //invalid
10 10 | <input disabled />;
11 11 | <input accept/** some comment */ />;
12 | - <input /** some comment */ accept />;
12 | + <input /** some comment */ accept={true} />;
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::prelude::*;
use rome_formatter::write;
use rome_js_syntax::{
JsAnyExpression, JsxExpressionAttributeValue, JsxExpressionAttributeValueFields,
TriviaPieceKind,
};

#[derive(Debug, Clone, Default)]
Expand All @@ -19,7 +20,6 @@ impl FormatNodeRule<JsxExpressionAttributeValue> for FormatJsxExpressionAttribut
expression,
r_curly_token,
} = node.as_fields();

write!(
f,
[group(&format_with(|f| {
Expand Down Expand Up @@ -70,7 +70,19 @@ impl FormatNodeRule<JsxExpressionAttributeValue> for FormatJsxExpressionAttribut
write!(f, [soft_block_indent(&expression.format())])?;
};

write!(f, [line_suffix_boundary(), r_curly_token.format()])
write!(f, [line_suffix_boundary(),])?;

// format if `}` has a `Skipped` leading trivia
// <div className={asdf asdf} />;
let r_curly_token_ref = r_curly_token.as_ref()?;
if matches!(
r_curly_token_ref.leading_trivia().first().map(|t| t.kind()),
Some(TriviaPieceKind::Skipped)
) {
write!(f, [space(), r_curly_token.format()])
} else {
write!(f, [r_curly_token.format()])
}
}))]
)
}
Expand Down
6 changes: 6 additions & 0 deletions crates/rome_js_formatter/tests/specs/jsx/attributes.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,9 @@

<test {...WithAVeryLongFunctionthat_exceeds_the_line_width_what_happens_with_ithis()} />;
<div {...["Chungking Express", "Fallen Angels", "In the Mood for Love", "Days of Living Wild", "Happy Together"]}/>;


// https://github.com/rome/tools/issues/2944
<div className={asdf asdf} />;
<div className={asdf
/* comment */ asdf } />
15 changes: 14 additions & 1 deletion crates/rome_js_formatter/tests/specs/jsx/attributes.jsx.snap
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: crates/rome_js_formatter/tests/spec_test.rs
assertion_line: 257
assertion_line: 259
expression: attributes.jsx
---
# Input
Expand Down Expand Up @@ -50,6 +50,11 @@ expression: attributes.jsx
<test {...WithAVeryLongFunctionthat_exceeds_the_line_width_what_happens_with_ithis()} />;
<div {...["Chungking Express", "Fallen Angels", "In the Mood for Love", "Days of Living Wild", "Happy Together"]}/>;


// https://github.com/rome/tools/issues/2944
<div className={asdf asdf} />;
<div className={asdf
/* comment */ asdf } />
=============================
# Outputs
## Output 1
Expand Down Expand Up @@ -112,6 +117,14 @@ let a = (
]}
/>;

// https://github.com/rome/tools/issues/2944
<div className={asdf asdf} />;
<div
className={
asdf
/* comment */asdf }
/>;


## Lines exceeding width of 80 characters

Expand Down
10 changes: 8 additions & 2 deletions crates/rome_js_parser/src/syntax/jsx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,8 @@ fn parse_jsx_attribute_value(p: &mut Parser) -> ParsedSyntax {
_ => ParsedSyntax::Absent,
}
}

// test_err jsx jsx_element_attribute_expression_error
// <div className={asdf asdf} />;
fn parse_jsx_expression_attribute_value(p: &mut Parser) -> ParsedSyntax {
if !p.at(T!['{']) {
return ParsedSyntax::Absent;
Expand All @@ -614,7 +615,12 @@ fn parse_jsx_expression_attribute_value(p: &mut Parser) -> ParsedSyntax {
let m = p.start();
p.bump(T!['{']);
parse_jsx_assignment_expression(p, false).or_add_diagnostic(p, expected_expression);
p.expect(T!['}']);
if !p.expect(T!['}']) && p.nth_at(1, T!['}']) {
p.parse_as_skipped_trivia_tokens(|p| {
p.bump_any();
});
p.expect(T!['}']);
}

ParsedSyntax::Present(m.complete(p, JSX_EXPRESSION_ATTRIBUTE_VALUE))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div className={asdf asdf} />;
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
JsModule {
interpreter_token: missing (optional),
directives: JsDirectiveList [],
items: JsModuleItemList [
JsExpressionStatement {
expression: JsxTagExpression {
tag: JsxSelfClosingElement {
l_angle_token: L_ANGLE@0..1 "<" [] [],
name: JsxName {
value_token: JSX_IDENT@1..5 "div" [] [Whitespace(" ")],
},
type_arguments: missing (optional),
attributes: JsxAttributeList [
JsxAttribute {
name: JsxName {
value_token: JSX_IDENT@5..14 "className" [] [],
},
initializer: JsxAttributeInitializerClause {
eq_token: EQ@14..15 "=" [] [],
value: JsxExpressionAttributeValue {
l_curly_token: L_CURLY@15..16 "{" [] [],
expression: JsIdentifierExpression {
name: JsReferenceIdentifier {
value_token: IDENT@16..21 "asdf" [] [Whitespace(" ")],
},
},
r_curly_token: R_CURLY@21..27 "}" [Skipped("asdf")] [Whitespace(" ")],
},
},
},
],
slash_token: SLASH@27..28 "/" [] [],
r_angle_token: R_ANGLE@28..29 ">" [] [],
},
},
semicolon_token: SEMICOLON@29..30 ";" [] [],
},
],
eof_token: EOF@30..31 "" [Newline("\n")] [],
}

0: JS_MODULE@0..31
0: (empty)
1: JS_DIRECTIVE_LIST@0..0
2: JS_MODULE_ITEM_LIST@0..30
0: JS_EXPRESSION_STATEMENT@0..30
0: JSX_TAG_EXPRESSION@0..29
0: JSX_SELF_CLOSING_ELEMENT@0..29
0: L_ANGLE@0..1 "<" [] []
1: JSX_NAME@1..5
0: JSX_IDENT@1..5 "div" [] [Whitespace(" ")]
2: (empty)
3: JSX_ATTRIBUTE_LIST@5..27
0: JSX_ATTRIBUTE@5..27
0: JSX_NAME@5..14
0: JSX_IDENT@5..14 "className" [] []
1: JSX_ATTRIBUTE_INITIALIZER_CLAUSE@14..27
0: EQ@14..15 "=" [] []
1: JSX_EXPRESSION_ATTRIBUTE_VALUE@15..27
0: L_CURLY@15..16 "{" [] []
1: JS_IDENTIFIER_EXPRESSION@16..21
0: JS_REFERENCE_IDENTIFIER@16..21
0: IDENT@16..21 "asdf" [] [Whitespace(" ")]
2: R_CURLY@21..27 "}" [Skipped("asdf")] [Whitespace(" ")]
4: SLASH@27..28 "/" [] []
5: R_ANGLE@28..29 ">" [] []
1: SEMICOLON@29..30 ";" [] []
3: EOF@30..31 "" [Newline("\n")] []
--
error[SyntaxError]: expected `}` but instead found `asdf`
┌─ jsx_element_attribute_expression_error.jsx:1:22
1 │ <div className={asdf asdf} />;
│ ^^^^ unexpected

--
<div className={asdf asdf} />;
5 changes: 5 additions & 0 deletions crates/rome_rowan/src/cursor/trivia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ impl SyntaxTrivia {
self.green_trivia().pieces().last()
}

/// Returns the first trivia piece element
pub(crate) fn first(&self) -> Option<&TriviaPiece> {
self.green_trivia().pieces().first()
}

/// Iterate over all pieces of the trivia. The iterator returns the offset
/// of the trivia as [TextSize] and its data as [Trivia], which contains its length.
/// See [SyntaxTriviaPiece].
Expand Down
11 changes: 11 additions & 0 deletions crates/rome_rowan/src/syntax/trivia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,17 @@ impl<L: Language> SyntaxTrivia<L> {
})
}

pub fn first(&self) -> Option<SyntaxTriviaPiece<L>> {
let piece = self.raw.first()?;

Some(SyntaxTriviaPiece {
raw: self.raw.clone(),
offset: self.raw.text_range().end() - piece.length,
trivia: *piece,
_p: Default::default(),
})
}

pub fn text(&self) -> &str {
self.raw.text()
}
Expand Down

0 comments on commit ec92885

Please sign in to comment.