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

fix(rome_js_parser): permisive instantiation expression #3359

Closed
wants to merge 4 commits into from
Closed

fix(rome_js_parser): permisive instantiation expression #3359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
022f67f
chore: 🤖 finish
IWANABETHATGUY Oct 7, 2022
e9ba015
chore: 🤖 refactor
IWANABETHATGUY Oct 7, 2022
90b759b
chore: 🤖 add some doc
IWANABETHATGUY Oct 10, 2022
f3b3447
chore: 🤖 more doc
IWANABETHATGUY Oct 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 73 additions & 29 deletions crates/rome_js_parser/src/syntax/typescript/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,9 @@ fn parse_ts_type_predicate(p: &mut Parser) -> ParsedSyntax {

// test ts ts_type_instantiation_expression
// type StringBox = Box<string>;
// // Parsed as instantiation expression
// const x4 = f<true>
// if (true) {}

// test_err ts ts_instantiation_expressions1
// const a8 = f<number><number>; // Relational operator error
Expand All @@ -1284,7 +1287,7 @@ pub(crate) fn parse_ts_type_arguments_in_expression(p: &mut Parser) -> ParsedSyn
p.re_lex(ReLexContext::TypeArgumentLessThan);
let arguments = parse_ts_type_arguments_impl(p, false);

if p.last() == Some(T![>]) && can_follow_type_arguments_in_expr(p.cur()) {
if p.last() == Some(T![>]) && can_follow_type_arguments_in_expr(p) {
Ok(Present(arguments))
} else {
Err(())
Expand All @@ -1294,35 +1297,76 @@ pub(crate) fn parse_ts_type_arguments_in_expression(p: &mut Parser) -> ParsedSyn
}

#[inline]
pub fn can_follow_type_arguments_in_expr(cur_kind: JsSyntaxKind) -> bool {
matches!(
cur_kind,
T!['(']
fn can_follow_type_arguments_in_expr(p: &mut Parser) -> bool {
let cur_kind = p.cur();
match cur_kind {
T!['('] | BACKTICK => true,
_ => !is_start_of_expr(p),
}
}

/// Checking if parser is at start of expression with some error recovery logic in it, e.g.
/// we counted `binary_operator` as start of an expression, but `binary_operator` is not part of expression actually
/// For more details, you could refer to https://github.com/microsoft/TypeScript/blob/42b1049aee8c655631cb4f0065de86ec1023d20a/src/compiler/parser.ts#L4475
fn is_start_of_expr(p: &mut Parser) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

How's this method different from is_at_expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is_start_of_expr has JS_BIG_INT_LITERAL, but is_at_expression don't. is_start_of_expr don't have T![super]

Copy link
Contributor

Choose a reason for hiding this comment

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

Have we checked if is_at_expression should have JS_BIG_INT_LITERAL and T![super]. Maybe that's a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is_at_expression, Also is_start_of_expr need is_binary_operator for better error recover. I am not sure if we need it too, becuase our error recover maybe different from Typescript

if is_start_of_left_hand_side_expression(p) {
return true;
}
match p.cur() {
T![+]
| T![-]
| T![~]
| T![!]
| T![delete]
| T![typeof]
| T![void]
| T![++]
| T![--]
| T![<]
| T![await]
| T![yield] => true,
// TODO: how to represent private identifier
// Error tolerance. If we see the start of some binary operator, we consider
// that the start of an expression. That way we'll parse out a missing identifier,
// give a good message about an identifier being missing, and then consume the
// rest of the binary expression.
_ => is_binary_operator(p),
}
}

fn is_binary_operator(p: &mut Parser) -> bool {
// TODO: support Optional Variance Annotations
// https://github.dev/microsoft/TypeScript/blob/42b1049aee8c655631cb4f0065de86ec1023d20a/src/compiler/parser.ts#L5142-L5144

// In typescript, the operatorPrecedence of `Comma` is 0,which means we need to ensure the `OperatorPrecedence` is bigger than `Comma`
// For more details, you could refer to (https://github.com/microsoft/TypeScript/blob/42b1049aee8c655631cb4f0065de86ec1023d20a/src/compiler/utilities.ts#L3555), https://github.com/microsoft/TypeScript/blob/42b1049aee8c655631cb4f0065de86ec1023d20a/src/compiler/parser.ts#L5146

matches!(OperatorPrecedence::try_from_binary_operator(p.cur()), Some(precedence) if precedence > OperatorPrecedence::Comma)
}

/// Checking if parser at left_hand_side_expression
fn is_start_of_left_hand_side_expression(p: &mut Parser) -> bool {
match p.cur() {
T![super]
| T![null]
| T![true]
| T![false]
| JS_NUMBER_LITERAL
| JS_BIG_INT_LITERAL
| JS_STRING_LITERAL
| BACKTICK
// These tokens can't follow in a call expression, nor can they start an
// expression. So, consider the type argument list part of an instantiation
// expression.
| T![,]
| T![.]
| T![?.]
| T![')']
| T![']']
| T![:]
| T![;]
| T![?]
| T![==]
| T![===]
| T![!=]
| T![!==]
| T![&&]
| T![||]
| T![??]
| T![^]
| T![&]
| T![|]
| T!['}']
| EOF
)
| T!['(']
| T!['{']
| T!['[']
| T![function]
| T![class]
| T![new]
| T![/]
| T![/=] => true,
T![import] => p.nth_at(1, T!['(']) || p.nth_at(1, T![<]),

_ => is_at_identifier(p),
}
}

pub(crate) fn parse_ts_type_arguments(p: &mut Parser) -> ParsedSyntax {
Expand Down
87 changes: 83 additions & 4 deletions crates/rome_js_parser/test_data/inline/ok/ts_type_instantiation_expression.rast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,61 @@ JsModule {
},
semicolon_token: SEMICOLON@28..29 ";" [] [],
},
JsVariableStatement {
declaration: JsVariableDeclaration {
kind: CONST_KW@29..74 "const" [Newline("\n"), Comments("// Parsed as instanti ..."), Newline("\n")] [Whitespace(" ")],
declarators: JsVariableDeclaratorList [
JsVariableDeclarator {
id: JsIdentifierBinding {
name_token: IDENT@74..77 "x4" [] [Whitespace(" ")],
},
variable_annotation: missing (optional),
initializer: JsInitializerClause {
eq_token: EQ@77..79 "=" [] [Whitespace(" ")],
expression: TsInstantiationExpression {
expression: JsIdentifierExpression {
name: JsReferenceIdentifier {
value_token: IDENT@79..80 "f" [] [],
},
},
arguments: TsTypeArguments {
l_angle_token: L_ANGLE@80..81 "<" [] [],
ts_type_argument_list: TsTypeArgumentList [
TsBooleanLiteralType {
literal: TRUE_KW@81..85 "true" [] [],
},
],
r_angle_token: R_ANGLE@85..86 ">" [] [],
},
},
},
},
],
},
semicolon_token: missing (optional),
},
JsIfStatement {
if_token: IF_KW@86..90 "if" [Newline("\n")] [Whitespace(" ")],
l_paren_token: L_PAREN@90..91 "(" [] [],
test: JsBooleanLiteralExpression {
value_token: TRUE_KW@91..95 "true" [] [],
},
r_paren_token: R_PAREN@95..97 ")" [] [Whitespace(" ")],
consequent: JsBlockStatement {
l_curly_token: L_CURLY@97..98 "{" [] [],
statements: JsStatementList [],
r_curly_token: R_CURLY@98..99 "}" [] [],
},
else_clause: missing (optional),
},
],
eof_token: EOF@29..30 "" [Newline("\n")] [],
eof_token: EOF@99..100 "" [Newline("\n")] [],
}

