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

8$: Improve error on 'await' on class method #930

Open
strager opened this issue Jan 31, 2023 · 13 comments
Open

8$: Improve error on 'await' on class method #930

strager opened this issue Jan 31, 2023 · 13 comments
Assignees
Labels
for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html good first issue Good for newcomers and C++ beginners

Comments

@strager
Copy link
Collaborator

strager commented Jan 31, 2023

class C {
  constructor() {}
  await myFunction() {
    let myPromise = Promise.resolve(null);
    await myPromise;
  }
}

quick-lint-js currently reports:

hello.js:3:3: error: unexpected token [E0054]
hello.js:5:5: error: 'await' is only allowed in async functions [E0162]

but quick-lint-js should instead treat 'await' as if it was 'async' and report only:

hello.js:3:3: error: use 'async' instead of 'await' on class methods [E????]

Note: The following example is legal because of the line break (await is interpreted as a field declaration with no type or initializer) and should produce no diagnostics:

class C {
  constructor() {}
  await
  myFunction() {
  }
}

Related: https://quick-lint-js.com/errors/E0178/

@strager strager added good first issue Good for newcomers and C++ beginners for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html labels Jan 31, 2023
@Deep642
Copy link

Deep642 commented Apr 8, 2023

Hi @strager , I want to contribute in open source, but I am not able to understand what is asked here to be done exactly? can you help me out ?

@Deep642
Copy link

Deep642 commented Apr 8, 2023

I want to do this assignment, please assign it to me.

@strager strager assigned strager and Deep642 and unassigned strager Apr 8, 2023
@strager
Copy link
Collaborator Author

strager commented Apr 8, 2023

I am not able to understand what is asked here to be done exactly?

Create a new diagnostic in quick-lint-js' parser.

Docs: https://quick-lint-js.com/contribute/create-diagnostic/

I think this is the relevant code in the parser, but I didn't double-check:

// class C {
// asyn method() {} // Invalid.
// }
p->diag_reporter_->report(diag_unexpected_token{
.token = property_name_span,
});

@strager
Copy link
Collaborator Author

strager commented Aug 29, 2023

@Deep642 Do you need help with implementing this feature?

@alyssaperkins
Copy link

alyssaperkins commented Sep 24, 2023

@strager Hi there! This is my first attempt at trying out an open source contribution. Is this a good starter bug, and if so can I claim it? I'm most comfortable in C++, Java. I may need some help along the way.

@strager
Copy link
Collaborator Author

strager commented Sep 26, 2023

Is this a good starter bug

I think yes.

can I claim it

Sure!

I may need some help along the way.

I'm in the middle of a cross-country move so I might be unresponsive for a week or two. I'll try my best to answer your questions.

@strager strager assigned alyssaperkins and unassigned Deep642 Sep 26, 2023
@alyssaperkins
Copy link

@strager Thanks for getting back with me! I'll be working on this tonight. I'll probably have questions soon.

@alyssaperkins
Copy link

alyssaperkins commented Sep 28, 2023

@starger So if I may for clarity, I am writing a new diagnostic that will detect when to throw the error OR I am adjusting the diagnostic mentioned here?

// class C {
// asyn method() {} // Invalid.
// }
p->diag_reporter_->report(diag_unexpected_token{
.token = property_name_span,
});

@strager
Copy link
Collaborator Author

strager commented Sep 29, 2023

I am writing a new diagnostic that will detect when to throw the error OR I am adjusting the diagnostic mentioned here?

Yes, both. You will need to make a new diagnostic type (in diagnostic-types-2.h) and also code to report the diagnostic (probably in parse-class.cpp). I think that the code you linked is the right code to change for reporting your new diagnostic.

Note that you linked an old version of the code. Here's the latest version for your convenience:

// class C {
// asyn method() {} // Invalid.
// }
p->diag_reporter_->report(Diag_Unexpected_Token{
.token = property_name_span,
});

If you look a few lines up, you'll see a special case for const. Maybe we can do something like that but for await.

@alyssaperkins
Copy link

@strager Ok thank you! I think I wrote the diagnostic correctly. I'm still figuring out how to link things, will be working this weekend or sooner to get that done. I was using the online chat to get some help, wasn't sure if there is a public discord for help?

@strager
Copy link
Collaborator Author

strager commented Oct 11, 2023

I was using the online chat to get some help

Yup! I saw your messages, but I wasn't available when you were chatting.

I now have stable internet access, so I am more available.

wasn't sure if there is a public discord for help?

No. The IRC channel you joined is the right place to discuss quick-lint-js live.

@alyssaperkins
Copy link

alyssaperkins commented Oct 13, 2023

Hey @strager ! I am still working on this issue. Got food poisoning last week so my whole schedule took a hit. I will be focusing on testing efforts and will get on the IRC if I hit any roadblockers. Thanks!

@strager
Copy link
Collaborator Author

strager commented Nov 16, 2023

@alyssaperkins Do you need any help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html good first issue Good for newcomers and C++ beginners
Projects
None yet
Development

No branches or pull requests

3 participants