[jira] [Created] (CALCITE-4199) Add nullability annotations to the methods and fields, ensure consistency with checkerframework

["Vladimir Sitnikov (Jira)" <ji...@apache.org>]

Vladimir Sitnikov created CALCITE-4199:
------------------------------------------

             Summary: Add nullability annotations to the methods and fields, ensure consistency with checkerframework
                 Key: CALCITE-4199
                 URL: https://issues.apache.org/jira/browse/CALCITE-4199
             Project: Calcite
          Issue Type: New Feature
    Affects Versions: 1.25.0
            Reporter: Vladimir Sitnikov


The current codebase uses jsr305 javax.annotation.Nullable / NonNull which helps to catch bugs while developing Calcite and libraries.

Unfortunately, the current annotation is not enforced, and it lacks support for generics.
jsr305.jar is probably abandoned (see https://github.com/google/guava/issues/2960), so we should probably migrate to something else.

https://checkerframework.org/ is a solid framework for machine verification which is tailored to Java.

The releases are quite frequent: https://github.com/typetools/checker-framework/releases
Their annotations are recognized by major IDEs.

So I see the following options:
a) Leave the code as is
b) Annotate (gradually?) the code with checkerframework annotations
c) Migrate (gradually?) the code to Kotlin

I've created a PR to verify which changes would be needed, verify if CI is able to type check in a reasonable time, and so on: https://github.com/apache/calcite/pull/1929

My findings regarding checkerframework so far are:
0) It does detect NPEs (which were hidden in the current code), and it does detect initialized files in the constructors
1) Checkerframework takes ~2 minutes for linq4j, and 13+min (unknown yet, 13m produces 100 errors and stops) for core
2) "non-nullable by default" is quite close to the current Calcite conventions.
3) There are cases when javadoc comments says "or null", however, the code reads much easier if {{@Nullable}} nullable appears in the signature
4) If a third-party library supplies invalid type annotations, there's a way to fix that. For instance, Guava's Function is annotated as "always nullable", and we can overrule that (so the nullability is taken from generic signature rather than "always nullable"). The override files are placed to src/main/config/checkerframework
5) Generic-heavy code might be challenging (they are always like that), however, in the most obscure cases there's always a way to suppress the warning
6) I've filed a Gradle improvement so it schedules recently modified files first for the compilation: https://github.com/gradle/gradle/issues/14332



--
This message was sent by Atlassian Jira
(v8.3.4#803005)