search

JetBrains / java-annotations

Annotations for JVM-based languages.
Apache License 2.0
404 stars 47 forks source link

What about @ParametersAreNonnullByDefault? #18

Closed bibarsov closed 1 week ago

bibarsov commented 4 years ago

I think it's a great replacement for jsr-305's annotations (in the context of IntellijIdea's inspection). Would Jetbrains support that annotation?

bibarsov commented 4 years ago

The same question is for @ParametersAreNullableByDefault

amaembo commented 4 years ago

For now, we are not planning to add them.

matheusstutzel commented 3 years ago

@amaembo any updates on this?

Sxtanna commented 3 years ago

Sorry but... there are existing semantics for this across several implementations of this annotation within this scope. There is precedent for this, it's really not MUCH more complex, as mentioned in #60 by @amaembo.

Please just make it work how other annotations have worked, for several years.

overheadhunter commented 2 years ago

For now, we are not planning to add them.

I guess the rationale for this decision was that JSR-305 already provides it and therefore there was no need to add it.

In the meantime the situation has changed, though: org.jetbrains.annotations is a proper java module, so I guess you are well aware of the split package mess that including JSR-305 would cause.

As JetBrains is known to be very supportive of (and many times even leading) new standards, I guess it's time to reconsider.

CC007 commented 2 years ago

I found this page: https://www.jetbrains.com/help/idea/parametersarenonnullbydefault-annotation.html

Does that mean it is supported now?

overheadhunter commented 2 years ago

Does that mean it is supported now?

No, not without said dependency which causes split packages:

To use the annotation, add the jsr305 library to module dependencies

alexei-osipov commented 5 months ago

If there no plans to add equivalent @ParametersAreNonnullByDefault could you please provide a recommendation on what should be used instead. Obviously, JSR 305 can't be used because of deprecation and split package problem since Java 9.

amaembo commented 1 week ago

NotNullByDefault is added in 26.0.0 https://javadoc.io/static/org.jetbrains/annotations/26.0.0/org/jetbrains/annotations/NotNullByDefault.html

matheusstutzel commented 1 week ago

@amaembo are there any plans to add NullableByDefault as well?

amaembo commented 1 week ago

@matheusstutzel no, it's unnecessary.

CC007 commented 1 week ago

@matheusstutzel no, it's unnecessary.

Is it unnecessary? What if you want notnull by default in some.package, but you want nullable by default in some.package.subpackage specifically? Or nullable by default in a certain class that's in a notnull package?

amaembo commented 1 week ago

There's no such thing as sub-packages in Java. The annotation doesn't affect any packages other than the package where it's declared.

Having Nullable by default is a rare choice, as most of the APIs are notnull. Also, and complex annotation hierarchy could be confusing for API users. It's better to annotate Nullable methods explicitly, or not using NotNullByDefault at all. Alternative, you can annotate every class with NotNullByDefault, except those classes where Nullable APIs prevail.

CC007 commented 1 week ago

Does NotNullByDefault support annotating modules, so the whole module is notnull by default?

amaembo commented 1 week ago

Not yet, though we are considering this.

CC007 commented 1 week ago

There's no such thing as sub-packages in Java. The annotation doesn't affect any packages other than the package where it's declared.

Packages inside other packages are often referred to as subpackages. Also, the concept is used often in things like component scans in Spring, which scans for components in the specified package and any subpackages.

It's fine though for this annotation to not apply to subpackages.

Maybe one day the annotation could be expanded with a boolean applyToSubPackages parameter. Maybe the discussion of adding a NullableByDefault can be reopened if that ever gets added.

amaembo commented 1 week ago

This is not a good idea. Packages with the same prefix could be a part of another artefact, which is completely unaware of your annotation. This problem should be solved with module-level annotation.

CC007 commented 1 week ago

This is not a good idea. Packages with the same prefix could be a part of another artefact, which is completely unaware of your annotation. This problem should be solved with module-level annotation.

I created an issue for the module-level support for this annotation.

Other than that, I agree that you don't want to willy nilly apply the annotation to packages with the same prefix in another module. I do believe it could be possible to only cascade down to the subpackages within the current module though. That way you would avoid that side effect.