Remove unnecessary Nullable annotation

[CodePlayer]

Remove unnecessary @Nullable annotations in Redis's Operation interfaces.

Many Redis APIs return null ONLY in pipeline or transaction mode, and these operations are not necessarily marked as @Nullable.

However, the IDE will perform WRONG Null checks on related codes and mislead developers, even though we hardly use pipeline and transaction modes.

What's more, in pipeline or transaction mode, some methods of Spring Data Redis are currently marked as @Nullable, while other methods are not marked. The style is NOT uniform, which is easily confusing.

Therefore, I think that for those Redis APIs that cannot return null in normal mode, the @Nullable annotation should be removed uniformly.

• You have read the Spring Data contribution guidelines.
• You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
• You submit test cases (unit or integration tests) that back your changes.
• You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

[pivotal-cla]

@CodePlayer Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-cla]

@CodePlayer Thank you for signing the Contributor License Agreement!

[jxblum]

I think 2 things:

1. First, you should always code defensively. To do anything less with eventually bite you.

As 1 example, libraries, frameworks or otherwise, should never return a null array or Collection (Map) from any method (in any language). Unfortunately, they do! Instead, they should be returning an empty array or Collection (Map) in the absence of a value.

Even more unsettling is the fact that null has special meaning in the context of Redis with respect to collections-like data structures, where nulls are frequently returned to represent the absence of the collection in Redis when given a key. In other words, how do you distinguish between an empty List/Set and a non-existing List/Set in Redis by only returning an empty array or Collection in Java? Clearly, there are ways (for example, for better or worse, a special List-type like NullList).

2. Second, if it is possible that a null value could be returned from a method at all, in any use case, then it should be appropriately documented with @Nullable annotation, regardless of the most common use case (i.e. when not using Redis pipelining or transaction modes).

Equally unfortunate is that fact that, you are right, there are many places where Spring Data Redis uses an underlying API that might return null, and that value obtained from the nullable API is used or returned in an unsafe way. By "returned unsafely", I mean the wrapping, exposed API does not declare the @Nullable annotation itself.

If you were to checkout the spring-data-redis codebase and setup a IDE project, you would see a lot of warnings with regard to nullability.

IMO, this is really part of a much larger effort to align things correctly.

However, this requires team attention (and I have brought such concerns to their attention already) in order to scope and prioritize this effort appropriately.

[jxblum]

Closing in favor of Issue #2707.

[CodePlayer]

I think 2 things:

1. First, you should always code defensively. To do anything less with eventually bite you.

As 1 example, libraries, frameworks or otherwise, should never return a null array or Collection (Map) from any method (in any language). Unfortunately, they do! Instead, they should be returning an empty array or Collection (Map) in the absence of a value.

Even more unsettling is the fact that null has special meaning in the context of Redis with respect to collections-like data structures, where nulls are frequently returned to represent the absence of the collection in Redis when given a key. In other words, how do you distinguish between an empty List/Set and a non-existing List/Set in Redis by only returning an empty array or Collection in Java? Clearly, there are ways (for example, for better or worse, a special List-type like NullList).

2. Second, if it is possible that a null value could be returned from a method at all, in any use case, then it should be appropriately documented with @Nullable annotation, regardless of the most common use case (i.e. when not using Redis pipelining or transaction modes).

Equally unfortunate is that fact that, you are right, there are many places where Spring Data Redis uses an underlying API that might return null, and that value obtained from the nullable API is used or returned in an unsafe way. By "returned unsafely", I mean the wrapping, exposed API does not declare the @Nullable annotation itself.

If you were to checkout the spring-data-redis codebase and setup a IDE project, you would see a lot of warnings with regard to nullability.

IMO, this is really part of a much larger effort to align things correctly.

However, this requires team attention (and I have brought such concerns to their attention already) in order to scope and prioritize this effort appropriately.

"always code defensively" -- it's correct.

However, it is impossible for our developers to perform null safe check on each line of code, which is also unrealistic.

Consider that in the regardless of the pipeline or transaction mode, many methods ( such as increment*(), size() ... ) are clearly impossible to return to null.
For unnecessary null check for a large number of similar code, the code will expand significantly.

These codes will NOT return null, but are detected and warned by the IDE, mixing real and false warnings, will interfere with our developers' judgment and efficiency.

However, during the execution of the pipeline or transaction model, the developer did NOT obtain the return value after each row of Redis calls. Even if there is, all the return values are null, and they can be corrected immediately in the first test.
Always return null , do you have any needs to do with them ? Obviously, there is no such necessary.

My suggestion is NOT to remove all @Nullable, but for which method that clearly does NOT return NULL, in the regardless of the pipeline or transaction mode.
​