Skip to content

Commit

Permalink
Error on 'for (async of []);'
Browse files Browse the repository at this point in the history 
For a while, the code 'for (async of []);' is ambiguous in the JS
standard. It's going to be made unambiguously a SyntaxError. Make
quick-lint-js report an error for this code, treating 'async' as a
variable name, and gracefully continue.

Many other forms, such as 'for (async.prop of []);' and
'for (async of => {}; cond; update);' are unambiguous, and shouldn't
result in any errors. This commit adds tests for these similar cases.

Discussion on the ambiguity and its resolution:
tc39/ecma262#2034
  • Loading branch information
@strager
strager committed Feb 20, 2021
1 parent aa5ea63 commit 96ebdb8
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 14 deletions.
8 changes: 8 additions & 0 deletions src/quick-lint-js/error.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@
{ source_code_span where; }, \
.error(QLJS_TRANSLATABLE("BigInt literal contains exponent"), where)) \
\
QLJS_ERROR_TYPE( \
error_cannot_assign_to_variable_named_async_in_for_of_loop, "E082", \
{ identifier async_identifier; }, \
.error( \
QLJS_TRANSLATABLE( \
"assigning to 'async' in a for-of loop requires parentheses"), \
async_identifier)) \
\
QLJS_ERROR_TYPE( \
error_cannot_declare_await_in_async_function, "E069", \
{ identifier name; }, \
Expand Down
100 changes: 86 additions & 14 deletions src/quick-lint-js/parse.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <quick-lint-js/null-visitor.h>
#include <quick-lint-js/padded-string.h>
#include <quick-lint-js/parse-visitor.h>
#include <quick-lint-js/warning.h>

#define QLJS_CASE_BINARY_ONLY_OPERATOR \
case token_type::kw_instanceof: \
Expand Down Expand Up @@ -1421,6 +1422,29 @@ class parser {

bool entered_for_scope = false;

QLJS_WARNING_PUSH
QLJS_WARNING_IGNORE_GCC("-Wshadow-local")
auto parse_in_or_of_or_condition_update =
[&, this](auto &v, expression_ptr init_expression) -> void {
QLJS_WARNING_POP
switch (this->peek().type) {
case token_type::semicolon:
this->skip();
this->visit_expression(init_expression, v, variable_context::rhs);
parse_c_style_head_remainder();
break;
case token_type::kw_in:
case token_type::kw_of: {
this->skip();
expression_ptr rhs = this->parse_expression();
this->visit_assignment_expression(init_expression, rhs, v);
break;
}
default:
QLJS_PARSER_UNIMPLEMENTED();
break;
}
};
switch (this->peek().type) {
// for (;;) {}
case token_type::semicolon:
Expand Down Expand Up @@ -1480,29 +1504,77 @@ class parser {
break;
}

// for (init; condition; update) {}
default: {
expression_ptr init_expression =
this->parse_expression(precedence{.in_operator = false});
// for (async; condition; update) {}
// for (async.prop; condition; update) {}
// for (async in things) {}
// for (async.prop of things) {}
// for (async of => {}; condition; update) {}
// for (async of things) {} // Invalid.
case token_type::kw_async: {
token async_token = this->peek();
expression_ptr async_var_expression =
this->make_expression<expression::variable>(
async_token.identifier_name(), async_token.type);
this->skip();
switch (this->peek().type) {
case token_type::semicolon:
this->skip();
this->visit_expression(init_expression, v, variable_context::rhs);
parse_c_style_head_remainder();
break;
case token_type::kw_in:
// for (async of => {}; condition; update) {}
// for (async of things) {} // Invalid.
case token_type::kw_of: {
token of_token = this->peek();
this->skip();
expression_ptr rhs = this->parse_expression();
this->visit_assignment_expression(init_expression, rhs, v);
if (this->peek().type == token_type::equal_greater) {
// for (async of => {}; condition; update) {}
this->skip();
std::array<expression_ptr, 1> parameters = {
this->make_expression<expression::variable>(
identifier(of_token.span()), of_token.type)};
expression_ptr ast = this->parse_arrow_function_body(
function_attributes::async, async_token.begin,
this->expressions_.make_array(std::move(parameters)));
ast = this->parse_expression_remainder(
ast, precedence{.in_operator = false});
parse_in_or_of_or_condition_update(v, ast);
} else {
// for (async of things) {} // Invalid.
this->error_reporter_->report(
error_cannot_assign_to_variable_named_async_in_for_of_loop{
.async_identifier = async_token.identifier_name(),
});
expression_ptr rhs = this->parse_expression();
this->visit_assignment_expression(async_var_expression, rhs, v);
}
break;
}
default:
QLJS_PARSER_UNIMPLEMENTED();

// for (async in things) {}
// for (async; condition; update) {}
case token_type::kw_in:
case token_type::semicolon:
parse_in_or_of_or_condition_update(v, async_var_expression);
break;

// for (async.prop; condition; update) {}
// for (async.prop of things) {}
case token_type::dot:
default: {
expression_ptr init_expression = this->parse_expression_remainder(
async_var_expression, precedence{.in_operator = false});
parse_in_or_of_or_condition_update(v, init_expression);
break;
}
}
break;
}

// for (init; condition; update) {}
// for (item of things) {}
// for (item in things) {}
default: {
expression_ptr init_expression =
this->parse_expression(precedence{.in_operator = false});
parse_in_or_of_or_condition_update(v, init_expression);
break;
}
}

QLJS_PARSER_UNIMPLEMENTED_IF_NOT_TOKEN(token_type::right_paren);
Expand Down
76 changes: 76 additions & 0 deletions test/test-parse.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3795,6 +3795,7 @@ TEST(test_parse, variables_can_be_named_contextual_keywords) {
ASSERT_EQ(v.variable_uses.size(), 1);
EXPECT_EQ(v.variable_uses[0].name, name);
}

{
spy_visitor v = parse_and_visit_statement(name + u8".method();");
EXPECT_THAT(v.visits,
Expand Down Expand Up @@ -3837,9 +3838,84 @@ TEST(test_parse, variables_can_be_named_contextual_keywords) {
ElementsAre(spy_visitor::visited_variable_use{name}, //
spy_visitor::visited_variable_use{u8"xs"}));
}

if (name != u8"async") {
// NOTE(strager): async isn't allowed here. See
// test_parse.cannot_assign_to_variable_named_async_in_for_of.
spy_visitor v =
parse_and_visit_statement(u8"for (" + name + u8" of xs) ;");
EXPECT_THAT(v.variable_assignments,
ElementsAre(spy_visitor::visited_variable_assignment{name}));
EXPECT_THAT(v.variable_uses,
ElementsAre(spy_visitor::visited_variable_use{u8"xs"}));
}

{
spy_visitor v =
parse_and_visit_statement(u8"for ((" + name + u8") of xs) ;");
EXPECT_THAT(v.variable_assignments,
ElementsAre(spy_visitor::visited_variable_assignment{name}));
EXPECT_THAT(v.variable_uses,
ElementsAre(spy_visitor::visited_variable_use{u8"xs"}));
}

{
spy_visitor v =
parse_and_visit_statement(u8"for (" + name + u8".prop of xs) ;");
EXPECT_THAT(v.variable_assignments, IsEmpty());
EXPECT_THAT(v.variable_uses,
ElementsAre(spy_visitor::visited_variable_use{name},
spy_visitor::visited_variable_use{u8"xs"}));
}

{
spy_visitor v =
parse_and_visit_statement(u8"for (" + name + u8"; cond;) ;");
EXPECT_THAT(v.variable_assignments, IsEmpty());
EXPECT_THAT(v.variable_uses,
ElementsAre(spy_visitor::visited_variable_use{name},
spy_visitor::visited_variable_use{u8"cond"}));
}

{
spy_visitor v =
parse_and_visit_statement(u8"for (" + name + u8".prop; cond;) ;");
EXPECT_THAT(v.variable_assignments, IsEmpty());
EXPECT_THAT(v.variable_uses,
ElementsAre(spy_visitor::visited_variable_use{name},
spy_visitor::visited_variable_use{u8"cond"}));
}
}
}

TEST(test_parse, for_loop_async_arrow_with_of_parameter_is_init_expression) {
spy_visitor v = parse_and_visit_statement(u8"for (async of => x; y; z);"_sv);
EXPECT_THAT(v.visits, ElementsAre("visit_enter_function_scope", //
"visit_variable_declaration", // of
"visit_enter_function_scope_body", //
"visit_variable_use", // x
"visit_exit_function_scope", //
"visit_variable_use", // y
"visit_variable_use")); // z
}

TEST(test_parse,
cannot_assign_to_variable_named_async_without_parentheses_in_for_of) {
padded_string code(u8"for (async of xs) ;"_sv);
spy_visitor v;
parser p(&code, &v);
p.parse_and_visit_statement(v);
EXPECT_THAT(v.variable_assignments,
ElementsAre(spy_visitor::visited_variable_assignment{u8"async"}));
EXPECT_THAT(v.variable_uses,
ElementsAre(spy_visitor::visited_variable_use{u8"xs"}));
EXPECT_THAT(v.errors,
ElementsAre(ERROR_TYPE_FIELD(
error_cannot_assign_to_variable_named_async_in_for_of_loop,
async_identifier,
offsets_matcher(&code, strlen(u8"for ("), u8"async"))));
}

TEST(test_parse, imported_variables_can_be_named_contextual_keywords) {
for (string8 name : {u8"async"}) {
SCOPED_TRACE(out_string8(name));
Expand Down

0 comments on commit 96ebdb8

Please sign in to comment.