Optional<T> should be Optional<T extends Object>

[cbracken]

Optional is intended as a substitute for non-nullable values. Ideally, it should have a generic type of T extends Object in order to prevent non-sensical declarations like Optional<Foo?>. Such declarations are unlikely to happen intentionally, but one could imagine the case of an Optional used in another generic class, where T is a nullable type.

Ideally, we should prevent such uses by declaring Optional as Optional<T extends Object>.

This is a potentially significant breaking change, since currently T implicitly extends dynamic and therefore checks like the following don't produce any analysis errors:

if (obj is Optional) {
  return obj.value.someMethod();
}

where someMethod is a method defined on T. All such call sites would need to be fixed up to obj is Optional<AppropriateType>.

A survey of the Google codebase suggests this would be a fairly extensive breaking change. Given that Optional makes little sense once null-safety is enabled in the language by default, the simpler change may simply be to leave as-is and deprecate the class once null-safety lands in the language.

[cbracken]

/cc @yjbanov and @davidmorgan for thoughts.

[davidmorgan]

I think just making Optional deprecated makes the most sense. People using Optional are likely to be the same people interested in migrating to null safety :)

Thanks.

[cbracken]

sgtm. That avoids a breaking change before (eventual) removal.

[cbracken]

Given that Quiver 3.0 will have a minimum SDK constraint of 2.12.0, I'll deprecate it in that branch now.

[cbracken]

Closing this since we’re deprecating Optional; no need to force people through two migrations here.

[yjbanov]

I propose that we reopen this issue. Even though deprecating Optional and nudging users to move to null safety is the eventual goal, T extends Object is a useful stepping stone towards that goal. By making T explicitly non-nullable users will get a hint from the analyzer at the time they are migrating to null safety to make they type that they feed into Optional non-null. From then on, moving from Optional<T> to T? is mostly a mechanical change.

[jakemac53]

Fwiw I have been running into this a fair bit where the migration tool suggests a nullable type for Optional objects, which makes no sense :(.

Not sure if it is too breaking or what, but definitely seems wrong that T is not bounded to non-nullable Object.

[jakemac53]

Another idea here is you could do a less breaking change, where of also still accepted a nullable value. So all that would be disallowed would be explicitly typing something nullable (Optional<Foo?>). That is always guaranteed to not do what people wanted anyways, so its worth having it be flagged imo.

And it would be worth cleaning up any such instances in google3 as well.