Skip to content

Compiler does not allow abstract generators #25710

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

Closed
aleph-naught2tog opened this issue Jul 17, 2018 · 14 comments
Closed

Compiler does not allow abstract generators #25710

aleph-naught2tog opened this issue Jul 17, 2018 · 14 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@aleph-naught2tog
Copy link

aleph-naught2tog commented Jul 17, 2018

TypeScript Version: First noticed on 2.9.1, confirmed in 3.0.0-dev.20180712

Search Terms:
generator abstract, "An overload signature cannot be declared as a generator.", generator overload, overload generator abstract, TS1222

Code

abstract class SomethingAbstract {
    // TS1222: An overload signature cannot be declared as a generator.
    abstract *sadGenerator(): any; // specifying a return type doesn't help
    *fineGenerator(): any {}
}

Expected behavior:
The compiler should allow abstract generator methods.

Actual behavior:
The compiler does not allow abstract generator methods: instead, it errors with: [ts] An overload signature cannot be declared as a generator.

I believe I know where and why this is and I'll open up a PR once I fix it.

Playground Link: Small sample

Related Issues:
Couldn't find any related ones

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Help Wanted You can do this Good First Issue Well scoped, documented and has the green light labels Jul 17, 2018
@DanielRosenwasser DanielRosenwasser added this to the Community milestone Jul 17, 2018
@ajafff
Copy link
Contributor

ajafff commented Jul 17, 2018

Why is this necessary? The asterisk here is just syntactic sugar for the return type:

abstract class SomethingAbstract {
    abstract sadGenerator(): IterableIterator<any>;
}

If the extending class implements the method as generator or just returns an IterableIterator is an implementation detail.

@aleph-naught2tog
Copy link
Author

That's true. However, I think the inconsistency inandofitself in the behavior is a problem: if we allow abstract generators, we should allow them to be written with generator syntax, sugar or not.

@yortus
Copy link
Contributor

yortus commented Jul 17, 2018

TypeScript does allow abstract async functions even though such declarations are also syntax sugar for a promise-returning abstract function (analogous to @ajafff's comment above). So it might be more consistent to allow abstract generators too.

@ghost
Copy link

ghost commented Jul 17, 2018

We don't allow async in interfaces, type literals, or function types -- allowing async in an abstract class was probably an oversight?

abstract class C {
    async abstract m(): Promise<number>; // OK?
}

interface I {
    async m(): Promise<number>; // Error
}
type T = { async m(): Promise<number>; } // Error
type Cb = async () => Promise<number>; // Error

@yortus
Copy link
Contributor

yortus commented Jul 18, 2018

@Andy-MS right, it looks like abstract async is the odd thing out here. If abstract async is made into an error, then tsc will be consistent across both async and generator functions, for abstract classes, interfaces, type literals and function types. All of them would reject async and * and require an appropriate return type annotation instead.

@aleph-naught2tog
Copy link
Author

Out of curiosity, what's the rationale for not allowing async then on interfaces, etc? Is it just because it's redundant and sugar for the return type?

If the consensus is to make abstract async into an error for consistency (and continue to not allow * syntax for abstract generators), I'd like to try tackling that issue instead then 😄

@mhegazy
Copy link
Contributor

mhegazy commented Jul 26, 2018

I do not think the original issue is a bug. generators, just like async functions, are just an implementation detail. the contract does not care how the function is implemented but rather what parameters it takes, and what return type it offers. A generator function implements the Iterator protocol, but it can also be implemented using different means, and there is no reason derived classes has to adhere to this implementation specification.

I do agree with @Andy-MS' comment that async being allowed on an abstract method is a bug.

@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript Good First Issue Well scoped, documented and has the green light Help Wanted You can do this labels Jul 26, 2018
@mhegazy mhegazy removed this from the Community milestone Jul 26, 2018
@aleph-naught2tog
Copy link
Author

@mhegazy Sounds good! Should I open an issue about async being allowed on abstract methods as a bug -- I would like to try fixing that.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 27, 2018

sure.

@dyst5422
Copy link

dyst5422 commented Oct 5, 2018

How should this be implemented in a concrete class?

Currently,

abstract class A {
  public abstract m(): IterableIterator<string>;
}

class C implements A {
  public *m() {
    yield 'foo';
  }
}

Has the error, [ts] Non-abstract class 'C' does not implement inherited abstract member 'm' from class 'A'.

@ghost
Copy link

ghost commented Oct 8, 2018

@dyst5422 I don't see any such error when trying to compile that example code in either [email protected] or typescript@next.

@dyst5422
Copy link

dyst5422 commented Oct 8, 2018

@Andy-MS

I think this was a file reference issue. Unrelated

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@idofilus
Copy link

Is it possible to revive this issue by a suggestion that simply allow this ?
We of course can without any issues abstract async, but what about async* ?

abstract class A
{
    abstract async* getTransactions(data: Object): AsyncIterator<TransactionEntity[]>;
}

class B extends A
{
    async* getTransactions(data: Object): AsyncIterator<TransactionEntity[]>
    {

    }
}

Error:
TS1222: An overload signature cannot be declared as a generator.

I'm currently using a workaround with:

abstract class A
{
    async* getTransactions(data: Object): AsyncIterator<TransactionEntity[]>
    {
        throw new Error("Not implemented");
    }
}

josephaxisa added a commit to looker-open-source/sdk-codegen that referenced this issue Dec 21, 2020
jkaster added a commit to looker-open-source/sdk-codegen that referenced this issue Dec 21, 2020
* Bumped TypeScript version to 4.1.3

* Removed async modifier from abstract method

This was a breaking change as of TS 4.x: microsoft/TypeScript#39963

More context:
microsoft/TypeScript#25710 (comment)

* Removed ts-node from non-root package.json files

* Added test:sdk script to root package.json

* Fixed specConverter paths

* Fixed logical error in declarationMiner.spec.ts

* yarn.lock

* Fix root path resolution for sdk-rtl testUtils.ts

Co-authored-by: John Kaster <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

8 participants