[GitHub] [iceberg] XN137 commented on issue #6625: Improve nullability check in Iceberg codebase

["XN137 (via GitHub)" <gi...@apache.org>]

XN137 commented on issue #6625:
URL: https://github.com/apache/iceberg/issues/6625#issuecomment-1403346772

   since the build is already using errorprone i think NullAway would be something to consider:
   https://github.com/uber/NullAway/
   https://www.uber.com/en-US/blog/nullaway/
   
   newer versions of the gradle-baseline plugins support configuring it more conveniently:
   https://github.com/palantir/gradle-baseline/tree/develop/baseline-null-away
   
   the basic idea is that (below a certain package) all reference types are assumed to be non-null unless they are annotated with `@Nullable`
   
   (which means that `@NonNull` is not needed, since it is the default)
   
   so these things become compile time errors:
   - returning `null` (or a nullable type) from a method that is not annotated
   - passing `null` (or a nullable type) into a method whose parameter is not annotated
   - assigning `null` (or a nullable type) to a field that is not annotated
   - leaving an unannotated field uninitialized after all constructor/initializer paths
   - (... others, you get the idea...)
   - most importantly: unconditionally de-referencing a `@Nullable` reference type
   
   inside an `if (x != null)` block, x of course is no longer considered to have a nullable type
   
   once these initial errors are fixed, you will see transitive errors and iterate on fixing them until you reach a stable state, where the use of null is "consistent" according to these rules.
   
   one could start with iceberg api, then do it for core and later all other downstream iceberg modules.
   
   there is a convenient way of getting started with these "1st level" rules by using the following errorprone checks:
   https://errorprone.info/bugpattern/ReturnMissingNullable
   https://errorprone.info/bugpattern/FieldMissingNullable
   https://errorprone.info/bugpattern/ParameterMissingNullable
   https://errorprone.info/bugpattern/EqualsMissingNullable
   
   note: you can run errorprone in a way so all these missing `@Nullable` annotations are added automatically
   
   so even if NullAway would not be used eventually, applying + enforcing those "1st level" errorprone checks, would help documenting (a subset of) the APIs in terms of nullability.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org