Bug 137437

Current approach:
1) module defining annotations org.netbeans.api.annotation.common
2) annotations are defined with SOURCE retention policy
3) with special build time flag annotations are compiled with CLASS retention policy and annotated with jsr305 meta
annotations (this is required by findbugs) - then you can perform findbugs check that will take these annotations into
account
4) note that any build performed without the special flag is "annotation free"

Unclear things and things not (yet) mentioned in wiki:
- external reference to jsr305 when using the nb build performed with the special flag
- annotation CheckForNull should be renamed or it should be splitted into two - one for method return value the other
for method parameter. This is because meaning of CheckForNull used on method parameter is not clear without reading the
documentation.
- if we design these annotations carefully there is no need to deprecate them in future (valid at least for nullness
annotations) as these can be just annotated with jsr305 meta annotations (maybe it is even preferred)

Created attachment 62929 [details]
current solution

Annotations on Lookup in previous patch are (right now) just for testing purposes.

Looks great - can't wait to use it.

Attaching updated patch. Perhaps we could remove CheckForNull completely and use NullAllowed even for methods.

Created attachment 64972 [details]
updated patch

Let's discuss this.

[JG01] I do not like the magical injection of this module into the classpath of other modules, conditional build of
different sources, etc. Just makes our complex build infrastructure and NBM project type even more complicated and
difficult to maintain. Would recommend the straightforward approach of one regular API in a regular autoload module,
with CLASS retention and @Documented. Does it really harm anything? If and when JSR 305 is released in final form, would
be trivial to switch to it.


[JG02] "SupressWarnings" is a typo.

Re JG01: I agree that direct CLASS retention and autoload module is much simpler. The trouble is the JSR305 reference -
our annotations has to be annotated with JSR305 meta annotations in order to be usable in code checking tools such as
findbugs. Additional side effect is that there would be no need to deprecate our annotations in future (maybe it would
be better because of confusing Nullable and CheckForNull annotations defined in JSR305).

I'm not sure whether it is legal to put current JSR305 jar to our module (although it is licensed under new BSD license).

Another solution would be just to patch findbugs for our purposes, but usually people are not happy about such approach.

re JG02: I'll fix it.

If including jsr305.jar in the build is a problem, perhaps you could create annotations with the same names in the
annotations API module; just do not expose the javax.** packages as public. I think FindBugs would then get it right.
When the JSR is released and we have legal approval, we just include it in the build as a new module; deprecate our
annotations module; do a simple search-and-replace over our sources to use the JSR annotations; make the old annotations
module dep on the JSR module rather than including its own private copy.

Y01 I'd like to slightly disagree with Jesse. Although having own annotations with CLASS retention is relatively 
acceptable from my point of view, I really do not want to re-export any 3rd party annotations as part of NetBeans 
platform. That means I do not want jsr305 & co. to be part of our final production build until the annotations are 
stabilized. I'd rather have more magical build than make promises to users of the NetBeans Platform, that we cannot 
keep, or influence - like inclusion of non-finished jsr305. Btw. why not have this 'build magic' only temporary until 
jsr305 is final?

To Y01 - my suggestion was

"perhaps you could create annotations with the same names in the annotations API module; just do not expose the javax.**
packages as public"

i.e. FindBugs would see the 305 meta-annotations (I hope) but these would not be visible in the module's API.

I think both approaches could work. Jardo is Jesse's last suggestion ok for you? Can we use javax namespace in our
module? I can prepare another patch once we will have some agreement on this.

Having non-public javax.** in api.annotations.common is OK, I guess. However I am afraid that this approach does not 
allow you to annotate Lookup & co. org-openide-util is not regular module and as such it cannot have dependency on 
api.annotation.common.

