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

ADTs with inheritance #20144

Closed
afroozeh opened this issue Nov 19, 2017 · 10 comments
Closed

ADTs with inheritance #20144

afroozeh opened this issue Nov 19, 2017 · 10 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@afroozeh
Copy link

afroozeh commented Nov 19, 2017

I'd like to define an ADT and also enforce that all concrete types implement another interface (I know that is not how ADTs are supposed to be used, but since TypeScript is a mixed paradigm language, I'd like to be able to do it the way I'm used to it in Scala). This is an attempt to achieve it:

type Shape = Circle | Rectangle

interface Drawable {
    draw();
}

class Circle implements Drawable {
    constructor(readonly kind: "circle", readonly radius: number) { }
    draw() {}
}

class Rectangle implements Drawable {
    constructor(readonly kind: "square", readonly weight: number, readonly height: number) { }
    draw() {} 
}

let s: Shape;
s.draw();

The problem is that I cannot enforce that all concrete classes implement Drawable when defining them:

type Shape = Circle | Square | Triangle

// This is ok, since there is no way to enforce Triangle to implement Drawable
class Triangle {
    constructor(readonly kind: "square", readonly side1: number, readonly side2: number, side3: number) { }
}

let s: Shape;
s.draw(); // Error: property draw does not exist on Shape

and I only will get an error when actually call draw() on a Shape instance.

In Scala, for example, I can enforce that all concrete case classes to implement the Drawable interface:

trait Drawable {
  def draw(): Unit
}

trait Shape extends Drawable

case class Circle(radius: Int) extends Shape {
  override def draw() {}
}

case class Square(width: Int, height: Int) extends Shape {
  override def draw() {}
}

// Error: Triangle does not implement draw
case class Triangle(side1: Int, side2: Int, side3: Int) extends Shape {
}

Is there a way to achieve this in TypeScript?
Is there a plan to allow discriminated union type matching feature with interfaces?
Is there a plan to allow type aliases to extend interfaces?

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Nov 20, 2017

implements is an expression of intent and provides tooling assistance when implementing a class, but it does not affect type checking.

This is because, unlike Scala which is nominally typed with the exception of the experimental scala.language.reflectiveCalls, TypeScript is structurally typed.

As you have noted, there is a static error at the call site meaning that type safety is uncompromised.

Is there a plan to allow discriminated union type matching feature with interfaces?

See the approach outlined in #9163, the pull request introducing the feature.

Is there a plan to allow type aliases to extend interfaces?

This can already be done

interface Tagged<K extends string> {
  kind: K;
}

interface Drawable {
  draw(): void;
}

type TaggedDrawableWithId<K extends string> = Tagged<K> & Drawable & {
  id: number
};

And interfaces can also extends type aliases

interface Pentagon extends TaggedDrawableWithId<'pentagon'> {
  points: [number, number, number, number, number];
}

Note: questions like these are better asked on Stack Overflow or Gitter.

@afroozeh
Copy link
Author

@aluanhaddad Thanks for the explanation. I'm a bit confused though:

  • Is there a way to get my Shape hierarchy working with intersection types example you gave in the end so that the compiler complains at definition site (at the Triangle definition) when there is no draw() method implemented?

  • Since Drawable has a draw method, TypeScript should be able to structurally enforce typing, in a sense to require all the unions of a type implement Drawable, as in Scala, or am I missing something here?

@ghost
Copy link

ghost commented Nov 20, 2017

Not a perfect solution but the following will give you an error "near" the declaration:

type T = A | B;
interface I { draw(): void; }
function __test_T_is_I(t: T): I { return t; }

class A { draw() {} }
class B {}

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Nov 20, 2017

If you want to remove the function so it doesn't exist at runtime, you might alternately write

type T = A | B;
interface I { draw(): void; }
type __test_T_is_I<U extends I = T> = {};

class A { draw() {} }
class B {}

Or actually, you could inline the constraint like

type Shape<S extends I = A | B> = S;
interface I {
  draw(): void;
}

but whichever variant you go with, I think @andy-ms's suggestion is the right approach.

@afroozeh
Copy link
Author

@andy-ms Very nice solution, thanks!

I think it is also possible to implement it into the language, something like this (with the < operator indicating upper bound for a type alias).

type T < I = A | B;
interface I { draw(): void; }

Is it something the TypeScript team consider adding to the language?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 20, 2017

I do not think these two concepts fit together really..

can you elaborate on the scenario here? and why structural checking is not sufficient at use sites?

@mhegazy mhegazy added the Needs More Info The issue still hasn't been fully clarified label Nov 20, 2017
@afroozeh
Copy link
Author

afroozeh commented Nov 20, 2017

@mhegazy You're probably right about the topics not being a fit, but let me explain with another example.

Consider I want to have an Expression hierarchy:

Expression = BinaryExpression | UnaryExpression | Literal

class BinaryExpression {
	constructor(public left: Expression, public right: Expression) {}
}

class UnaryExpression {
	constructor(public expr: Expression) {}
}

class Literal {
	constructor(public val: number) {}
}

Now consider that I also want my Expression hierarchy also have a children property and I don't want to implement it as a separate function, e.g., no function of the type Expression => Iterable<Expression>.

I defined the following interface that defines the children() property:

interface AST {
	children(): Iterable<AST>;
}

My question is that is there a way to restrict the Expression union type to also extend AST? So that, the compiler complains that BinaryExpression, UnaryExpression, Literal should have the children property?

In other words, is there a way to enforce a that all members of a union type have a specific property? In Scala, using case classes I can have such a property and was wondering if it is possible to also get it in TypeScript.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 20, 2017

I understand, but why is not use site sufficient in this case. i.e. passing Expression to something that expects AST would flag if not all constituents have a method children on them.

@afroozeh
Copy link
Author

afroozeh commented Nov 20, 2017

A call site error is not a real problem, and the error message is also very clear indicating which type does not have children on it.

There are two things, which I would say would be nice, if I could enforce a super type on the union type:

  1. I don't need to really expose the AST type, as children is already part of the expression hierarchy. The client code does not even need to know that there is an AST interface.

  2. It helps with discovery of methods. Let's say, in the Expression example, I add another concrete type to the union (say TernaryExpression) and I forget to implement children there. If children is not used elsewhere in the code base, I will not get an error, but at the same time, children disappears from the list of properties. It is a bit problematic when first writing the code, when I need some IDE completion support to see what methods are available.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

4 participants