Skip to content

Commit

Permalink
fix(typescript): fix parsing of 'c ? (p): T => b : f'
Browse files Browse the repository at this point in the history
':' serves two purposes: mark type annotations (e.g. for arrow function
return types, as in '(p): T => b') and delimit the false branch of a
conditional expression (as in 'c ? t : f'). Our parser gets confused in
cases where the two are mixed.

Follow TypeScript's behavior and use a transaction, backtracking when
needed.
  • Loading branch information
strager committed Dec 10, 2023
1 parent fa4535e commit 7a17bc9
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 2 deletions.
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ Semantic Versioning.
[E0426][] ("type predicates are only allowed as function return types").
* `export default` now supports separately-declared TypeScript interfaces and
types.
* `cond ? (param): ReturnType => body : f` is now correctly parsed as a
conditional expression with a function in the truthy branch.
(`cond ? (t) : param => body` continues to be parsed as a conditional
expression with a function in the falsy branch.)

### Fixed

Expand Down
51 changes: 49 additions & 2 deletions src/quick-lint-js/fe/parse-expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1952,7 +1952,8 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v,
} else {
true_expression = this->parse_expression(
v, Precedence{
.colon_type_annotation = Allow_Type_Annotations::never,
.colon_type_annotation = Allow_Type_Annotations::
typescript_only_if_legal_as_conditional_true_branch,
});
}

Expand Down Expand Up @@ -2105,8 +2106,14 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v,

// x: Type // TypeScript only.
case Token_Type::colon: {
Expression* child = binary_builder.last_expression();
bool should_parse_annotation;
switch (prec.colon_type_annotation) {
case Allow_Type_Annotations::
typescript_only_if_legal_as_conditional_true_branch:
should_parse_annotation =
this->options_.typescript && child->kind() == Expression_Kind::Paren;
break;
case Allow_Type_Annotations::typescript_only:
should_parse_annotation = this->options_.typescript;
break;
Expand All @@ -2120,10 +2127,11 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v,
if (!should_parse_annotation) {
break;
}
Expression* child = binary_builder.last_expression();
Source_Code_Span colon_span = this->peek().span();
Buffering_Visitor type_visitor(&this->type_expression_memory_);

Parser_Transaction transaction = this->begin_transaction();

// If an arrow function has a return type, then the return type must *not*
// be parenthesized. Exception: an possibly-parenthesized arrow return type
// is okay.
Expand All @@ -2143,6 +2151,45 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v,
.allow_assertion_signature_or_type_predicate =
is_possibly_arrow_function_return_type_annotation,
});
if (prec.colon_type_annotation ==
Allow_Type_Annotations::
typescript_only_if_legal_as_conditional_true_branch &&
this->options_.typescript &&
this->peek().type == Token_Type::equal_greater) {
// cond ? (t) : param => body
// cond ? (param): RetType => body : f
// ^^

// TODO(strager): We should call validate_arrow_function_parameter_list,
// similar to if we did 'goto arrow_function;'.

this->skip();
Stacked_Buffering_Visitor arrow_body_visits =
this->buffering_visitor_stack_.push();
Expression* arrow_function =
this->parse_arrow_function_expression_remainder(
arrow_body_visits.visitor(), /*generic_parameter_visits=*/nullptr,
child,
/*return_type_visits=*/&type_visitor, /*prec=*/prec);

if (this->peek().type == Token_Type::colon) {
// (cond ? (param): RetType => body : f)
// ^
// We correctly interpreted ': RetType' as a return type annotation.
this->commit_transaction(std::move(transaction));
arrow_body_visits.visitor().move_into(v);
binary_builder.replace_last(arrow_function);
break;
} else {
// (cond ? (x) : param => body)
// ^
// We incorrectly interpreted ': param' as a return type annotation.
this->roll_back_transaction(std::move(transaction));
break;
}
}
this->commit_transaction(std::move(transaction));

