tl icon indicating copy to clipboard operation
tl copied to clipboard

Optional / required arguments for functions

Open hishamhm opened this issue 5 years ago • 7 comments

This is an experiment implementing arity checks as discussed in #71.

These are not option types, but they address one of the most common use-cases for which option types are usually wanted: catching missing arguments in function calls.

Syntax is function (r: string, o?: number, p?: boolean) where r is required and o and p are optional. Only the rightmost arguments can be optional. For type definitions, the syntax to specify optional arity in unnamed arguments is local f: function(string, ? string, ? boolean).

Behavior and syntax for return types have not changed in this PR; it only deals with input arguments.

My first impression is overall positive: it's nice to have the compiler tell you that you forgot one argument in a function!

However, there are some quirks. Assigning a function function (string, string) to a callback of type function (string...) is not valid, because the caller of the callback may not pass sufficient arguments — this is correct, strictly speaking, but it causes annoyances in things like the function callback of string.gsub and the __call metamethod... you're forced to make all arguments that match the vararg optional, which feels unnatural given that you can often be sure that the arguments are there based on context that the programmer knows but the compiler doesn't (for example, the captures in gsub). If we move forward with this, I might make an unsound exception for subtype matching of variadic functions, since those are often used in especially dynamically-typed callback APIs.

hishamhm avatar Jan 29 '21 06:01 hishamhm

Teal Playground URL: https://366--teal-playground.netlify.app

github-actions[bot] avatar Jan 29 '21 06:01 github-actions[bot]

Yes, callbacks and optional arguments lead to the variance rabbit hole, see #318 (also, does "optional" mean "you can pass nil" and vice-versa...)

catwell avatar Jan 29 '21 14:01 catwell

Yes, callbacks and optional arguments lead to the variance rabbit hole, see #318 (also, does "optional" mean "you can pass nil" and vice-versa...)

Unless this PR changes this, a T is the same as T | nil, so I see no reason why you wouldn't be able to pass nil explicitly to an optional parameter.

lenscas avatar Jan 29 '21 14:01 lenscas

Of course. But there is no type corresponding to an optional parameter (it is not T | nil) and that causes a lot of issues.

Anyway this issue is one of the reasons why TypeScript went with an unsound type system. There is an even better explanation here. I am not saying Teal needs to do this, they had to because you can't have a type system that is annoying to use with callbacks when extending JavaScript. In Teal / Lua I would say having, for instance, to cast manually with as would be fine.

catwell avatar Jan 29 '21 14:01 catwell

Yes, this PR does not change the other subtyping rules, only the arity rules, so yes, you can pass an explicit nil.

In fact, this PR is implemented in pieces, and one serious possibility is to apply the arity rules for function calls only, and leave the function type comparisons unchanged (keep the check in the @funcall operator, but remove the parts that change same_type and is_a).

It would be a smaller step towards correctness checking of your programs, but it would still be a step nonetheless, and would avoid all of the annoyances with variance mismatching (with the caveat of keeping them as runtime crashes in case of mistakes, just as they currently are).

It might make sense to keep arity checks for calls and returns only, and save the actual subtyping comparisons for eventual true option types. In my experience, the former caught some mistakes in my program and caused no major annoyances. The lack of arity checking in subtyping only annoyed me in assignments, because in forward function declarations you want them to match perfectly, but in callbacks you often want it to be more lenient. At one point I had an even stricter version of subtyping comparison implemented than what's on the PR right now, but then I found myself sprinkling dummy arguments all over to make the typechecker happy and that didn't feel right.

hishamhm avatar Jan 29 '21 14:01 hishamhm

Anyway this issue is one of the reasons why TypeScript went with an unsound type system. There is an even better explanation here.

Yeah, this is precisely the awkward behavior I hit right away. If we merge this PR only until commit 90f31f9, then we get pretty much the TypeScript behavior. I'm thinking this is the safest way to go, and then I can implement the same for return arities (which will affect return statements only).

hishamhm avatar Jan 29 '21 20:01 hishamhm

+1000 After using Teal a lot lately, missing a function parameter and nil reference errors are the biggest runtime errors I encounter

svermeulen avatar Apr 17 '22 08:04 svermeulen

What is the current status of this pull request?

Nukiloco avatar Jun 27 '23 22:06 Nukiloco

@svermeulen @Nukiloco This PR was merged into the next branch, along with a bunch of other features!

For backwards compatibility, I've added --feat-arity=off flag, which allows for the more lax argument checking to still work. But it's not a lot of work to port code to the new style, adding ? to function arguments where needed!

https://github.com/teal-language/tl/tree/next

hishamhm avatar Jan 08 '24 22:01 hishamhm

@hishamhm Sounds awesome, looking forward to those features getting merged!

svermeulen avatar Jan 09 '24 04:01 svermeulen