facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.06k stars 1.85k forks source link

Create a "Comparable" interface #388

Open danvk opened 9 years ago

danvk commented 9 years ago

This code:

/** @flow */

class Foo<T: (number|string)> {
  val: T;

  lessThan(other: Foo<T>) {
    return this.val < other.val;  // XXX flow doesn't like this comparison
  }
}

produces the following error:

baderror.js:3:15,20: number
This type is incompatible with
  baderror.js:7:12,31: string

baderror.js:3:22,27: string
This type is incompatible with
  baderror.js:7:12,31: number

Found 2 errors

In most type systems, the T in other: Foo<T> would be the same T as for the current class. This would result in number↔number and string↔string comparisons, but never number↔string.

Removing the <T> in other: Foo<T> makes the error go away, but now I have no confidence that Flow is actually checking anything.

danvk commented 9 years ago

(I'm using flow 0.8.0)

danvk commented 9 years ago

From IRC:

glevi: danvk: So number is a subtype of (number | string) and string is a subtype of (number | string)
glevi: danvk: However (number | string) is also a subtype of (number | string )

and if T=number|string, then T < T is possibly invalid.

gabelevi commented 9 years ago

More IRC logs (gist in case that link rots)

danvk commented 9 years ago

Creating a Comparable marker type would be an acceptable solution. So would allowing an OR of types, e.g. T=number or T=string, but not T=number|string.