const Char8* type_end = this->lexer_.end_of_previous_token();
binary_builder.replace_last(
this->make_expression<Expression::Type_Annotated>(
Expand Down
10 changes: 10 additions & 0 deletions src/quick-lint-js/fe/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,16 @@ class Parser {
// > 'unsigned char:2' in initialization
enum Allow_Type_Annotations : std::uint8_t {
typescript_only,
// Used inside 't' in 'c ? t : f':
//
// c ? (param): Type => body : f; // 'Type' is a type annotation.
// c ? x : y => body : f; // 'y' is not a type annotation because
// // 'x' is not parenthesized. Invalid.
// c ? (x) : param => body; // 'param' is not a type annotation
// // because no ':' follows 'body'.
// Only parse annotations if they are valid return type annotations inside
// the '?:'.
typescript_only_if_legal_as_conditional_true_branch,
always,
never,
};
Expand Down
75 changes: 75 additions & 0 deletions test/test-parse-expression-typescript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,81 @@ TEST_F(Test_Parse_Expression_TypeScript,
Expression* ast = p.parse_expression();
EXPECT_EQ(summarize(ast), "cond(var cond, var x, var Type)");
}

{
Test_Parser p(u8"cond ? t : param => body"_sv, typescript_options);
Expression* ast = p.parse_expression();
EXPECT_EQ(summarize(ast), "cond(var cond, var t, arrowfunc(var param))");
}

{
Test_Parser p(u8"cond1 ? cond2 ? t2 : param => body : f1"_sv,
typescript_options);
Expression* ast = p.parse_expression();
EXPECT_EQ(summarize(ast),
"cond(var cond1, cond(var cond2, var t2, arrowfunc(var param)), "
"var f1)");
}
}

TEST_F(Test_Parse_Expression_TypeScript,
colon_in_conditional_can_be_arrow_return_type_annotation) {
{
Test_Parser p(u8"cond ? (param): ReturnType => body : f"_sv,
typescript_options);
Expression* ast = p.parse_expression();
EXPECT_EQ(summarize(ast), "cond(var cond, arrowfunc(var param), var f)");
}

{
Test_Parser p(u8"cond ? async (param): ReturnType => body : f"_sv,
typescript_options);
Expression* ast = p.parse_expression();
EXPECT_EQ(summarize(ast),
"cond(var cond, asyncarrowfunc(var param), var f)");
}
}

TEST_F(
Test_Parse_Expression_TypeScript,
colon_in_conditional_can_be_arrow_return_type_annotation_despite_following_syntax_error) {
{
// TypeScript resolves this ambiguity as a syntax error, so we should too.
// TypeScript's rule seems to be that '(t2)' is not a parameter list if
// 'body' is followed by a ':'.
test_parse_and_visit_expression(
u8"cond1 ? cond2 ? (t2) : param => body : f1"_sv, //
u8" ` Diag_Missing_Colon_In_Conditional_Expression.expected_colon\n"_diag
u8" ^ .question"_diag,
typescript_options);
}
}

TEST_F(
Test_Parse_Expression_TypeScript,
colon_in_conditional_true_branch_cannot_be_arrow_return_type_annotation_if_arrow_body_not_followed_by_colon) {
{
Test_Parser p(u8"cond ? (t) : param => body"_sv, typescript_options);
Expression* ast = p.parse_expression();
EXPECT_EQ(summarize(ast),
"cond(var cond, paren(var t), arrowfunc(var param))");
}

{
// This example triggers backtracking in the parser. Ensure that the
// backtracking walks back any speculative visits.
Spy_Visitor p = test_parse_and_visit_expression(
u8"cond ? (t) : param => body"_sv, no_diags, typescript_options);
EXPECT_THAT(p.visits, ElementsAreArray({
"visit_enter_function_scope", //
"visit_variable_declaration", // param
"visit_enter_function_scope_body", //
"visit_variable_use", // body
"visit_exit_function_scope", //
"visit_variable_use", // cond,
"visit_variable_use", // t
}));
}
}

TEST_F(Test_Parse_Expression_TypeScript, non_null_assertion) {
Expand Down

0 comments on commit 7a17bc9

Please sign in to comment.