UnpackArgs should accept None for optional args

[nicks]

Expected behavior:

Consider the following Go function implementation:

var optionalArg MyCustomValue
if err := UnpackArgs("my_func", args, kwargs, "optional_arg?", &optionalArg); err != nil {
  return nil, err
}

I would expect this starlark code to work:

my_func(optional_arg=None)

with the reasoning that "None" should be semantically equivalent to omitting the argument.

Actual behavior

You get an error:

for parameter "optional_arg": got NoneType, want MyCustomValueType

Other notes

This is purely an argument about the ergonomics of UnpackArgs, not about the semantics of Starlark itself.

Also! I spotted at least one bug in starlark caused by this API gotcha. Note that in Python,

$ python3
Python 3.7.2 (default, Jul  6 2020, 12:18:55) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> sorted([], key=None)
[]

is fine, but in Starlark,

$ starlark
Welcome to Starlark (go.starlark.net)
>>> sorted([], key=None)
Traceback (most recent call last):
  <stdin>:1:7: in <expr>
Error in sorted: sorted: for parameter "key": got NoneType, want callable

[alandonovan]

with the reasoning that "None" should be semantically equivalent to omitting the argument.

I don't think we can assume this to be desirable across all Starlark (or Python) APIs. For example, getattr(x, "f") behaves differently from getattr(x, "f", None): the former fails if the field x.f does not exist, whereas the latter returns None. For that reason I don't think we should hardwire this policy choice into the core APIs.

One can always define an Unpacker that implements "None means absent" semantics. Perhaps we could provide a utility function to make that easier?

[nicks]

re: getattr - ya, that's a good counter-example. I was trying to come up with a good rule thumb for when None is passed to optional arguments.

Maybe the rule should be: passing None to an optional argument should never be an error, and that UnpackArgs should leave the pointer untouched rather than returning an error. But it's OK if it's semantically meaningful.

A utility function for the "None means absent" semantics seems reasonable. I worry that it makes the API harder to use if you have to know about this function to handle an obscure edge case. Were you thinking something like:

var optionalArg MyCustomValue
if err := UnpackArgs("my_func", args, kwargs, "optional_arg?", IgnoreNone(&optionalArg)); err != nil {
  return nil, err
}

?

[alandonovan]

Another approach would be to use a suffix other than "?" to indicate that a parameter is both optional and "Noneable", though users will just as easily forget to add a suffix as they will forget to call IgnoreNone.

Given that it's too late too change the default behavior of "?", I like the clarity and explicitness of IgnoreNone.

[nicks]

What about using a ?? suffix ? Feels like every language has been adding it these days between C# and Javascript as the null-coalescing operator.

[alandonovan]

That seems reasonable.