Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #11507 getAttribute javadoc #11843

Merged
merged 1 commit into from
May 27, 2024

Conversation

gregw
Copy link
Contributor

@gregw gregw commented May 26, 2024

Fix #11507 getAttribute javadoc.
As some stage we should consider @nullable or @NotNull annotations...

Fix #11507 getAttribute javadoc.
As some stage we should consider @nullable or @NotNull annotations...
@gregw gregw requested a review from janbartel May 26, 2024 23:11
@olamy
Copy link
Member

olamy commented May 27, 2024

Fix #11507 getAttribute javadoc. As some stage we should consider @nullable or @NotNull annotations...

yup but which one :)

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

this list is missing the new jakarta one and this other new one coming https://jspecify.dev/

@cowwoc
Copy link
Contributor

cowwoc commented May 27, 2024

@olamy fortunately or unfortunately, it doesn't matter. You can declare an annotation with the same name in any package and IDEs typically treat it correctly. You could declare your own @NotNull (or whatever name IDEs look for by default) and switch over some official annotation library when such a thing gains popularity.

@gregw gregw merged commit 7e36f3c into jetty-12.0.x May 27, 2024
10 checks passed
@gregw
Copy link
Contributor Author

gregw commented May 27, 2024

@olamy fortunately or unfortunately, it doesn't matter. You can declare an annotation with the same name in any package and IDEs typically treat it correctly. You could declare your own @NotNull (or whatever name IDEs look for by default) and switch over some official annotation library when such a thing gains popularity.

@cowwoc That makes me smile! I often say facetiously :

standards are such a great thing that it is worth while having two or three of them!
and
when there are too many frameworks to evaluate them all, it is bet just to write your own.

This @NotNull situation perfectly captures both.

I'll open an issue for us to consider this at some stage.

@gregw
Copy link
Contributor Author

gregw commented May 27, 2024

opened #11848

@olamy
Copy link
Member

olamy commented May 27, 2024

@olamy fortunately or unfortunately, it doesn't matter. You can declare an annotation with the same name in any package and IDEs typically treat it correctly. You could declare your own @NotNull (or whatever name IDEs look for by default) and switch over some official annotation library when such a thing gains popularity.

nah It's a problem. I don't mind IDE. But I wonder about build tools you will eventually need to bind a different plugin for each annotation :)

@cowwoc
Copy link
Contributor

cowwoc commented May 28, 2024

@olamy Are you aware of any tools that bind against a fully-qualified annotation name? I wouldn't be surprised if this was the case, but I've never seen one. Every single tool I've seen ignored the package name (by design) precisely because everyone was declaring their own.

@olamy
Copy link
Member

olamy commented May 28, 2024

@cowwoc I didn't check but even if not using the fqcn the simple name is already different :)

NotNull
NonNull
Nonnull

@cowwoc
Copy link
Contributor

cowwoc commented Jun 2, 2024

Interestingly enough, it turns out that IntelliJ treats @CheckReturnValue differently from nullability annotations. The former ignores the package name. The latter only checks for the following fully-qualified annotation names:

android.support.annotation.Nullable
androidx.annotation.Nullable
androidx.annotation.RecentlyNullable
com.android.annotations.Nullable
edu.umd.cs.findbugs.annotations.Nullable
io.reactivex.annotations.Nullable
io.reactivex.rxjava3.annotations.Nullable
jakarta.annotation.Nullable
javax.annotation.CheckForNull
javax.annotation.Nullasble
org.checkerframework.checker.nullness.compatqual.NullableDecl
org.checkerframework.checker.nullness.compatqual.NullableType
org.checkerframework.checker.nullness.qual.Nullable
org.eclipse.jdt.annotation.Nullable
org.jetbrains.annotations.Nullable
org.jspecify.annotations.Nullable

They have a similar list for not-null annotations. So I take it back, you can't just create your own annotation (unless you want to go around convincing all the IDEs to add support for it).

@joakime
Copy link
Contributor

joakime commented Jun 3, 2024

The org.eclipse.jdt.annotation.Nullable would be easy for us to use, as it's very license compatible.

<dependency>
    <groupId>org.eclipse.jdt</groupId>
    <artifactId>org.eclipse.jdt.annotation</artifactId>
    <version>2.3.0</version>
</dependency>

It's pretty lightweight too ...

$ jar -tvf org.eclipse.jdt.annotation-2.3.0.jar 
  2741 Fri Jan 12 17:26:36 CST 2024 META-INF/MANIFEST.MF
  2348 Fri Jan 12 17:26:38 CST 2024 META-INF/ECLIPSE_.SF
  9555 Fri Jan 12 17:26:38 CST 2024 META-INF/ECLIPSE_.RSA
     0 Fri Jan 12 17:26:36 CST 2024 META-INF/
     0 Fri Jan 12 17:26:36 CST 2024 org/
     0 Fri Jan 12 17:26:36 CST 2024 org/eclipse/
     0 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/
     0 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/
   478 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/NotOwning.class
   132 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/package-info.class
   580 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/NonNullByDefault.class
   436 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/Nullable.class
  9257 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/Checks.class
   476 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/Owning.class
   434 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/NonNull.class
  1435 Fri Jan 12 17:26:36 CST 2024 org/eclipse/jdt/annotation/DefaultLocation.class
     0 Fri Jan 12 17:26:36 CST 2024 src/
     0 Fri Jan 12 17:26:36 CST 2024 src/org/
     0 Fri Jan 12 17:26:36 CST 2024 src/org/eclipse/
     0 Fri Jan 12 17:26:36 CST 2024 src/org/eclipse/jdt/
     0 Fri Jan 12 17:26:36 CST 2024 src/org/eclipse/jdt/annotation/
 20426 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/Checks.java
  4520 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/DefaultLocation.java
  2574 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/NonNull.java
  3198 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/NonNullByDefault.java
  1833 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/NotOwning.java
  1947 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/Nullable.java
  4826 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/Owning.java
  1060 Fri Jan 12 17:15:38 CST 2024 src/org/eclipse/jdt/annotation/package-info.java
   216 Fri Jan 12 17:26:36 CST 2024 .api_description
  1460 Fri Jan 12 17:15:38 CST 2024 about.html
   608 Fri Jan 12 17:15:38 CST 2024 bundle.properties

The pom also has no dependencies, or even a parent pom.

@joakime joakime deleted the fix/jetty-12.0.x/11507/getAttribute branch June 27, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

org.eclipse.jetty.util.Attributes.getAttribute() should specify return type if no match is found
5 participants