ably / ably-java

Java, Android, Clojure and Scala client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
85 stars 39 forks source link

Consider whether to add nullability annotations #639

Open QuintinWillison opened 3 years ago

QuintinWillison commented 3 years ago

In particular for our public APIs, but would also assist internally in some places too.

This section in the Java interoperability section of the Kotlin docs presents a nice summary of the options we could adopt.

I had previously asked sacOO7 to remove Jetbrains annotations when he added them in one of his PRs, but that was more because I didn't want to conflate this with that particular fix.

Working on this issue has three clear components:

  1. Select which annotations to use
  2. Add them to our public APIs
  3. Possibly add them our internal APIs

┆Issue is synchronized with this Jira Research by Unito

kavalerov commented 3 years ago

Just to add my two cents - I am not sure about web and enterprise Java developers, but afaik in Android world de-facto standard is JetBrains annotations. Because of that I suggest we use them as well, unless there is a strong reason not to.

sacOO7 commented 3 years ago

+1 @kavalerov . I can see it being used almost everywhere too. The only issue would be we need to configure it to generate an error via a plugin when it violates null contracts. Currently, IDE can only generate warnings when null constraints are violated.

lawrence-forooghian commented 1 year ago

I've just been creating an interface in our Asset Tracking Android product which is meant to wrap the ably-java SDK. I found myself having to use my knowledge of our SDKs in order to make judgements about nullability. If we had these annotations I wouldn't have needed to do that. All that is to say this sounds like a good idea 👍.