Sudden Sonar issue with @Nullable added in 7.14.0

[vijayrcse]

Hi

We recently upgraded our open api maven plugin to 7.14.0.

Post upgrade, schema code generation is showing @nullable annotation on many fields.

This introduction suddenly creates bunch of sonar errors on java:S2259 rule.

is there a config options that we can use to remove @nullable annotation ?

[embee1981]

I am also seeing this issue when going from 7.10.0 to 7.14.0.

I believe it was introduced in 7.11.0, via #20345 , listed in the release notes as

Adds https://github.com/nullable annotation to Spring Boot generator #20345

It is not listed as a breaking change in the release notes.

It's confusing as I have a field marked with nullable: false but I still see it marked with @Nullable (aren't fields implicitly non-nullable?). Marking the field as required removes the @Nullable annotation. Is this the intent of this change?

Like @vijayrcse, our use of FindBugs for static analysis has gone haywire over flagging null checks required in over 30 microservice repos in pending Renovate updates for the open api plugin. This is despite some having the nullable:false set.

7.10.0

7.11.0

7.14.0

@slobodator @wing328 , any suggestions guys?

[wing328]

@slobodator can you please take a look when you've time? thanks

[slobodator]

@embee1981

TL; DR.

a field marked with nullable: false... (aren't fields implicitly non-nullable?)

No!
There is no issue, just don't use nullable, you need required instead.

Long story

It is quite confusing and was discussed multiple times but will hit again and again, unfortunately.

nullable is a bit contrintuitive.
It is for hadling 3 states, something like PATCH

  "foo" : 1
  "bar" : "someValue"

... for setting foo to 1 and bar to someValue

  "foo" : 1

... for setting foo to 1 and skipping bar

And

  "foo" : 1
  "bar" : null

... for setting foo to 1 and bar explicitly to null, overwritting its current value.

That couldn't be achieved with a regular

class RequestDto {
  Long foo;
  String bar;
}

It needs a special type JsonNullable<String> bar = JsonNullable.undefined();

Once again, unless one needs to discriminate explicit null and absense of the field, they don't need nullable.

Much more common scenario is having, for instance, mandatory/required foo and optional/non-required bar. Then it will be generated

class RequestDto {
  @NotNull
  Long foo;

  @Nullable
  String bar;
}

If the field is required, it makes sense to validate it, right? Then, it will be safe to to do something like request.getFoo() > 100. An NPE won't be thrown.

Another thing is non-required bar. To prevent stupid issues like request.getBar().length()... ooops, NPE... I've added that @Nullable annotation.

Note: it is Java, not Kotlin, so just a warning, but better than nothing.

CC: @wing328

[slobodator]

@embee1981

In other words nullable: false

• is by default, thus not needed
• does NOT mean that the field couldn't not be assigned to null. It could be null. This is quite confusing, isn't it?
• just means that the 3-state is NOT needed.

A set of required fields should be used instead.

[slobodator]

@wing328 @embee1981 @vijayrcse Could we close it then?

[vijayrcse]

Sonar validation is now forcing to do null check in places like request.getBar().length() because of this annotation getting generated by default.
It literally 100s of places in 100s of repos.
what is the suggestion to handle this ?
is there a config option that can disable this @nullable annotation without changing the api spec ?

[slobodator]

@vijayrcse

Ok, now I got what you're talking about.

You don't want to amend the spec and mark required parameters as required. That would make them clear both for a client and backend and add validation too, but indeed it is a spec change.

If they are not required and could be null, then @Nullable and Sonar force developers to handle potential NPEs properly but, again, you an't gonna do that.

You prefer to keep the legacy behaviour.
I am puzzled how the NPEs are managed then but it is not my business.

Ok, there are several options.

• downgrade the openapi-generator plugin
• or use the recent version but adjust the template with @Nullable and use a custom one
• suppress Sonar warnings

@wing328 My bad, I thought that adding @Nullable is just a warning and wouldn't break anything. It seems that any improvement (doesn't matter how good or bad it is) should be issued wrapped by a feature-flag?

It remembers me the argument between Optional.of and Optional.ofNullable.

Thank you for adding the flag for that.

Is it the only way?

[wing328]

You don't want to amend the spec and mark required parameters as required.

I prefer updating the spec to mark the parameters as required instead as other tools (testing, documentation, etc) won't recognize the parameters as required either given that these parameters are indeed required instead of optional

[wing328]

another way is to add a openapi normalizer rule to automatically mark a property as required if nullable:false is set manually (assuming swagger parser/core provides a way to tell if nullable is set explicitly to false)

[vijayrcse]

@slobodator for now we downgraded the version openapi-generator plugin.
Adding parameters or properties as required will also require changes to API spec( each schema having so many fields ), for existing projects it is a significant effort, all of sudden. A feature flag will allow everyone to make such a change in incremental approach