facebook / flow

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

Correct function overload not used #753

Closed Macil closed 7 years ago

Macil commented 9 years ago

The following code should give a flow error on the line with foo.getEventStreamTYPO(). If that's corrected, then Flow should give an error because makeNumberStream() returns Stream<string> instead of Stream<number>. Instead, Flow gives no errors in both cases.

/* @flow */
//jshint ignore:start

declare class Stream<T> {
  map<U>(cb: (value: T) => U): Stream<U>;
  flatMap(): Stream;
  flatMap<U>(cb: (value: T) => Stream<U>): Stream<U>;
  filter(cb: (value: T) => any): Stream<T>;
}

declare class Foo {
  getEventStream(): Stream<string>;
}

function makeNumberStream(): Stream<number> {
  return ((global.fooer()): Stream<Foo>)
    .flatMap((foo) =>
      foo.getEventStreamTYPO()
    );
}

The issue is that Flow is picking the flatMap() overload that takes no parameters, and treats the callback as an extra parameter to it. Removing the no-parameter overload causes Flow to work as expected. I don't think it should be legal for Flow to accept extra arguments to a function when that function has an overload that can take an argument in that position.

(The Stream class declaration is based off of Kefir. If flatMap is passed no argument, it defaults to the x=>x identity function, which doesn't lend itself very well to Flow typing, so I tried to make flatMap have a no-parameter overload which returned an untyped Stream.)

vkurchatkin commented 7 years ago

Flow picks first applicable overload, so you should put them in correct order