From ba394888af245faf5815a424fb3be925efb00d58 Mon Sep 17 00:00:00 2001 From: keyehzy Date: Mon, 17 May 2021 14:49:43 -0300 Subject: [PATCH 1/6] Suggest `async` when `await` is followed by arrow function. Attempts to fix issue #279. Excluded the cases where `await` is followed by an `async` then followed by an arrow function. In this case `await` must be an unary operator. This commit does not solve issue #278 just yet, but should be a start. Here the error is only caught for top-level await and async function, i.e cases where async is *not* treated as an identifier. --- src/parse.cpp | 16 +++++++++++++++- src/quick-lint-js/error.h | 7 +++++++ test/test-parse-expression.cpp | 10 ++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/parse.cpp b/src/parse.cpp index 1e0809b740..075d1f6d12 100644 --- a/src/parse.cpp +++ b/src/parse.cpp @@ -727,11 +727,25 @@ expression* parser::parse_await_expression(token await_token, precedence prec) { } expression* child = this->parse_expression(prec); - if (child->kind() == expression_kind::_invalid) { + + switch (child->kind()) { + case expression_kind::arrow_function_with_statements: + case expression_kind::arrow_function_with_expression: + if (child->attributes() != function_attributes::async) { + this->error_reporter_->report(error_await_creating_arrow_function{ + .await_operator = operator_span, + }); + } + break; + case expression_kind::_invalid: this->error_reporter_->report(error_missing_operand_for_operator{ .where = operator_span, }); + break; + default: + break; } + return this->make_expression(child, operator_span); } } diff --git a/src/quick-lint-js/error.h b/src/quick-lint-js/error.h index 918e1c7e93..676ab9fc10 100644 --- a/src/quick-lint-js/error.h +++ b/src/quick-lint-js/error.h @@ -66,6 +66,13 @@ .error(QLJS_TRANSLATABLE("'await' is only allowed in async functions"), \ await_operator)) \ \ + QLJS_ERROR_TYPE( \ + error_await_creating_arrow_function, "E???", \ + { source_code_span await_operator; }, \ + .error(QLJS_TRANSLATABLE("'await' does not create an async arrow " \ + "function; use 'async' instead [E???]"), \ + await_operator)) \ + \ QLJS_ERROR_TYPE( \ error_big_int_literal_contains_decimal_point, "E005", \ { source_code_span where; }, \ diff --git a/test/test-parse-expression.cpp b/test/test-parse-expression.cpp index be380b3729..c5017aec41 100644 --- a/test/test-parse-expression.cpp +++ b/test/test-parse-expression.cpp @@ -869,6 +869,16 @@ TEST_F(test_parse_expression, await_unary_operator_inside_async_functions) { EXPECT_EQ(summarize(ast), "await(var x)"); EXPECT_THAT(p.errors(), IsEmpty()); } + + { + test_parser p(u8"await () => {}"_sv); + auto guard = p.parser().enter_function(function_attributes::async); + expression* ast = p.parse_expression(); + EXPECT_THAT(p.errors(), + ElementsAre(ERROR_TYPE_FIELD( + error_await_creating_arrow_function, await_operator, + offsets_matcher(p.code(), 0, u8"await")))); + } } TEST_F(test_parse_expression, From 0dd962dd50b58947ade90486b2f96aff550cb7b2 Mon Sep 17 00:00:00 2001 From: keyehzy Date: Mon, 17 May 2021 16:25:08 -0300 Subject: [PATCH 2/6] Fix github CI error Will squash it latter. --- test/test-parse-expression.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test-parse-expression.cpp b/test/test-parse-expression.cpp index c5017aec41..1183ac0668 100644 --- a/test/test-parse-expression.cpp +++ b/test/test-parse-expression.cpp @@ -874,6 +874,9 @@ TEST_F(test_parse_expression, await_unary_operator_inside_async_functions) { test_parser p(u8"await () => {}"_sv); auto guard = p.parser().enter_function(function_attributes::async); expression* ast = p.parse_expression(); + EXPECT_EQ(ast->kind(), expression_kind::await); + EXPECT_EQ(ast->child_0()->kind(), + expression_kind::arrow_function_with_statements); EXPECT_THAT(p.errors(), ElementsAre(ERROR_TYPE_FIELD( error_await_creating_arrow_function, await_operator, From 5e508a234bc1be44f6ee8bef7beccf3fa07bb938 Mon Sep 17 00:00:00 2001 From: keyehzy Date: Mon, 17 May 2021 18:17:20 -0300 Subject: [PATCH 3/6] fix double error code --- src/quick-lint-js/error.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/quick-lint-js/error.h b/src/quick-lint-js/error.h index 676ab9fc10..51adb64788 100644 --- a/src/quick-lint-js/error.h +++ b/src/quick-lint-js/error.h @@ -70,7 +70,7 @@ error_await_creating_arrow_function, "E???", \ { source_code_span await_operator; }, \ .error(QLJS_TRANSLATABLE("'await' does not create an async arrow " \ - "function; use 'async' instead [E???]"), \ + "function; use 'async' instead"), \ await_operator)) \ \ QLJS_ERROR_TYPE( \ From a2148b2c1b4a722cc74cb2b5903dd243b65fef7d Mon Sep 17 00:00:00 2001 From: keyehzy Date: Mon, 17 May 2021 20:03:04 -0300 Subject: [PATCH 4/6] Attempts to fix #278. It's done by checking if there is a arrow function after `await` even if its treated as an identifier. There is some code duplication between this commit and the earlier one so I will try to work out a way to combine these two. Also, I don't know enough JS to know if the tests are strict enough to catch all the cases. --- src/parse.cpp | 28 +++++++++++++++++++++++++++- test/test-parse-expression.cpp | 15 +++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/parse.cpp b/src/parse.cpp index 075d1f6d12..4a7e867789 100644 --- a/src/parse.cpp +++ b/src/parse.cpp @@ -683,12 +683,38 @@ expression* parser::parse_await_expression(token await_token, precedence prec) { [[fallthrough]]; case token_type::complete_template: case token_type::incomplete_template: - case token_type::left_paren: case token_type::left_square: case token_type::minus: case token_type::plus: return !this->in_top_level_; + case token_type::left_paren: { + source_code_span operator_span = await_token.span(); + buffering_error_reporter temp_error_reporter; + error_reporter* old_error_reporter = + std::exchange(this->error_reporter_, &temp_error_reporter); + lexer_transaction transaction = this->lexer_.begin_transaction(); + + // Try to parse as an arrow function + expression* ast = this->parse_expression(prec); + + this->lexer_.roll_back_transaction(std::move(transaction)); + this->error_reporter_ = old_error_reporter; + + switch (ast->kind()) { + case expression_kind::arrow_function_with_statements: + case expression_kind::arrow_function_with_expression: + this->error_reporter_->report(error_await_creating_arrow_function{ + .await_operator = operator_span, + }); + break; + default: + break; + } + + return !this->in_top_level_; + } + case token_type::bang: case token_type::dot_dot_dot: case token_type::identifier: diff --git a/test/test-parse-expression.cpp b/test/test-parse-expression.cpp index 1183ac0668..c56e16e64f 100644 --- a/test/test-parse-expression.cpp +++ b/test/test-parse-expression.cpp @@ -882,6 +882,21 @@ TEST_F(test_parse_expression, await_unary_operator_inside_async_functions) { error_await_creating_arrow_function, await_operator, offsets_matcher(p.code(), 0, u8"await")))); } + + { + test_parser p(u8"await () => {}"_sv); + auto guard = p.parser().enter_function(function_attributes::normal); + expression* ast = p.parse_expression(); + EXPECT_THAT( + p.errors(), + ElementsAre( + ERROR_TYPE_FIELD(error_await_creating_arrow_function, + await_operator, + offsets_matcher(p.code(), 0, u8"await")), + ERROR_TYPE_FIELD( + error_missing_operator_between_expression_and_arrow_function, + where, offsets_matcher(p.code(), 0, u8"await (")))); + } } TEST_F(test_parse_expression, From 3481de152a13405688e8e131c48e2255bd02d49d Mon Sep 17 00:00:00 2001 From: keyehzy Date: Mon, 17 May 2021 21:44:54 -0300 Subject: [PATCH 5/6] Combining I figured that you don't need to distinguish between the cases where the `await` function is inside an `async` function or not. In either case, if we see an arrow function we can emit the same error suggesting the change to `async`. --- src/parse.cpp | 66 ++++++++++++++-------------------- test/test-parse-expression.cpp | 1 + 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/parse.cpp b/src/parse.cpp index 4a7e867789..153df34165 100644 --- a/src/parse.cpp +++ b/src/parse.cpp @@ -683,38 +683,12 @@ expression* parser::parse_await_expression(token await_token, precedence prec) { [[fallthrough]]; case token_type::complete_template: case token_type::incomplete_template: + case token_type::left_paren: case token_type::left_square: case token_type::minus: case token_type::plus: return !this->in_top_level_; - case token_type::left_paren: { - source_code_span operator_span = await_token.span(); - buffering_error_reporter temp_error_reporter; - error_reporter* old_error_reporter = - std::exchange(this->error_reporter_, &temp_error_reporter); - lexer_transaction transaction = this->lexer_.begin_transaction(); - - // Try to parse as an arrow function - expression* ast = this->parse_expression(prec); - - this->lexer_.roll_back_transaction(std::move(transaction)); - this->error_reporter_ = old_error_reporter; - - switch (ast->kind()) { - case expression_kind::arrow_function_with_statements: - case expression_kind::arrow_function_with_expression: - this->error_reporter_->report(error_await_creating_arrow_function{ - .await_operator = operator_span, - }); - break; - default: - break; - } - - return !this->in_top_level_; - } - case token_type::bang: case token_type::dot_dot_dot: case token_type::identifier: @@ -741,6 +715,30 @@ expression* parser::parse_await_expression(token await_token, precedence prec) { } }(); + if (this->peek().type == token_type::left_paren) { + source_code_span operator_span = await_token.span(); + buffering_error_reporter temp_error_reporter; + error_reporter* old_error_reporter = + std::exchange(this->error_reporter_, &temp_error_reporter); + lexer_transaction transaction = this->lexer_.begin_transaction(); + + // Try to parse as an arrow function + expression* ast = this->parse_expression(prec); + + this->lexer_.roll_back_transaction(std::move(transaction)); + this->error_reporter_ = old_error_reporter; + + bool is_arrow = + (ast->kind() == expression_kind::arrow_function_with_statements) || + (ast->kind() == expression_kind::arrow_function_with_expression); + + if (is_arrow) { + this->error_reporter_->report(error_await_creating_arrow_function{ + .await_operator = operator_span, + }); + } + } + if (is_identifier) { return this->make_expression( await_token.identifier_name(), await_token.type); @@ -754,22 +752,10 @@ expression* parser::parse_await_expression(token await_token, precedence prec) { expression* child = this->parse_expression(prec); - switch (child->kind()) { - case expression_kind::arrow_function_with_statements: - case expression_kind::arrow_function_with_expression: - if (child->attributes() != function_attributes::async) { - this->error_reporter_->report(error_await_creating_arrow_function{ - .await_operator = operator_span, - }); - } - break; - case expression_kind::_invalid: + if (child->kind() == expression_kind::_invalid) { this->error_reporter_->report(error_missing_operand_for_operator{ .where = operator_span, }); - break; - default: - break; } return this->make_expression(child, operator_span); diff --git a/test/test-parse-expression.cpp b/test/test-parse-expression.cpp index c56e16e64f..a5f3d5ac84 100644 --- a/test/test-parse-expression.cpp +++ b/test/test-parse-expression.cpp @@ -887,6 +887,7 @@ TEST_F(test_parse_expression, await_unary_operator_inside_async_functions) { test_parser p(u8"await () => {}"_sv); auto guard = p.parser().enter_function(function_attributes::normal); expression* ast = p.parse_expression(); + EXPECT_EQ(ast->kind(), expression_kind::binary_operator); EXPECT_THAT( p.errors(), ElementsAre( From 798b911f5addb720d897191924975e0b5e9dce91 Mon Sep 17 00:00:00 2001 From: keyehzy Date: Tue, 18 May 2021 12:29:04 -0300 Subject: [PATCH 6/6] Added more tests and nit things --- src/parse.cpp | 6 +++--- test/test-parse-expression.cpp | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/parse.cpp b/src/parse.cpp index 153df34165..4ef429557c 100644 --- a/src/parse.cpp +++ b/src/parse.cpp @@ -722,17 +722,17 @@ expression* parser::parse_await_expression(token await_token, precedence prec) { std::exchange(this->error_reporter_, &temp_error_reporter); lexer_transaction transaction = this->lexer_.begin_transaction(); - // Try to parse as an arrow function + // Try to parse leading ( as arrow function. expression* ast = this->parse_expression(prec); this->lexer_.roll_back_transaction(std::move(transaction)); this->error_reporter_ = old_error_reporter; - bool is_arrow = + bool is_arrow_kind = (ast->kind() == expression_kind::arrow_function_with_statements) || (ast->kind() == expression_kind::arrow_function_with_expression); - if (is_arrow) { + if (is_arrow_kind) { this->error_reporter_->report(error_await_creating_arrow_function{ .await_operator = operator_span, }); diff --git a/test/test-parse-expression.cpp b/test/test-parse-expression.cpp index a5f3d5ac84..bed6e9448c 100644 --- a/test/test-parse-expression.cpp +++ b/test/test-parse-expression.cpp @@ -883,6 +883,19 @@ TEST_F(test_parse_expression, await_unary_operator_inside_async_functions) { offsets_matcher(p.code(), 0, u8"await")))); } + { + test_parser p(u8"await () => "_sv); + auto guard = p.parser().enter_function(function_attributes::async); + expression* ast = p.parse_expression(); + EXPECT_EQ(ast->kind(), expression_kind::await); + EXPECT_EQ(ast->child_0()->kind(), + expression_kind::arrow_function_with_expression); + EXPECT_THAT(p.errors(), + ElementsAre(ERROR_TYPE_FIELD( + error_await_creating_arrow_function, await_operator, + offsets_matcher(p.code(), 0, u8"await")))); + } + { test_parser p(u8"await () => {}"_sv); auto guard = p.parser().enter_function(function_attributes::normal); @@ -898,6 +911,22 @@ TEST_F(test_parse_expression, await_unary_operator_inside_async_functions) { error_missing_operator_between_expression_and_arrow_function, where, offsets_matcher(p.code(), 0, u8"await (")))); } + + { + test_parser p(u8"await () => "_sv); + auto guard = p.parser().enter_function(function_attributes::normal); + expression* ast = p.parse_expression(); + EXPECT_EQ(ast->kind(), expression_kind::binary_operator); + EXPECT_THAT( + p.errors(), + ElementsAre( + ERROR_TYPE_FIELD(error_await_creating_arrow_function, + await_operator, + offsets_matcher(p.code(), 0, u8"await")), + ERROR_TYPE_FIELD( + error_missing_operator_between_expression_and_arrow_function, + where, offsets_matcher(p.code(), 0, u8"await (")))); + } } TEST_F(test_parse_expression,