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

Type hole with compatibility between optional parameters/extra parameters #3049

Open
jesseschalken opened this issue Dec 20, 2016 · 4 comments
Labels
bug Typing: soundness No false negatives (type checker claims that there is no error in the incorrect program)

Comments

@jesseschalken
Copy link
Contributor

jesseschalken commented Dec 20, 2016

//@flow

const x = (a: number = 1): number => a;
const y: () => number = x;
// No Flow error
// Runtime error: Uncaught TypeError: y(...).toFixed is not a function
y('x').toFixed();

const z: (a: string) => number = y;
// No Flow error
// Runtime error: Uncaught TypeError: z(...).toFixed is not a function
z('x').toFixed();

I expected Flow to either report that (number|void) => number is incompatible with () => number, or that () => number is incompatible with (string) => number. Either would fix this problem, but allowing both in combination is definitely unsound.

@jesseschalken
Copy link
Contributor Author

It basically comes down to: What type is a function assumed to accept for extra parameters, mixed or void? i.e., is (number) => string considered (number, void, void, ...) => string or (number, mixed, mixed, ...) => string? It can only be one or the other. It can't be both.

If extra parameters are considered mixed, then (number|void) => number is incompatible with () => number, because mixed is incompatible with number|void.

If extra parameters are considered void, then () => number is incompatible with (string) => number because string is incompatible with void.

@jesseschalken
Copy link
Contributor Author

jesseschalken commented Dec 20, 2016

Here's a way to get the best of both worlds while maintaining soundness: Have extra parameters of a function type be void, and the extra parameters of a function definition be mixed. This means:

  • A function implementing a function type can omit parameters, since any extra parameters to the function are considered mixed (since the function will ignore whatever is passed), which everything is compatible with.
  • A function implementing a function type, or function type implementing another function type, can add extra optional parameters, since any extra parameters in the base function type are considered void, which is compatible with an optional parameter (void|...).

Here's an example:

//@flow

interface Foo {
  fooMethod(x: string): void;
}

let foo: Foo = {
  // Implementing a function by omitting a parameter is ok, since the function will ignore the
  // parameter so it can be considered as accepting `mixed` for extra parameters.
  fooMethod(): void {}
};

interface Blah extends Foo {
  // Implementing a function with another function that needs an extra optional parameter is
  // ok, since extra parameters in Foo.fooMethod are considered `void`, which is compatible with
  // string|void.
  fooMethod(x: string, y: string|void): void;
}

// If you call a function literal with extra parameters (or function type inferred from a function
// literal), it's okay, because the extra parameters are `mixed`.
const x = () => 'hello';
x('foo');

// If you call a function type with extra parameters, the parameters must be compatible with
// `void`, since the function may be implemented by one which has hidden optional parameters.
const y = (a: number = 1): number => a;
const z: () => number = y;
z('foo'); // Error: string is incompatible with void

// For the same reason, you can't assign a function type to one accepting more parameters.
const w: (a: string) => number = z; // Error: string is incompatible with void

Put differently, function types without rest parameters would have an implied rest parameter of ...rest: Array<void> for the purpose of typing, and function definitions without rest parameters would have an implied rest parameter of ...rest: Array<mixed> for the purpose of typing.

@vkurchatkin vkurchatkin added the Typing: soundness No false negatives (type checker claims that there is no error in the incorrect program) label Feb 2, 2017
@masaeedu
Copy link

@jesseschalken Is your proposal a breaking change for code like this?

type F = (a: number) => string
type G = (a: number, b: string) => string

const f: F = a => a.toString()
const g: G = f

@jesseschalken
Copy link
Contributor Author

@masaeedu Yes, it would be a breaking change for that code. A function compatible with F may accept additional optional parameters, so F must not be considered compatible with G, since G has a second parameter which may not be compatible with an optional parameter of the underlying function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Typing: soundness No false negatives (type checker claims that there is no error in the incorrect program)
Projects
None yet
Development

No branches or pull requests

3 participants