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

Cannot overload interface method with different return types #54354

Closed
enisdenjo opened this issue May 23, 2023 · 17 comments
Closed

Cannot overload interface method with different return types #54354

enisdenjo opened this issue May 23, 2023 · 17 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@enisdenjo
Copy link

enisdenjo commented May 23, 2023

Bug Report

Defining an overloaded method in an interface raises an error during implementation if overloads have different return types.

🔎 Search Terms

overload, method, interface, factory function

🕗 Version & Regression Information

Doesn't work in v3, v4 or v5.

⏯ Playground Link

Playground link with relevant code

💻 Code

interface Shape {
  area(x: number): number;
  area(x: number, y: number): string;
}

🙁 Actual behavior

class ShapeImpl implements Shape {
  // ❌
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}

function createShape(): Shape {
  return {
    // ❌
    area(_x: number, _y?: number): number | string {
      return 0;
    },
  };
}

🙂 Expected behavior

class ShapeImpl implements Shape {
    // ✅
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}

function createShape(): Shape {
  return {
    // ✅
    area(_x: number, _y?: number): number | string {
      return 0;
    },
  };
}

More details

If the return type does not change, overloads work as expected.

interface Shape {
  area(x: number): number;
  area(x: number, y: number): number; // 👈
}

class ShapeImpl implements Shape {
  area(_x: number, _y?: number): number {
    return 0;
  }
}

function createShape(): Shape {
  return {
    area(_x: number, _y?: number): number {
      return 0;
    },
  };
}

Also works if the overloads are defined in the class or on root level functions

class Shape {
  area(x: number): number;
  area(x: number, y: number): string;
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}

function area(x: number): number;
function area(x: number, y: number): string;
function area(_x: number, _y?: number): number | string {
  return 0;
}
@enisdenjo enisdenjo changed the title Cannot overload interface method when using factory functions Cannot overload interface method with different return types May 23, 2023
@jcalz
Copy link
Contributor

jcalz commented May 23, 2023

Essentially #47669 (focuses on arrow functions but it’s the same issue: we currently have no way to say “check this the same loose way that overload implementation are checked”)

@RyanCavanaugh
Copy link
Member

These are just correct errors; the function provided doesn't match the overloads you wrote and a valid call could result in an unsound result.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label May 23, 2023
@fatcerberus
Copy link

I mean to be fair that's true of regular overloads too, and area(x: number, y?: number): number | string is otherwise a valid implementation signature for this overload set, since it works if you copy and paste the overload signatures from the interface into the class definition.

@enisdenjo
Copy link
Author

These are just correct errors; the function provided doesn't match the overloads you wrote and a valid call could result in an unsound result.

Hmm, but how doesn't it match? Also, why do the inline overflows work then?

@RyanCavanaugh
Copy link
Member

Hmm, but how doesn't it match?

function createShape(): Shape {
  return {
    // ❌
    area(_x: number, _y?: number): number | string {
      return 0;
    },
  };
}
createShape().area(3, 3).substring(0); // no error, but crashes

I can't figure out what you're proposing. Are you saying that any overload implementation signature should be an error too?

@enisdenjo
Copy link
Author

Basically this:

interface Shape {
  area(x: number): number;
  area(x: number, y: number): string;
}

function createShape(): Shape {
  return {
    area(_x: number, _y?: number): number | string {
      return 0;
    },
  };
}

const num: number = createShape().area(3);
const str: string = createShape().area(3, 3);

@RyanCavanaugh
Copy link
Member

People would complain if we did that because it would be inconsistent, since this program would have to error:

interface Shape {
  area(x: number): number;
  area(x: number, y: number): string;
}

function createShape(): Shape {
  const x = {
    area(_x: number, _y?: number): number | string {
      return 0;
    },
  };
  return x;
}

const num: number = createShape().area(3);
const str: string = createShape().area(3, 3);

@enisdenjo
Copy link
Author

enisdenjo commented May 23, 2023

Ok, but, whats the difference here?

// ❌ doesn't work
interface Shape {
  area(x: number): number;
  area(x: number, y: number): string;
}
class ShapeImpl implements Shape {
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}
const num: number = (new ShapeImpl()).area(3);
const str: string = (new ShapeImpl()).area(3, 3);

^ playground link

// ✅ works
class ShapeImpl {
  area(x: number): number;
  area(x: number, y: number): string;
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}
const num: number = (new ShapeImpl()).area(3);
const str: string = (new ShapeImpl()).area(3, 3);

^ playground link

@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.

@enisdenjo
Copy link
Author

@RyanCavanaugh I don't want to be a nuisance, I just want to understand - can you please explain #54354 (comment)? Thanks in advance!

@RyanCavanaugh
Copy link
Member

I don't understand the question. You're basically saying that if you an implements clause, then your methods you write should not have the types that you wrote on them?

@fatcerberus
Copy link

fatcerberus commented May 28, 2023

@enisdenjo I think I understand what Ryan is getting at - currently, implements is merely a hint to the compiler to check that the class is a valid implementation of the type, but doesn’t change the type at all. Under your proposal the single area signature would become an overload implementation and thus the presence of implements theoretically changes the type (by hiding that signature from callers).

@RyanCavanaugh The above being said, I don’t think that’s necessarily true - I’m envisioning that if you call the method directly through a ShapeImpl instance, you’d get the declared signature (with or without the implements clause), while if you call it through the interface you get the overloads. So I thus read this is just a request to make the implements check more loose in considering ShapeImpl a valid implementation. Which makes sense to me since overloads only ever get one implementation anyway. It would just mean that calling methods through the class type is more permissive than calling through the interface, which is generally already the status quo for interfaces anyway.

@enisdenjo
Copy link
Author

enisdenjo commented May 31, 2023

You're basically saying that if you an implements clause, then your methods you write should not have the types that you wrote on them?

@RyanCavanaugh no, I am saying they should. This also doesn't work:

// ❌ doesn't work
interface Shape {
  area(x: number): number;
  area(x: number, y: number): string;
  area(x: number, y?: number): number | string;
}
class ShapeImpl implements Shape {
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}
const num: number = (new ShapeImpl()).area(3);
const str: string = (new ShapeImpl()).area(3, 3);

^ playground link

Why are overloads different when defined in interfaces compared to directly on functions/methods?

@RyanCavanaugh
Copy link
Member

implements clauses don't change the shape of your class; this fact is intentional. IOW, for these two snippets

class ShapeImpl implements Shape {
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}

and

class ShapeImpl {
  area(_x: number, _y?: number): number | string {
    return 0;
  }
}

ShapeImpl.area has the same type in both.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 31, 2023

So I thus read this is just a request to make the implements check more loose in considering ShapeImpl a valid implementation.

Uh, but then what? As soon as you write

const x: Shape = new ShapeImpl();

you'll get a type error, which is the exact error you were trying to ensure wouldn't happen by writing implements

@fatcerberus
Copy link

@RyanCavanaugh I’d probably just expect that assignment to succeed.

@RyanCavanaugh
Copy link
Member

Are you just saying that any signature which roughly matches an overload list should be assignable to it? Or that classes with implements lists would allow extra stuff that wouldn't normally be allowed to happen?

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

5 participants