Bug 137437 - Annotations for NPE (and some other errors) detection
Annotations for NPE (and some other errors) detection
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: -- Other --
6.x
All All
: P2 (vote)
: 6.x
Assigned To: apireviews
issues@platform
: API, API_REVIEW
Depends on: 157320 157328
Blocks: 89594
  Show dependency treegraph
 
Reported: 2008-06-17 14:12 UTC by Petr Hejl
Modified: 2012-11-21 21:23 UTC (History)
10 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
current solution (33.54 KB, text/plain)
2008-06-17 14:35 UTC, Petr Hejl
Details
updated patch (36.14 KB, text/plain)
2008-07-18 15:19 UTC, Petr Hejl
Details
current work (36.02 KB, text/plain)
2008-11-05 20:33 UTC, Petr Hejl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Hejl 2008-06-17 14:13:00 UTC
There is big interest in software error detection. Tools like findbugs (and several others) already define annotations
to achieve this. I think it would be very useful to have similar annotation types in NetBeans.

There is already JSR305 in progress, but I'm afraid we can't wait for JDK7.

http://wiki.netbeans.org/FindBugsAnnotationsProposal
Comment 1 Petr Hejl 2008-06-17 14:33:36 UTC
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)
Comment 2 Petr Hejl 2008-06-17 14:35:21 UTC
Created attachment 62929 [details]
current solution
Comment 3 Petr Hejl 2008-06-17 14:36:10 UTC
Annotations on Lookup in previous patch are (right now) just for testing purposes.
Comment 4 Torbjorn Norbye 2008-06-19 01:16:20 UTC
Looks great - can't wait to use it.
Comment 5 Petr Hejl 2008-07-18 15:18:02 UTC
Attaching updated patch. Perhaps we could remove CheckForNull completely and use NullAllowed even for methods.
Comment 6 Petr Hejl 2008-07-18 15:19:17 UTC
Created attachment 64972 [details]
updated patch
Comment 7 Petr Hejl 2008-07-18 15:22:45 UTC
Let's discuss this.
Comment 8 Jesse Glick 2008-07-18 17:33:36 UTC
[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.
Comment 9 Petr Hejl 2008-07-18 19:03:09 UTC
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.
Comment 10 Jesse Glick 2008-07-18 20:43:24 UTC
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.
Comment 11 Jaroslav Tulach 2008-07-22 09:13:33 UTC
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?

Comment 12 Jesse Glick 2008-07-22 19:35:00 UTC
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.
Comment 13 Petr Hejl 2008-10-13 21:49:23 UTC
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.
Comment 14 Jaroslav Tulach 2008-10-14 08:11:08 UTC
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.
Comment 15 Jesse Glick 2008-10-15 01:55:17 UTC
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.
Comment 16 Jaroslav Tulach 2008-10-15 10:42:35 UTC
> 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.
Comment 17 Jesse Glick 2008-10-15 15:50:41 UTC
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/>.
Comment 18 Vitezslav Stejskal 2008-11-05 16:06:01 UTC
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
Comment 19 Petr Hejl 2008-11-05 16:14:05 UTC
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.
Comment 20 Jesse Glick 2008-11-05 18:27:45 UTC
"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.
Comment 21 Vitezslav Stejskal 2008-11-05 19:00:27 UTC
> I'm trying to prepare the next patch ...

Awesome, thank you.
Comment 22 Torbjorn Norbye 2008-11-05 19:40:17 UTC
"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.)
Comment 23 Petr Hejl 2008-11-05 20:25:27 UTC
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)
Comment 24 Jesse Glick 2008-11-05 20:26:36 UTC
[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.]
Comment 25 Jesse Glick 2008-11-05 20:28:55 UTC
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.
Comment 26 Petr Hejl 2008-11-05 20:33:50 UTC
Created attachment 73321 [details]
current work
Comment 27 Petr Hejl 2008-11-06 17:33:36 UTC
Filed issue 152562 as requested.
Thanks Jesse!
Comment 28 Jaroslav Tulach 2008-12-11 21:23:36 UTC
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!
Comment 29 Petr Hejl 2009-01-06 19:05:17 UTC
If there are no objections I'll integrate the last patch on Thursday.
Comment 30 Petr Hejl 2009-01-08 12:29:48 UTC
Pushed to main - a9ddb7d4b05e and dd5881232265.
Comment 31 Quality Engineering 2009-01-09 16:48:35 UTC
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
Comment 32 _ gtzabari 2012-11-21 21:18:07 UTC
Guys,

It's not clear which annotations you ended up supporting in the end. Where is this feature documented? What annotations are supported?
Comment 33 Petr Hejl 2012-11-21 21:23:28 UTC
(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.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo