[
https://issues.apache.org/jira/browse/CALCITE-4199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17187590#comment-17187590
]
Chunwei Lei commented on CALCITE-4199:
--------------------------------------
If it is enforced, does it take a lot of effort to change the current code
base? BTW, does your findings 1) mean the checkerframework would increase much
compile time?
> 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
> Priority: Major
>
> 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)