0: JS_MODULE@0..30
0: JS_MODULE@0..100
0: (empty)
1: JS_DIRECTIVE_LIST@0..0
2: JS_MODULE_ITEM_LIST@0..29
2: JS_MODULE_ITEM_LIST@0..99
0: TS_TYPE_ALIAS_DECLARATION@0..29
0: TYPE_KW@0..5 "type" [] [Whitespace(" ")]
1: TS_IDENTIFIER_BINDING@5..15
Expand All @@ -49,4 +96,36 @@ JsModule {
0: STRING_KW@21..27 "string" [] []
2: R_ANGLE@27..28 ">" [] []
5: SEMICOLON@28..29 ";" [] []
3: EOF@29..30 "" [Newline("\n")] []
1: JS_VARIABLE_STATEMENT@29..86
0: JS_VARIABLE_DECLARATION@29..86
0: CONST_KW@29..74 "const" [Newline("\n"), Comments("// Parsed as instanti ..."), Newline("\n")] [Whitespace(" ")]
1: JS_VARIABLE_DECLARATOR_LIST@74..86
0: JS_VARIABLE_DECLARATOR@74..86
0: JS_IDENTIFIER_BINDING@74..77
0: IDENT@74..77 "x4" [] [Whitespace(" ")]
1: (empty)
2: JS_INITIALIZER_CLAUSE@77..86
0: EQ@77..79 "=" [] [Whitespace(" ")]
1: TS_INSTANTIATION_EXPRESSION@79..86
0: JS_IDENTIFIER_EXPRESSION@79..80
0: JS_REFERENCE_IDENTIFIER@79..80
0: IDENT@79..80 "f" [] []
1: TS_TYPE_ARGUMENTS@80..86
0: L_ANGLE@80..81 "<" [] []
1: TS_TYPE_ARGUMENT_LIST@81..85
0: TS_BOOLEAN_LITERAL_TYPE@81..85
0: TRUE_KW@81..85 "true" [] []
2: R_ANGLE@85..86 ">" [] []
1: (empty)
2: JS_IF_STATEMENT@86..99
0: IF_KW@86..90 "if" [Newline("\n")] [Whitespace(" ")]
1: L_PAREN@90..91 "(" [] []
2: JS_BOOLEAN_LITERAL_EXPRESSION@91..95
0: TRUE_KW@91..95 "true" [] []
3: R_PAREN@95..97 ")" [] [Whitespace(" ")]
4: JS_BLOCK_STATEMENT@97..99
0: L_CURLY@97..98 "{" [] []
1: JS_STATEMENT_LIST@98..98
2: R_CURLY@98..99 "}" [] []
5: (empty)
3: EOF@99..100 "" [Newline("\n")] []
3 changes: 3 additions & 0 deletions crates/rome_js_parser/test_data/inline/ok/ts_type_instantiation_expression.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
type StringBox = Box<string>;
// Parsed as instantiation expression
const x4 = f<true>
if (true) {}