Flow: Calling a function with too many arguments should be an error

[danvk]

I find it surprising that this code type checks:

function foo(x: number) { return x; }
foo(1, 2, 3);

$ echo 'function foo(x: number) { return x; } foo(1, 2, 3);' | flow check-contents
No errors!
$ flow version
Flow, a static type checker for JavaScript, version 0.18.1

This would not pass muster in most languages with type systems (e.g. C or Java).

I realize that the motivation for this is two-fold:

1. This is valid JavaScript.
2. It comes up often for callbacks, e.g. the function passed to Array.prototype.map which technically takes three parameters (the value, the index and the Array) but is usually defined with only one or two parameters.

In other words, this should be fine:

([1, 2, 3]).map(v => 1 + v);

I'm not exactly sure how to draw a distinction between these. The foo example feels like it should be an error. The map example clearly shouldn't be.

Two ideas:

1. Perhaps calling a concrete function with too many parameters should be an error, but calling a function passed as a parameter with too many parameters shouldn't be? I'm not sure whether flow can distinguish these cases.
2. Writers of functions which take callbacks (e.g. the declaration of Array.prototype.map) need to explicitly say that functions with an incomplete set of arguments are OK.

For example, the declaration of Array.prototype.map is currently:

map<U>(callbackfn: (value: T, index: number, array: Array<T>) => U, thisArg?: any): Array<U>;

Maybe it should actually be something like:

map<U>(callbackfn: (value: T, index: number, array: Array<T>) => U, thisArg?: any): Array<U>;
map<U>(callbackfn: (value: T, index: number) => U, thisArg?: any): Array<U>;
map<U>(callbackfn: (value: T) => U, thisArg?: any): Array<U>;

And then calling [].map with a function that takes four variables would be an error.

See also #375 and discussion at https://github.com/splodingsocks/FlowTyped/pull/1#issuecomment-149344085

[mroch]

in (1), I think that the former is a flow from FunT ~> CallT (a function is called) and the latter is a flow from FunT ~> FunT (a function is passed to something expecting a function), so I do think flow distinguishes these cases already.

[pasha-mf]

I'd like to register an observation that Flow has a way to specify that some arguments in the function are optional:

declare function f(arg1: number, optionalArg2?: number);

As well as specify that a function can take a variable number of parameters:

declare function g(arg1: string, ...otherArgs: Array<string>);

So intuitively one expects that when neither of these forms is used, you'd obtain a function with a fixed number of arguments -- however that's not the case, and is rather frustrating. Hope you guys can fix it soon!

[danvk]

@samwgoldman points out that you can make a function take a variable number of void parameters to effectively opt-in to checking this:

function foo(x: number, ..._:void[]) { return x; }
foo(1, 2, 3);

foo.js:10
 10: foo(1, 2, 3);
     ^^^^^^^^^^^^ function call
 10: foo(1, 2, 3);
            ^ number. This type is incompatible with
  9: function foo(x: number, ..._:void[]) { return x; }
                                  ^^^^ undefined

This works but it looks a bit odd and produces strange error messages. I think it would make sense to have some form of parameter count checking on by default.

[michaelhogg]

@danvk @pasha-mf: I completely agree – I too find it very surprising that Flow's default behaviour is to allow excess arguments, and that one has to explicitly opt-in using ...rest: Array<void>.

Maybe functions could support a Flow syntax similar to exact object types:

the object type { x: string } ensures that an object contains _at least_ the property x of type string. However, { x: string } may have other properties in addition to x.

Sometimes we want to also make sure that x is the only property of the object. For this purpose there are exact object types, which use {| and |} instead of { and }

Maybe something like function foo(| x: number |):

function foo(| x: number |) { return x; }
foo(1, 2, 3);

foo(1, 2, 3);
       ^ number. Parameter not accepted by
function foo(| x: number |) { return x; }
             ^^^^^^^^^^^^^ Exact arguments

If a function should not accept any arguments:

function foo(||) { return Math.random(); }

[jedmao]

I agree with @michaelhogg that Flow's default behavior is backwards. TypeScript handles this first by making it the default behavior and supporting the reverse behavior with rest parameters:

function foo(bar: string, ...rest: any[]) {
    return bar;
}

Of course, linters might complain that you're not actually doing anything with rest in the above example.

Basically, like TypeScript, the default behavior should handle the more common scenarios.

[STRML]

The core team has decided to make this happen: https://flow.org/blog/2017/05/07/Strict-Function-Call-Arity/

[kumar303]

This is excellent news. I read the blog post and everything sounds good but why did this issue get closed? Which issue can I subscribe to for updates on when it's implemented?

[vkurchatkin]

It's implemented already