Skip to content

Conversation

@baywet
Copy link
Member

@baywet baywet commented Nov 16, 2020

related and generation result microsoftgraph/msgraph-sdk-java#558

  • updates java generation so it includes nullable and nonnull annotations for kotlin
  • adds additional nonnull nullable annotations for java sdk
  • updates test files

@baywet baywet requested review from MIchaelMainer, ddyett, nikithauc and zengin and removed request for zengin November 16, 2020 15:58
@baywet baywet self-assigned this Nov 16, 2020
@baywet baywet added the java label Nov 16, 2020
@zengin
Copy link
Contributor

zengin commented Nov 17, 2020

How do we decide on where to use Nonnull and Nullable?

@baywet
Copy link
Member Author

baywet commented Nov 17, 2020

Flip of a coin?
No, I only tag return types as Nonnull if the body explicitly cannot be null (return this, return new something...).
I only tag parameters as Nonnull if the method cannot work without that parameter, of if there is an overload without this parameter.
I only tag getters as Nonnull if they are initialized (ctor, initialized field...) no matter what, and if the setter is tagged as Nonnull.
I only tag setters as Nonnull if it makes sense semantically.
Rest is tagged as Nullable.

I do get this is a rule of thumb and it won't be perfect. But it's always going to be better for the Kotlin devs out there using the SDK than the compiler taking guesses and defaulting everything as Nullable. Now they'll get warnings as far as I understand if they try to pass a nullable in a nonnull (whether it's something they pass from to our SDK, or something they get from the SDK and use in their code/another lib). And we can always adjust that in the future if we get some feedback.

@zengin
Copy link
Contributor

zengin commented Nov 18, 2020

Flip of a coin?
No, I only tag return types as Nonnull if the body explicitly cannot be null (return this, return new something...).
I only tag parameters as Nonnull if the method cannot work without that parameter, of if there is an overload without this parameter.
I only tag getters as Nonnull if they are initialized (ctor, initialized field...) no matter what, and if the setter is tagged as Nonnull.
I only tag setters as Nonnull if it makes sense semantically.
Rest is tagged as Nullable.

I do get this is a rule of thumb and it won't be perfect. But it's always going to be better for the Kotlin devs out there using the SDK than the compiler taking guesses and defaulting everything as Nullable. Now they'll get warnings as far as I understand if they try to pass a nullable in a nonnull (whether it's something they pass from to our SDK, or something they get from the SDK and use in their code/another lib). And we can always adjust that in the future if we get some feedback.

Thanks for the explanation.

@baywet baywet merged commit fe78380 into feature/java-v3 Nov 18, 2020
@baywet baywet deleted the feature/java-api-level branch November 18, 2020 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants