On Thu, Apr 1, 2021 at 9:57 AM Brian Hulette <bhule...@google.com> wrote:
> What value does it add? Is it that it enables them to use checkerframework > with our interfaces? > Actually if they are also using checkerframework the defaults are the same so it is not usually needed (though some defaults can be changed). Making defaults explicit is most useful for interfacing with other tools with different defaults, such as Spotbugs [1], IDEs [2] [3], or JVM languages with null safety bult-in, etc [4] [5]. Kenn [1] https://spotbugs.readthedocs.io/en/stable/annotations.html#edu-umd-cs-findbugs-annotations-nullable [2] https://www.jetbrains.com/help/idea/2021.1/nullable-and-notnull-annotations.html [3] https://wiki.eclipse.org/JDT_Core/Null_Analysis [4] https://kotlinlang.org/docs/null-safety.html [5] https://kotlinlang.org/docs/java-interop.html#null-safety-and-platform-types 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. >>>> >>>>
