dnault / therapi-runtime-javadoc

Read Javadoc comments at run time.
Apache License 2.0
117 stars 19 forks source link

backported project to Java 7 #23

Closed bbottema closed 6 years ago

bbottema commented 6 years ago

Backported project to Java 7 for https://github.com/dnault/therapi-runtime-javadoc/issues/22 (but tests still run in Java 8 due to Google Compiler dependency, couldn't solve that one). It now works with the widely accepted and supported @Nullable and @NonNull annotations by findbugs (used in Guava as well).

I hope you can use this (or a variation of this), but please let me know if this is a path you are willing to take. If not, I intend to (re)publish this Java 7 fork in maven central (with a different package of course) if the poc with my own library is successful.

Looking forward to hear your thoughts.

dnault commented 6 years ago

Thanks for this! No need to fork. I'll review over the weekend and will release the new version asap.

dnault commented 6 years ago

After doing some research, I'm wondering about the advisability of adding a dependency on the jsr305 annotations at this time in history. Apparently it can cause issues with Java 9, and there are some ambiguities about the licensing. Google appears to be moving away from it, along with some other folks.

https://github.com/google/guava/issues/2960 https://github.com/rsocket/rsocket-java/issues/423 https://blog.codefx.org/java/jsr-305-java-9/ https://lists.apache.org/thread.html/%3Cebaba8a3-0750-b95c-38ae-36714ec0404f@gmx.de%3E

I'll remove the annotations for now.

bbottema commented 6 years ago

Yikes, that's new to me. I'll look into it myself as well since I'm using it everywhere (thanks for those links btw). I know there are alternative annotations that enjoy the same wide support and don't share existing packages from the JDK.

Personally I'm not sure where I stand on using Optional or Null object pattern in Simple Java Mail. It feels like hiding the underlying problem to me, which is that data is missing. I think it will cause me to very defensively check .isPresent() which is basically an JDK API for an existing language construct (superfluous abstraction). Maybe I'll change my mind after this research though!

Very glad you released a Java 7 version of your library.

bbottema commented 6 years ago

There seems to be a split between the spotbugs annotations, checker framework and -as an intermediate painless solution until there is a clear path-: Jetbrain's Intellij annotations. Not only is the issue to find an alternative that is still maintained, compatible with Java 9, actually has all the annotations currently used, doesn't bring transitive baggage and is supported by analyses tools, but on top of all that license also is an issue:

I'm surprised noone is discussing replacing nullability-annotations with Optional return types. I assume because that would mean many breaking changes to the API.

For now I'm partial to the JetBrains annotations, but they are very limited. Luckily the two included are exactly the ones I use right now (@Nullable and @NonNull). I might check out the checker framework as well if I have some time.

dnault commented 6 years ago

I think it will cause me to very defensively check .isPresent()

That would be unfortunate. It's possible to use an "absent" ClassJavadoc object just like a "present" one... it just wont have docs for any fields or methods, and its comment will be blank. This is not so different from the behavior you'd see if you documented only one method of a class; in that case, the class-level comment would be blank, and you'd only see info for the single documented method.

Maybe it would have been better to have an isEmpty() method instead of isPresent().

Well, anyway... give it a try and let me know how it goes.