FR: allow passing optional parameters in StarlarkMethod as java optionals
- Dominant language
- Java
- Stars
- 25.8k
- Forks
- 4.6k
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 72
Description
A common pattern we see in StarlarkMethod is optional parameters whose default value is either `None` (a.k.a. Starlark.NONE) or `unbound` (a.k.a. Starlark.UNBOUND), and when the value is not the default, then it is of one definite desired type (string, label, etc.). On the Java side, currently such parameters must be declared as Object, and the implementation function (which is typically in a different Java file than the annotation), for each such parameter, does something along the lines of
```java
DesiredType desiredTypeValue;
if (value == Starlark.NONE /* or UNBOUND or ... */ ) {
desiredTypeValue = sensibleDefault();
} else {
desiredTypeValue = uglyAndUnsafeCastOperation(value);
}
````
which is frankly terrible, even when wrapped in a helper function (of which there are many independent variations scattered around our codebase).
What we ought to do instead is allow net.starlark.java.annot.Param to be marked as `passAsOptional` (tentative name). The java implementation's parameter in this case would be an Optional\. If the value is Param's `default` (`None` or `unbound`), `BuiltingFunction.getArgumentVector()` would convert it to `Optional.empty()`; otherwise, to `Optional.of(value)`.
I would suggest:
* restricting passAsOptional only to params whose default is `None` or `unbound` - other default values poorly map to the concept of a "missing" value that Optional.empty() signifies;
* enforcing via StarlarkMethodProcessor that the java parameter is an Optional and has the correct generic type (in other words, if our Param's allowedTypes is String.class, NoneType.class, we should required the java parameter to be Optional\).
Contributor guide
Assessment
This issue has not been assessed yet.