Regarding annotating org-openide-util.jar classes: I don't think this is an issue, as we would only use CLASS-retention
annotations. (RUNTIME-retention annotations could only be used on o-o-u.jar if present in another lib/*.jar "fixed"
module, because you then need to consider class loader visibility during linkage.) Using CLASS annotations requires your
project.xml to specify <build-prerequisite/> and <compile-dependency/>, but <runtime-dependency/> is optional.

> Using CLASS annotations requires your
> project.xml to specify <build-prerequisite/> and <compile-dependency/>, but <runtime-dependency/> is optional.

Looks quite hacky from my point of view. Maybe more than few changes in projectized.xml, but OK, do what you want.

For regular modules, you can have a regular module dependency on the annotations module if you like, and I would expect
most people to do it this way simply because you do not need to know any better. It is only lib/*.jar and core/*.jar
("fixed" modules) which need to omit <run-dependency/>.

What is the status of this work, please? Is this proposal still active? Is anybody working on it? Do we have any
estimated timeframe for this work? Thank you

I'm trying to prepare the next patch (with private javax packages as suggested by Jesse at desc11). Right now when I use
it the javac fails with ugly message. I want to try it with javac from jdk6.

"Ugly message"?

We _are_ using javac from JDK 6. If you run Ant with JDK 6 (should be using nbjdk.home=JDK5), including running Ant from
inside NetBeans when NB is running on JDK 6, the standard javac is used. If you run Ant with JDK 5 (discouraged), javac
from OpenJDK 6 is used.

> I'm trying to prepare the next patch ...

Awesome, thank you.

"We _are_ using javac from JDK 6".  Sorry for going off topic, but I haven't been able to build the trunk on a Mac lately. Since I'm working on a release65 
branch clone (python65) this hasn't blocked me, but I just want to make sure that somebody has checked that it does work on a Mac. (I have a pre-release of 
JDK6 installed on my system, which shouldn't be used since it's not in my path, but it's possible it's my environment. Again, just want to make sure that this 
has been confirmed to work on the Mac.)

Regarding the message - I didn't want to pollute this issue with complaints that could be caused by my misuse of the
infrastructure and I wanted to try figure out what was going wrong actually. I tried few changes but still getting the
same message. The message does not appear for compilation of the module itself, but when the annotation is used in other
module.

Now I figured it out - annotations from javax packages (private) are not included in public-packages jar, while these
are required even for compilation of dependent module using annotations from public package.

I'll attach my current patch.

The message itself is (I'm trying to use annotation in server module):
Compiling 19 source files to /home/sickboy/workspace/netbeans-experimental/server/build/classes
/home/sickboy/workspace/netbeans-experimental/nbbuild/build/public-package-jars/org-netbeans-api-annotations-common.jar(org/netbeans/api/annotations/common/NonNull.class):
warning: Cannot find annotation method 'when()' in type 'javax.annotation.Nonnull': class file for
javax.annotation.Nonnull not found
An exception has occurred in the compiler (1.6.0_10). Please file a bug at the Java Developer Connection
(http://java.sun.com/webapps/bugreport)  after checking the Bug Parade for duplicates. Include your program and the
following diagnostic in your report.  Thank you.
com.sun.tools.javac.code.Symbol$CompletionFailure: class file for javax.annotation.meta.When not found
/home/sickboy/workspace/netbeans-experimental/nbbuild/templates/common.xml:146: Compile failed; see the compiler error
output for details.
BUILD FAILED (total time: 0 seconds)

[To Tor's off-topic note: AFAIK it works at the moment but without a Mac I cannot personally confirm. Tomas Zezula is
the person to ask.]

Your "ugly message" looks to be a bug in javac. Please file a separate issue for me in apisupport.harness to (1) report
the bug properly, (2) look for a workaround in the public package distillation code.

Created attachment 73321 [details]
current work

Filed issue 152562 as requested.
Thanks Jesse!

What is really missing to get this done for next milestone? Issue 152562 is fixed. I've read the patch 
http://www.netbeans.org/nonav/issues/showattachment.cgi/73321/annotations_tmp.patch and it looks fine. If everythign 
builds and also works OK with FindBugs, then let's stop waiting and use this for 7.0!

If there are no objections I'll integrate the last patch on Thursday.

Pushed to main - a9ddb7d4b05e and dd5881232265.

Integrated into 'main-golden', will be available in build *200901091401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/a9ddb7d4b05e
User: phejl@netbeans.org
Log: #137437 Annotations for NPE (and some other errors) detection

Guys,

It's not clear which annotations you ended up supporting in the end. Where is this feature documented? What annotations are supported?

(In reply to comment #32)
> Guys,
> 
> It's not clear which annotations you ended up supporting in the end. Where is
> this feature documented? What annotations are supported?

Check the Javadoc of api.annotations.common or the changeset mentioned above.