What value does it add? Is it that it enables them to use checkerframework with our interfaces?
On Thu, Apr 1, 2021 at 8:54 AM Kenneth Knowles <k...@apache.org> wrote: > Thanks for filing that. Once it is fixed in IntelliJ, the annotations > actually add value for downstream users. > > Kenn > > On Thu, Apr 1, 2021 at 1:10 AM Jan Lukavský <je...@seznam.cz> wrote: > >> Hi, >> >> I created the issue in JetBrains tracker [1]. I'm still not 100% >> convinced that it is correct for the checker to actually modify the >> bytecode. An open questions is - in guava this does not happen. Does guava >> apply the check on code being released? From what is in this thread is >> seems to me, that the answer is no. >> >> Jan >> >> [1] https://youtrack.jetbrains.com/issue/IDEA-265658 >> On 4/1/21 6:15 AM, Kenneth Knowles wrote: >> >> Hi all, >> >> About the IntelliJ automatic method stub issue: I raised it to the >> checkerframework list and got a helpful response: >> https://groups.google.com/g/checker-framework-discuss/c/KHQdjF4lesk/m/dJ4u1BBNBgAJ >> >> It eventually reached back to Jetbrains and they would appreciate a >> detailed report with steps to reproduce, preferably a sample project. Would >> you - Jan or Ismaël or Reuven - provide them with this issue report? It >> sounds like Jan you have an example ready to go. >> >> Kenn >> >> On Mon, Mar 15, 2021 at 1:29 PM Jan Lukavský <je...@seznam.cz> wrote: >> >>> Yes, annotations that we add to the code base on purpose (like @Nullable >>> or @SuppressWarnings) are aboslutely fine. What is worse is that the >>> checked is not only checked, but a code generator. :) >>> >>> For example when one wants to implement Coder by extending CustomCoder >>> and use auto-generating the overridden methods, they look like >>> >>> @Overridepublic void encode(Long value, @UnknownKeyFor @NonNull >>> @Initialized OutputStream outStream) throws >>> @UnknownKeyFor@NonNull@Initialized CoderException, >>> @UnknownKeyFor@NonNull@Initialized IOException { >>> >>> } >>> >>> Which is really ugly. :-( >>> >>> Jan >>> >>> On 3/15/21 6:37 PM, Ismaël Mejía wrote: >>> >>> +1 >>> >>> Even if I like the strictness for Null checking, I also think that >>> this is adding too much extra time for builds (that I noticed locally >>> when enabled) and also I agree with Jan that the annotations are >>> really an undesired side effect. For reference when you try to auto >>> complete some method signatures on IntelliJ on downstream projects >>> with C-A-v it generates some extra Checkers annotations like @NonNull >>> and others even if the user isn't using them which is not desirable. >>> >>> >>> >>> On Mon, Mar 15, 2021 at 6:04 PM Kyle Weaver <kcwea...@google.com> >>> <kcwea...@google.com> wrote: >>> >>> Big +1 for moving this to separate CI job. I really don't like what >>> annotations are currently added to the code we ship. Tools like Idea add >>> these annotations to code they generate when overriding classes and that's >>> very annoying. Users should not be exposed to internal tools like >>> nullability checking. >>> >>> I was only planning on moving this to a separate CI job. The job would >>> still be release blocking, so the same annotations would still be required. >>> >>> I'm not sure which annotations you are concerned about. There are two >>> annotations involved with nullness checking, @SuppressWarnings and >>> @Nullable. @SuppressWarnings has retention policy SOURCE, so it shouldn't >>> be exposed to users at all. @Nullable is not just for internal tooling, it >>> also provides useful information about our APIs to users. The user should >>> not have to guess whether a method argument etc. can be null or not, and >>> for better or worse, these annotations are the standard way of expressing >>> that in Java. >>> >>>
