Disallows async abstract method declarations
#28517
Conversation
Since we do not allow `async` as a modifier on type members such as in an interface, it was decided that it should also not be allowed on abstract methods. Fixes microsoft#28516.
aleph-naught2tog
force-pushed
the
abstract-async
branch
from
e727f7b to
0a1f3b6
Compare
| @@ -0,0 +1,4 @@ | |||
| // @lib: es2015 | |||
There was a problem hiding this comment.
@rbuckton is // @lib: es2015 the right thing or is there a leaner way to run this?
| @@ -28342,6 +28342,9 @@ namespace ts { | |||
| if (flags & ModifierFlags.Private) { | |||
| return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "private", "abstract"); | |||
| } | |||
| if (flags & ModifierFlags.Async) { | |||
| return grammarErrorOnNode(modifier, Diagnostics._0_modifier_cannot_be_used_with_1_modifier, "async", "abstract"); | |||
There was a problem hiding this comment.
I think the error spans that we give are on the wrong keyword in all of these places. For example, the problem isn't that you've declared the thing as abstract, it's that it makes no sense to mark an abstract member as async.
But I guess that's not your problem. It might be suitable for a follow-up PR if it's not too disruptive.
There was a problem hiding this comment.
Do you mean the error should appear on async instead? I agree that's not really in the scope of this PR but that is one I would be happy to tackle as an experiment to evaluate whether it is too disruptive.
|
I'm not sure why you're getting a test failure; all I get on my machine is |
|
Yeah; I don't get the whitespace notice on my machine, but no failed tests or baseline issues here, either, just when I push up to CI 🤔 |
|
@weswigham @DanielRosenwasser suggested I ping you about the CI failure; I'm not sure where to go next in terms of debugging it since it works on my machine, I accept the baseline, etc |
|
@aleph-naught2tog: The CI appears to be failing because abstractAsync.types differs from the reference baseline. Can you ensure the latest version of that file is included? |
|
This PR hasn't seen any activity for quite a while, so I'm going to close it to keep the number of open PRs manageable. Feel free to open a fresh PR or ask for more help here. |
Since we do not allow
asyncas a modifier on type members such as in an interface, it was decided that it should also not be allowed on abstract methods.Submitting a PR for this had been discussed on the original issue (which was then concluded to not be an actual bug, whereas this scenario is). This PR addresses the issue discussed in the comments here.
Edit by @DanielRosenwasser: Fixes #28516