Class inheritance about reasonablytyped HOT 13 OPEN

I'd like to start researching this. This will certainly be the deepest I've ever gone into the module/object system!

from reasonablytyped.

@rrdelaney I have some plans I think I can implement this week, but I thought I'd get your feelings on approach. You may already have seen the Discord convo a couple days ago, but it boiled down to implementing inheritance via denormalization. So, for each type from which a type inherits, you essentially reprint the non-overridden parts of the inherited types, for each inheriting type. I haven't fully tested it, but rough examples printed at the bottom.

I can think of two reasons not to do this:

• Potentially very long duplicated type bodies
• Declarations outside the body of the inherited type itself that accept/return/store it seem like they'd require our compile step to be aware of every subtype of that type. Js like declare function genericArg<T: A>(a: T): string would need a Reason equivalent for each extends A that exists, which based on my limited compiler knowledge seems like it would require awareness of a much larger context than most of the conversion logic. Does this strike you as problematic?

An alternate option would be "all classes are open Js.t objects, so inheritance is just a type argument". I think this is technically not true in Flow, and it could conceivably mean that a BS-generated object could have extra members that somehow trigger unwanted behavior; maybe the underlying JS checks for some key foo and if it finds it, does something unexpected. However, it seems like it would cleave closer to idiomatic Reason, seem more "natural", and...probably mostly work. Thoughts?

Example of the first approach, in case my description had any ambiguities:

JS:

  declare export class A {
    constructor(s: string): A,
    fn1(a: number): string
  }

  declare export class B extends A {
    constructor(s: string): B,
    fn2(b: number): string
  }

RE:

module A = {
  type t = Js.t {.
    fn1 : (float => string) [@bs.meth]
  };
  external make : string => t = "A" [@@bs.new];
};

module B = {
  type t = Js.t {.
    fn1 : (float => string) [@bs.meth],
    fn2 : (float => string) [@bs.meth]
  };
  external make : string => t = "B" [@@bs.new];
};

from reasonablytyped.

@bbqbaron I saw the discussion on Discord - super through 👍

I don't know if this implementation would work though because OCaml uses nominal typing, but matching against structure. For example the following code causes a compile error in BuckleScript:

type class1 = Js.t {. x: float };
type class2 = Js.t {. x: float, y: string };

external print_class : class1 => unit = "console.log" [@@bs.val];
let class1_instance: class1 = [%bs.raw "{ x: 1 }"];
let class2_instance: class2 = [%bs.raw "{ x: 1, y: 'hello' }"];

print_class class1_instance;
print_class class2_instance;

Although class2_instance has all properties needed to satisfy the type class1 it cannot be passed as an instance of it. I image we would need to generate code similar to that for the following JS:

declare module 'test' {
  declare class Base { x: number };
  declare class Sub extends Base { y: string };

  declare function print(b: Base): string
}

Although in Flow we could pass an instance of Sub into print, the above OCaml doesn't allow it.

Is there anyway around that? Otherwise I don't have any problem generating a super-long definition per type.

from reasonablytyped.

I think it's ok if we delay this feature a bit, we should start throwing a compile error when a class extends another though.

from reasonablytyped.

Sure, no problem; I can add the failures quite soon.

I was also mulling over the larger issue a little. To answer your question, I think the brute-force solution is simply to duplicate declarations for all known possible cases. So, in your example, print_class would need to be re-declared as accepting every subclass of Base that exists:

external print_class1 : class1 => unit = "console.log" [@@bs.val];
external print_class2 : class2 => unit = "console.log" [@@bs.val];

(ignoring for a moment whether we care about mangling the name print_class or not)

Further, if one set of decls (let's call it A) refers to another B and A subclasses anything in B, you'd also need to reprint any declarations from B that are now contained in or need to refer to any of the new subclasses from A, wouldn't you? So if B has a class C and a function doStuff<T: C>(t: T), and A has class D: C you'd need to declare doStuff(t: D), and so on and so on.

That doesn't necessarily seem intellectually difficult; it just smells complex/wide in terms of adequate testing, recursion, making sure you don't miss cases, etc. Does my logic hold up?

If so, it makes me kind of favor an earlier option: Just make classes open types. They technically aren't, but anything in flow that takes an A also takes its subclasses, and as you say, we aren't vulnerable to collisions of structural typing, which I think removes a lot of vulnerability. There are some edge cases and fine points, to be sure, but doesn't it seem more sort of...natural to just say, look, a class is Js.t {..}? Mind if I POC that, or was that option already discussed and discarded somewhere?

from reasonablytyped.

Class inheritance rejected in feaf58c

from reasonablytyped.

I think there are more problems that arise from copying members and methods that creating a permutation can't solve. For example, when we support importing files, the following would need duplication across files:

// A.js
declare module 'A' {
  declare export class A {
    print(a: A): void
  }
}

// B.js
import type { A } from 'A'

declare module 'B' {
  declare export class B extends A {
    other(b: B): void
  }

  declare export var b: B
}

I think it would cause too many problems for us in the future, and it might be a good idea to put it on hold. You're right in that doing all of this is theoretically easy, but it's a serious smell.

Thanks so much for adding the extends check, my code broke the tests a while ago though. I don't think it was that change.

from reasonablytyped.

Just to make sure I understand, is the classes-as-open-objects a no-go? It seems like it would enable the important parts of inheritance behavior.

Also, it occurs to me that we should implement interface inheritance no matter what! I've been focused on classes this whole time, but interfaces are clearly open, right? We could at least knock those out.

from reasonablytyped.

Can you paste an example of what the code generation would look like for open objects?

from reasonablytyped.

Sure; how does this strike you? It's based on how I think about classes, which is, conveniently, "basically open objects"; I may be missing some of the fine points of deep OO. This seems to work in try-reason, or at least doesn't have compile errors!

module A = {
  type t 'a = Js.t {..} as 'a;
  external make : unit => t 'a = "A" [@@bs.new];
  external aFunc : t 'a => string = "";
};

module B = {
  type t 'a = A.t (Js.t {..}) as 'a;
  external make : unit => t 'a = "B" [@@bs.new];
  external bFunc : t 'a => float = "";
};

module C = {
  type t 'a = B.t (Js.t {..}) as 'a;
  external make : unit => t 'a = "C" [@@bs.new];
};

let b: B.t (Js.t {.}) = B.make ();
let bString = A.aFunc b;
let c: C.t (Js.t {.}) = C.make ();
let cString = A.aFunc c;
let cFloat = B.bFunc c;

from reasonablytyped.

The snippet didn't compile for me in try 😅 I think if I'm reading this right though, all classes become open object types, and we pass them into methods defined on the module, rather than access with class##property.

If my interpretation is correct, the solution above works, however I think we would be breaking type contracts for a lot of libraries. In the above example, you can pass c to any function in any other class module, which means we couldn't constrain the types to a subtype.

Is there any way we could constrain those objects to only be passed to certain methods?

from reasonablytyped.

Sorry, could you elaborate/expand on this part?

In the above example, you can pass c to any function in any other class module, which means we couldn't constrain the types to a subtype.

Maybe an example of where this could misbehave? I think I'm just not parsing the terminology correctly.

from reasonablytyped.

Is this still the best workaround in absence of this feature?

module Animal {
  type t;
  val name: () => string;
}

module Mammal {
  type t;
}

external asAnimal : Mammal.t => Animal.t = "%identity";

from reasonablytyped.