You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Matt Sicker <ma...@musigma.org> on 2024/12/02 18:57:53 UTC

Re: Add JSpecify as `compile` dependency

I’m alright with the JSpecify dependency assuming it has accumulated the momentum we expected. Also, I thought annotations didn’t have to exist at runtime anyways as long as you weren’t using reflection on them.

> On Nov 22, 2024, at 03:52, Piotr P. Karwasz <pi...@mailing.copernik.eu> wrote:
> 
> Hi all,
> 
> On 19.10.2024 09:28, Piotr P. Karwasz wrote:
>> PS: Regarding annotations, can you take a look at issue #3110[4].
>> Users are regularly reporting issues for annotations with a `provided`
>> scope. This is due to the `classfile` option to `-Xlint`, which
>> basically only covers missing annotations (of both `CLASS` and
>> `RUNTIME` retention) that occur at **compile time**. I would like to
>> take a per-annotation approach here: some annotations like JSpecify or
>> Error Prone's `@InlineMe` can be really useful to users, so we can:
>> 
>> * add 3819 bytes of JSpecify dependency to the `compile` scope of all
>> artifacts. In the near future nullability annotations will be all over
>> the place and JSpecify (compared to the other 13 kinds of annotations
>> there) have some strong supporters[5].
>> * keep the current _status quo_ for some annotations (e.g. `@InlineMe`
>> and those OSGi versioning annotations that are not mirrored in the
>> Manifest),
>> * write a tool that removes the other annotations from the class files.
>> 
>> [5]https://jspecify.dev/about/
> 
> I decided to start implementing this proposal and I created PR #3228 to add JSpecify as `compile` dependency. The PR still allows users (include OSGi and JPMS users) to manually exclude the dependency, but I doubt anyone will do it.
> 
> Are you OK with this change? IIRC the main argument against having non-optional dependencies in `log4j-api` and `log4j-core` was that we don't control them. The argument still stands, but it is micro dependency that is controlled by a large group of vendors, so its evolution is pretty much under control.
> 
> Piotr
> 
> [1] https://github.com/apache/logging-log4j2/pull/3228


Re: Add JSpecify as `compile` dependency

Posted by Matt Sicker <ma...@musigma.org>.
Yeah, now I’m noticing even more, particularly from library-specific versions of the annotations. I’m glad we decided to use this library directly rather than introduce another set of annotations!

