-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Adds check for abstract methods to generator grammar #25711
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
Adds check for abstract methods to generator grammar #25711
Conversation
Before, the compiler would fail on abstract generator methods, since it would confirm the abstract method had no body; but when checking the generator grammar, it would see the lack of a `node.body` and fail it as an overloaded generator. Adding a check for the Abstract flag means the compiler accurately recognizes the abstract generator not as an overload, but as an abstract method.
The first fix for allowing abstract generators also meant that abstract overloaded generators were allowed. This prevents any overloading of generators, abstract or not, while still allowing the declaration of abstract generator methods.
src/compiler/checker.ts
Outdated
if (!node.body) { | ||
return grammarErrorOnNode(node.asteriskToken!, Diagnostics.An_overload_signature_cannot_be_declared_as_a_generator); | ||
if (!node.body && !hasModifier(node, ModifierFlags.Abstract)) { | ||
return grammarErrorOnNode(node.asteriskToken!, Diagnostics.An_overload_signature_cannot_be_declared_as_a_generator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix the indentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielRosenwasser done!
I do not think this is a change we want to make. see my comment in #25710 (comment) |
Fixes #25710