Flow: Enforce use of function return

[jacobp100]

Swift and Rust both have the behaviour that if you define a function that returns a value, you must use the value.

function return5(): number { return 5 }
return5() // Error
const five = return5() // No error

In both of these languages, you can disregard the return value by assigning to a special variable, _. In JS, we could use the void keyword.

void return5() // No error

Would this be a good addition?

And if so, what would be the best approach? Would something like this work,

  and funcalltype = {
    call_this_t: t;
    call_args_tlist: call_arg list;
    call_tout: t;
    call_closure_t: int;
    call_strict_arity: bool;
+   call_return_used: bool;
  }

And set call_return_used to true whenever the CallExpression is a direct child of ExpressionStatement. Then finally match CallT(_ { call_return_used: true }) to add errors?

[borela]

You can set the expected returned type and flow will warn you about these mistakes.

[jacobp100]

@borela, I think you misunderstood my post. Let me know if you need anything clearing up!

[borela]

Yeah, my bad. There are some cases where I would want to force the user of the API to not ignore the returned value and receive at least a warning; I think it would need another keyword/operator to require that the return value to be used.

[jacobp100]

I've not had issues with Swift forcing me to use a return value, even if it is just throwing it away. Would this not be a good default?

[borela]

It's javascript's default behavior, many libraries rely on it and it would suddenly show errors where none existed before.

[jacobp100]

Can you point to any code that makes extensive deliberate use of this?

it would suddenly show errors where none existed before

Isn't that the point of flow? 😂

[borela]

Can you point to any code that makes extensive deliberate use of this?

All functions have a implicit "return undefined" and many programmers take advantage of that to save 1 line of code, this is more common than I would like.

Isn't that the point of flow? :joy:

It would show errors on valid code.

[jacobp100]

many programmers take advantage of that to save 1 line of code

I see what you mean! I should have been clearer: for all non-void functions, you should use the return value. Flow already knows that in your example, the function still returns void.

If anything, it would highlight errors if someone changed iAmVoid to something that returns a value, then calls to iReturnVoid would return a value that might need to be used in places.

[jesseschalken]

It would definitely be a handy lint.

There are often requests (both on TypeScript and Flow) for a check to make sure you don't forget to await a function that returns a Promise<void>. This would catch that as well. Since Promise<void> is not ignorable, but void is, you must either await it or explicitly throw it away.

[borela]

@jacobp100 I didn't know flow was able to catch that. Warning of the return values could be a good default then.

[noppa]

Seems nice, but I hope this can be opted out of. I fear this is going to introduce a lot of errors in cases like

componentWillMount() {
    loadData()
        .then(
            data => dispatch({ type: 'LOAD_SUCCESS', data }),
            err => dispatch({ type: 'LOAD_FAILURE', err })
        );
}

[jacobp100]

The heuristic described wouldn't give an error there—you're using the return value!

[noppa]

Ok, I just assumed that since Promise#then returns a promise, that would also need to be used.

Even if that's not the case, the example could just as well be

const loadDataAndDispatch = () =>
    loadData().then(
            data => dispatch({ type: 'LOAD_SUCCESS', data }),
            err => dispatch({ type: 'LOAD_FAILURE', err })
        );

...

componentWillMount() {
    loadDataAndDispatch();
}

Wouldn't that still cause an error?

[jacobp100]

That would. My suggestion would be to write void loadDataAndDispatch() to be clear that you don't want to handle the return value.

[RyanCavanaugh]

Many JS functions return values which you usually don't care about. It'd be bothersome to have to write code like this:

const arr = [1, 2, 3];
void arr.push(4); // I don't care about the new length
void arr.sort(); // I don't need another reference to the array
void arr.reverse(); // I don't need another reference to the array
void arr.splice(2, 1); // I don't need the removed element

You could have some syntax for declaring "This is an important return value" but then you get to have religious wars over whether or not Array#pop truly returns an important value or not (I just wanted to shorten the array! No, you wanted the last element! No, I just wanted a shorter array! Gaze upon the last element, I demand it!)

[leebyron]

Forgetting to return or await a Promise is a really common mistake that I hope can be solved by a change like this, however I agree with the comments that there are too many existing javascript functions, especially in the standard lib, that return values you usually don't care about.

I'd love to see behavior like this as an opt-in option, but as the default specifically for Promise values

[vkurchatkin]

Related: https://github.com/facebook/flow/issues/2281

[thecotne]

there is eslint plugin for this Disallow Unused Expressions (no-unused-expressions)

[noppa]

@thecotne From the documentation for that rule:

This rule does not apply to function calls or constructor calls with the new operator, because they could have side effects on the state of the program.

[thecotne]

@noppa i did not payed much attention Hah

but this issue implies that in order to use function with side effects you need to put void in front of function call

it can be implemented in eslint with option like disallowFunctionCalls disallowConstructorCalls

i think this is easier to implement in eslint then flow