> On Dec 2, 2024, at 13:28, Gary D. Gregory <gg...@apache.org> wrote:
> 
> Just on the CP for one app tooling stack:
> 
> org.jetbrains.annotations.NotNull
> org.jetbrains.annotations.Nullable
> com.sun.istack.NotNull
> com.sun.istack.Nullable
> org.springframework.lang.NonNull
> org.springframework.lang.Nullable
> jakarta.annotation.Nonnull
> jakarta.annotation.Nullable
> javax.annotation.Nonnull
> javax.annotation.Nullable
> org.wildfly.common.annotation.NotNull
> org.wildfly.common.annotation.Nullable
> 
> What a (sad) joke!
> 
> Gary
> 
> On 2024/12/02 19:14:55 Matt Sicker wrote:
>> Yeah, that’s an annoying problem we have in Spinnaker. We have the javax.annotation one, we have Lombok’s annotation, there is the javax.validation one, the jakarta.validation update, and more nullability annotations from various static code analysis tools. Lombok is one of the things that looks for annotations named “NonNull” or “NotNull” (case insensitive).
>> 
>>> On Dec 2, 2024, at 13:01, Gary Gregory <ga...@gmail.com> wrote:
>>> 
>>> The main issue with these annotations is that there are so many
>>> providers. It's so bad that tooling just now look for names like
>>> @NonNull _without considering the package name_, so there is no limit
>>> as to how much you end up with :-(
>>> 
>>> On Mon, Dec 2, 2024 at 1:58 PM Matt Sicker <ma...@musigma.org> wrote:
>>>> 
>>>> I’m alright with the JSpecify dependency assuming it has accumulated the momentum we expected. Also, I thought annotations didn’t have to exist at runtime anyways as long as you weren’t using reflection on them.
>>>> 
>>>>> On Nov 22, 2024, at 03:52, Piotr P. Karwasz <pi...@mailing.copernik.eu> wrote:
>>>>> 
>>>>> Hi all,
>>>>> 
>>>>> On 19.10.2024 09:28, Piotr P. Karwasz wrote:
>>>>>> PS: Regarding annotations, can you take a look at issue #3110[4].
>>>>>> Users are regularly reporting issues for annotations with a `provided`
>>>>>> scope. This is due to the `classfile` option to `-Xlint`, which
>>>>>> basically only covers missing annotations (of both `CLASS` and
>>>>>> `RUNTIME` retention) that occur at **compile time**. I would like to
>>>>>> take a per-annotation approach here: some annotations like JSpecify or
>>>>>> Error Prone's `@InlineMe` can be really useful to users, so we can:
>>>>>> 
>>>>>> * add 3819 bytes of JSpecify dependency to the `compile` scope of all
>>>>>> artifacts. In the near future nullability annotations will be all over
>>>>>> the place and JSpecify (compared to the other 13 kinds of annotations
>>>>>> there) have some strong supporters[5].
>>>>>> * keep the current _status quo_ for some annotations (e.g. `@InlineMe`
>>>>>> and those OSGi versioning annotations that are not mirrored in the
>>>>>> Manifest),
>>>>>> * write a tool that removes the other annotations from the class files.
>>>>>> 
>>>>>> [5]https://jspecify.dev/about/
>>>>> 
>>>>> I decided to start implementing this proposal and I created PR #3228 to add JSpecify as `compile` dependency. The PR still allows users (include OSGi and JPMS users) to manually exclude the dependency, but I doubt anyone will do it.
>>>>> 
>>>>> Are you OK with this change? IIRC the main argument against having non-optional dependencies in `log4j-api` and `log4j-core` was that we don't control them. The argument still stands, but it is micro dependency that is controlled by a large group of vendors, so its evolution is pretty much under control.
>>>>> 
>>>>> Piotr
>>>>> 
>>>>> [1] https://github.com/apache/logging-log4j2/pull/3228
>>>> 
>> 
>> 


Re: Add JSpecify as `compile` dependency

Posted by "Piotr P. Karwasz" <pi...@mailing.copernik.eu>.
Hi Gary,

On 2.12.2024 20:28, Gary D. Gregory wrote:
> Just on the CP for one app tooling stack:
>
> org.jetbrains.annotations.NotNull
> org.jetbrains.annotations.Nullable
> com.sun.istack.NotNull
> com.sun.istack.Nullable
> org.springframework.lang.NonNull
> org.springframework.lang.Nullable
> jakarta.annotation.Nonnull
> jakarta.annotation.Nullable
> javax.annotation.Nonnull
> javax.annotation.Nullable
> org.wildfly.common.annotation.NotNull
> org.wildfly.common.annotation.Nullable
>
> What a (sad) joke!

There are two types of nullability annotations:

* Those introduced by IDEs and tools to perform static analysis, like 
the `javax/jakarta` and `org.jetbrains` annotations. See [1] for a 
longer list.

* Those introduced as internal annotations by some projects, so they 
don't depend on an external dependency. In your list `com.sun.istack`, 
`org.springframework` and `org.wildfly` are of the second kind. As long 
as developers prefer to use internal annotations instead of adding an 
external dependency, the list of these annotations will only grow longer.

This is why I prefer to use an external dependency like JSpecify, 
instead of creating yet-another set of annotations you need to configure 
in your static analysis tool

Piotr

[1] 
https://stackoverflow.com/questions/4963300/which-notnull-java-annotation-should-i-use


Re: Add JSpecify as `compile` dependency

Posted by "Gary D. Gregory" <gg...@apache.org>.
Just on the CP for one app tooling stack:

org.jetbrains.annotations.NotNull
org.jetbrains.annotations.Nullable
com.sun.istack.NotNull
com.sun.istack.Nullable
org.springframework.lang.NonNull
org.springframework.lang.Nullable
jakarta.annotation.Nonnull
jakarta.annotation.Nullable
javax.annotation.Nonnull
javax.annotation.Nullable
org.wildfly.common.annotation.NotNull
org.wildfly.common.annotation.Nullable

What a (sad) joke!

Gary

On 2024/12/02 19:14:55 Matt Sicker wrote:
> Yeah, that’s an annoying problem we have in Spinnaker. We have the javax.annotation one, we have Lombok’s annotation, there is the javax.validation one, the jakarta.validation update, and more nullability annotations from various static code analysis tools. Lombok is one of the things that looks for annotations named “NonNull” or “NotNull” (case insensitive).
> 
> > On Dec 2, 2024, at 13:01, Gary Gregory <ga...@gmail.com> wrote:
> > 
> > The main issue with these annotations is that there are so many
> > providers. It's so bad that tooling just now look for names like
> > @NonNull _without considering the package name_, so there is no limit
> > as to how much you end up with :-(
> > 
> > On Mon, Dec 2, 2024 at 1:58 PM Matt Sicker <ma...@musigma.org> wrote:
> >> 
> >> I’m alright with the JSpecify dependency assuming it has accumulated the momentum we expected. Also, I thought annotations didn’t have to exist at runtime anyways as long as you weren’t using reflection on them.
> >> 
> >>> On Nov 22, 2024, at 03:52, Piotr P. Karwasz <pi...@mailing.copernik.eu> wrote:
> >>> 
> >>> Hi all,
> >>> 
> >>> On 19.10.2024 09:28, Piotr P. Karwasz wrote:
> >>>> PS: Regarding annotations, can you take a look at issue #3110[4].
> >>>> Users are regularly reporting issues for annotations with a `provided`
> >>>> scope. This is due to the `classfile` option to `-Xlint`, which
> >>>> basically only covers missing annotations (of both `CLASS` and
> >>>> `RUNTIME` retention) that occur at **compile time**. I would like to
> >>>> take a per-annotation approach here: some annotations like JSpecify or
> >>>> Error Prone's `@InlineMe` can be really useful to users, so we can:
> >>>> 
> >>>> * add 3819 bytes of JSpecify dependency to the `compile` scope of all
> >>>> artifacts. In the near future nullability annotations will be all over
> >>>> the place and JSpecify (compared to the other 13 kinds of annotations
> >>>> there) have some strong supporters[5].
> >>>> * keep the current _status quo_ for some annotations (e.g. `@InlineMe`
> >>>> and those OSGi versioning annotations that are not mirrored in the
> >>>> Manifest),
> >>>> * write a tool that removes the other annotations from the class files.
> >>>> 
> >>>> [5]https://jspecify.dev/about/
> >>> 
> >>> I decided to start implementing this proposal and I created PR #3228 to add JSpecify as `compile` dependency. The PR still allows users (include OSGi and JPMS users) to manually exclude the dependency, but I doubt anyone will do it.
> >>> 
> >>> Are you OK with this change? IIRC the main argument against having non-optional dependencies in `log4j-api` and `log4j-core` was that we don't control them. The argument still stands, but it is micro dependency that is controlled by a large group of vendors, so its evolution is pretty much under control.
> >>> 
> >>> Piotr
> >>> 
> >>> [1] https://github.com/apache/logging-log4j2/pull/3228
> >> 
> 
> 

Re: Add JSpecify as `compile` dependency

Posted by Matt Sicker <ma...@musigma.org>.
Yeah, that’s an annoying problem we have in Spinnaker. We have the javax.annotation one, we have Lombok’s annotation, there is the javax.validation one, the jakarta.validation update, and more nullability annotations from various static code analysis tools. Lombok is one of the things that looks for annotations named “NonNull” or “NotNull” (case insensitive).

> On Dec 2, 2024, at 13:01, Gary Gregory <ga...@gmail.com> wrote:
> 
> The main issue with these annotations is that there are so many
> providers. It's so bad that tooling just now look for names like
> @NonNull _without considering the package name_, so there is no limit
> as to how much you end up with :-(
> 
> On Mon, Dec 2, 2024 at 1:58 PM Matt Sicker <ma...@musigma.org> wrote:
>> 
>> I’m alright with the JSpecify dependency assuming it has accumulated the momentum we expected. Also, I thought annotations didn’t have to exist at runtime anyways as long as you weren’t using reflection on them.
>> 
>>> On Nov 22, 2024, at 03:52, Piotr P. Karwasz <pi...@mailing.copernik.eu> wrote:
>>> 
>>> Hi all,
>>> 
>>> On 19.10.2024 09:28, Piotr P. Karwasz wrote:
>>>> PS: Regarding annotations, can you take a look at issue #3110[4].
>>>> Users are regularly reporting issues for annotations with a `provided`
>>>> scope. This is due to the `classfile` option to `-Xlint`, which
>>>> basically only covers missing annotations (of both `CLASS` and
>>>> `RUNTIME` retention) that occur at **compile time**. I would like to
>>>> take a per-annotation approach here: some annotations like JSpecify or
>>>> Error Prone's `@InlineMe` can be really useful to users, so we can:
>>>> 
>>>> * add 3819 bytes of JSpecify dependency to the `compile` scope of all
>>>> artifacts. In the near future nullability annotations will be all over
>>>> the place and JSpecify (compared to the other 13 kinds of annotations
>>>> there) have some strong supporters[5].
>>>> * keep the current _status quo_ for some annotations (e.g. `@InlineMe`
>>>> and those OSGi versioning annotations that are not mirrored in the
>>>> Manifest),
>>>> * write a tool that removes the other annotations from the class files.
>>>> 
>>>> [5]https://jspecify.dev/about/
>>> 
>>> I decided to start implementing this proposal and I created PR #3228 to add JSpecify as `compile` dependency. The PR still allows users (include OSGi and JPMS users) to manually exclude the dependency, but I doubt anyone will do it.
>>> 
>>> Are you OK with this change? IIRC the main argument against having non-optional dependencies in `log4j-api` and `log4j-core` was that we don't control them. The argument still stands, but it is micro dependency that is controlled by a large group of vendors, so its evolution is pretty much under control.
>>> 
>>> Piotr
>>> 
>>> [1] https://github.com/apache/logging-log4j2/pull/3228
>> 


Re: Add JSpecify as `compile` dependency

Posted by Gary Gregory <ga...@gmail.com>.
The main issue with these annotations is that there are so many
providers. It's so bad that tooling just now look for names like
@NonNull _without considering the package name_, so there is no limit
as to how much you end up with :-(

On Mon, Dec 2, 2024 at 1:58 PM Matt Sicker <ma...@musigma.org> wrote:
>
> I’m alright with the JSpecify dependency assuming it has accumulated the momentum we expected. Also, I thought annotations didn’t have to exist at runtime anyways as long as you weren’t using reflection on them.
>
> > On Nov 22, 2024, at 03:52, Piotr P. Karwasz <pi...@mailing.copernik.eu> wrote:
> >
> > Hi all,
> >
> > On 19.10.2024 09:28, Piotr P. Karwasz wrote:
> >> PS: Regarding annotations, can you take a look at issue #3110[4].
> >> Users are regularly reporting issues for annotations with a `provided`
> >> scope. This is due to the `classfile` option to `-Xlint`, which
> >> basically only covers missing annotations (of both `CLASS` and
> >> `RUNTIME` retention) that occur at **compile time**. I would like to
> >> take a per-annotation approach here: some annotations like JSpecify or
> >> Error Prone's `@InlineMe` can be really useful to users, so we can:
> >>
> >> * add 3819 bytes of JSpecify dependency to the `compile` scope of all
> >> artifacts. In the near future nullability annotations will be all over
> >> the place and JSpecify (compared to the other 13 kinds of annotations
> >> there) have some strong supporters[5].
> >> * keep the current _status quo_ for some annotations (e.g. `@InlineMe`
> >> and those OSGi versioning annotations that are not mirrored in the
> >> Manifest),
> >> * write a tool that removes the other annotations from the class files.
> >>
> >> [5]https://jspecify.dev/about/
> >
> > I decided to start implementing this proposal and I created PR #3228 to add JSpecify as `compile` dependency. The PR still allows users (include OSGi and JPMS users) to manually exclude the dependency, but I doubt anyone will do it.
> >
> > Are you OK with this change? IIRC the main argument against having non-optional dependencies in `log4j-api` and `log4j-core` was that we don't control them. The argument still stands, but it is micro dependency that is controlled by a large group of vendors, so its evolution is pretty much under control.
> >
> > Piotr
> >
> > [1] https://github.com/apache/logging-log4j2/pull/3228
>