Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest fix for mispelled await inside async function #488

Merged
merged 3 commits into from
Nov 5, 2021
Merged

Suggest fix for mispelled await inside async function #488

merged 3 commits into from
Nov 5, 2021

Conversation

keyehzy
Copy link
Contributor

@keyehzy keyehzy commented Nov 4, 2021

Fix for issue #279.

@strager
Copy link
Collaborator

strager commented Nov 4, 2021

It looks like there are test failures. (See the CI job failures.) Do you need help fixing the tests?

@keyehzy
Copy link
Contributor Author

keyehzy commented Nov 5, 2021

I was some commits behind and error code 176 was already taken, oops.

Copy link
Collaborator

@strager strager left a comment

Choose a reason for hiding this comment

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

Looks good! Let me know when I should merge.

Because we diagnose the await and suggest async instead, we should parse the arrow function's body as if async was written instead. For example, this should report only one error, not two:

let f = await () => { // should error on 'await'
  await g(); // shouldn't error because of 'await'; we're supposed to be in an async function
};

But we can leave that for a separate PR.

src/parse.cpp Outdated
if (child->kind() == expression_kind::_invalid) {
this->error_reporter_->report(error_missing_operand_for_operator{
.where = operator_span,
});
} else if (this->in_async_function_ && this->is_arrow_kind(child) &&
!is_followed_by_async) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Instead of checking for the async token, you can ask if child is async:

Suggested change
!is_followed_by_async) {
child->attributes() == function_attributes::async) {

@@ -88,6 +88,13 @@
ERROR(QLJS_TRANSLATABLE("'await' is only allowed in async functions"), \
await_operator)) \
\
QLJS_ERROR_TYPE( \
error_await_followed_by_arrow_function, "E177", \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: Now E177 is taken. xD Try E178. (I can fix this when I merge if you want.)

Comment on lines +94 to +95
ERROR(QLJS_TRANSLATABLE("'await' cannot be followed by an arrow " \
"function; use 'async' instead"), \
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@keyehzy
Copy link
Contributor Author

keyehzy commented Nov 5, 2021

You can merge if you wish. I can make a followup PR, one step at a time.

@strager strager merged commit d32117c into quick-lint:master Nov 5, 2021
@keyehzy keyehzy deleted the fix#279part2 branch November 5, 2021 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants