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)
