bazelbuild / bazelbuild/bazel

FR: allow passing optional parameters in StarlarkMethod as java optionals

Open
#17,520 0 comments 0 reactions 1 assignee Claimed by @tetromino View on GitHub
P2 team-Starlark-Integration type: feature request
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.