Community
Participate
Working Groups
The mechanisms of bug 186342 and bug 331647 are limited by the fact that existing libraries do not (yet) specify the nullity contracts of their API. A profile mechanism should be provided that feeds external nullity information regarding those libraries into the compiler. Such a profile can be obtained from byte code analysis of the library. Such tools and profiles exist in other projects and the possibility to re-use those should be investigated. Using the default mechanism from bug 331647 should help to keep profiles compact.
Putting in JDT/Core triaged for now. Will revisit once bug 186342 is fixed.
It may not be necessary to invent a new format. The "annotation file format" of the Annotation File Utilities (http://types.cs.washington.edu/annotation-file-utilities/annotation-file-format.html) is already used by Javarifier (http://types.cs.washington.edu/javari/javarifier/), the Julia Analyzer (http://julia.scienze.univr.it/), the Nit Nullability Inference Tool (http://nit.gforge.inria.fr/), JastAdd (http://jastadd.org/), the Checker Framework (http://types.cs.washington.edu/checker-framework/), and other tools. The Annotation File Utilities (http://types.cs.washington.edu/annotation-file-utilities/) provide tools for reading annotations from, and writing them to, .java and .class files. There are also tools for creating summaries of libraries. The format is forward-compatible with the type annotations that are planned for Java 8. Re-using this existing format will give instant access to tools for manipulating the annotation files, in addition to interoperability with existing inference and type-checking tools.
(In reply to comment #2) Many thanks for the links. This looks exactly like what we'd need here. So this file format is used by many tools: good. The format is specified but not an official standard of any kind: hm. There's implementation under the MIT license, which should in theory be compatible with EPL, but let's see if it is worth the heavy IP process: the JDT would only need a parser for that format. Would that be the class annotations.io.IndexParser? Could that be used without a lot of dependencies? Would you be willing to contribute this code to Eclipse (to be further developed here and under the EPL)? Were there any additional authors involved in writing that code? OTOH, as I'm starting to think about a potential implementation: I could actually see this integrated via an extension point, something like an ExternalAnnotationProvider. That would spare us the discussion about standards and going through the IP process up-front. We could define a simple API of our own that could easily be served from an annotation file parser. We would still need to discuss who provides the plug-in implementing the annotation provider for the Annotation File Format, but that sounds like an easily solvable issue to me. Out of curiosity: do you have estimates how large a file would get when used to store all null contracts for the JDK? Would that be feasible? I remember Patrice Chalin mentioning they had computed all these contracts. Have you done similar analysis?
I don't see the need to transfer ownership of the code, nor to introduce a dependency on Eclipse in every project that uses the Annotation File Format. However, your bug reports and patches will be gratefully accepted. Of course, I can't stop you from forking the code if you must do that. Integrating it as an external (to Eclipse) library seems quite expedient. > Out of curiosity: do you have estimates how large a file would get when used > to store all null contracts for the JDK? Would that be feasible? We annotated part of the JDK. You can browse it at http://code.google.com/p/checker-framework/source/browse/#hg%2Fcheckers%2Fjdk%2Fnullness%2Fsrc%2Fjava It's bigger than it needs to be, because in some cases Javadoc and method bodies are included. It's smaller than it needs to be, because it is only java.io, java.lang, java.util, and their subpackages. > I remember Patrice Chalin mentioning they had computed all these contracts. Are you thinking of this paper? "Non-null references by default in {Java}: Alleviating the nullity annotation burden", by Patrice Chalin and Perry R. James. In ECOOP 2007. My understanding is that this measures the number of variables that are dereferenced without first being checked against null. For all other variables, we don't know whether they ought to be annotated as nullable or as non-null. A subsequent paper (http://www.cs.washington.edu/homes/mernst/pubs/pluggable-checkers-issta2008-abstract.html) shows that a different default than proposed in Chalin's paper is even more effective.
(In reply to comment #4) > I don't see the need to transfer ownership of the code, nor to introduce a > dependency on Eclipse in every project that uses the Annotation File > Format. However, your bug reports and patches will be gratefully accepted. > Of course, I can't stop you from forking the code if you must do that. > Integrating it as an external (to Eclipse) library seems quite expedient. I wasn't asking you to abandon your rights :) From my understanding it's a hard requirement that the Eclipse Java compiler must run as a standalone application with no third-party dependencies. This could mean that this combination is only possible if we indeed go through the IP process etc. For that we would need your help in answering some provenance questions. OTOH I will investigate whether some kind of plug-in mechanism can be used in the compiler even in standalone mode. > We annotated part of the JDK. You can browse it at > > http://code.google.com/p/checker-framework/source/browse/#hg%2Fcheckers%2Fjdk%2Fnullness%2Fsrc%2Fjava that's cool. thanks for the link. Did you use any tool for inferring these annotations? > > I remember Patrice Chalin mentioning they had computed all these contracts. > > Are you thinking of this paper? > > "Non-null references by default in {Java}: Alleviating the nullity > annotation burden", by Patrice Chalin and Perry R. James. In ECOOP 2007. These are separate issues: the empirical study (this paper) and the JDK contracts (somewhere else). > My understanding is that this measures the number of variables that are > dereferenced without first being checked against null. For all other > variables, we don't know whether they ought to be annotated as nullable or > as non-null. The paper is an empirical investigation over the hypothesis: "In Java programs, at least 2/3 of declarations (other than local variables) that are of reference types are meant to be non-null, based on design intent." From their data they conclude that non-null is indeed the better default. In section 5.2 they have a discussion of backwards compatibility and migration that is very relevant for our bug 331647. > A subsequent paper > (http://www.cs.washington.edu/homes/mernst/pubs/pluggable-checkers-issta2008-abstract.html) > shows that a different default than proposed in Chalin's paper is even more > effective. I don't see how your NonNullExceptLocals is different from Patrice's suggestion, am I missing anything? (I'm not saying this as criticism but I'm happy about the convergence actually).
I recently did more experiments with my current implementation for bug 186342. After adding relevant annotations, almost 20% of the remaining warnings are due to missing null information from the JDK. Thusly motivated let me summarize the questions and options I currently see: A. *FILE FORMAT* 1. Normal Java class files, just without method bodies. 2. The "annotation file format" mentioned in comment 2 3. A custom format Abitilty to parse (1) is natively given in the compiler. Tooling for (2) exists, but after discussions at ECOOP I seemed to understand that this format is designed more for offline tools, not necessarily for the compiler. (3) should only be developed, if the others don't satisfy. B. *CREATION PROCESS* 1. Created offline and globally collected in some repository. (a) only full specifications or (b) best-effort partial spec. allowed 2. Cummulative while using the IDE: Quickfix: add null-spec for this API to local spec-repo C. *COMPILER INTEGRATION* 1. A fully spec'ed profile using Java class files could actually be used /instead/ of the original API during compilation. -> Legal issues? -> Is it safe to compile against a different jar than the JRE used at RT? 2. Use null-spec as an overlay in addition to regular jars 3. Where are null-specs found? (classpath vs. separatly configured location) Perhaps: put on classpath but mark with custom file extension. 4. Is the profile/spec support part of the compiler proper, or a pluggable extension of some sort? Am I missing any relevant questions / options? Particularly B(2) would have been helpful during my recent experiments, because just a few methods from the JDK caused a significant number of unnecessary warnings.
(In reply to comment #6) > C. *COMPILER INTEGRATION* > 1. A fully spec'ed profile using Java class files could actually be used > /instead/ of the original API during compilation. > -> Legal issues? > -> Is it safe to compile against a different jar than the JRE used at RT? Just replacing existing library jars with stubs that only contain the annotated signatures would of course be the *easiest* solution, requiring *no* changes in the JDT. Assuming we indeed want to superimpose annotations over those third party jars we'll have to address the performance issues of potentially having to deal with twice the number of files. Pondering about these performance questions I'm leaning towards thinking of two different file formats: (A) use a standard format (like .class) for exchange with other tools (B) use an optimized format internally, maybe we can cache all annotations in memory with only minimal footprint. As an internal format the following information would be sufficient: (I) per jar: + file name (absolute path on local disk?) + CRC to detect when the jar was replaced (II) per class within a jar: + fqn (III) per annotated member: + index into the original constant pool + sequence of bytes(?) denoting annotations per parameter and (return) type. For these bytes we could, e.g., simulate one shared constant pool per workspace. By sharing the constant pool indices of the original jar it would be extremely easy for the ClassFileReader to associate external annotations with binary methods / fields. Also, the footprint of external annotations would be very small. This internal format could be created by some kind of import action which reads both the original jar and a corresponding jar containing just the signatures w/ annotations. This action could be triggered, e.g., whenever external annotations are attached to a given library jar. I could also think of an integration with the index manager. I guess I should make some experiments with such an internal format to check performance and complexity of integration with the compiler. This could be done independently of what exact tool chain and workflow we use for creating this format and for feeding it into the JDT. Other formats like the one mentioned in comment 2 can still be used but such usage would be fully decoupled from the compiler, as we would transform any text-format to .class first before importing to the internal format. Initially it will be sufficient to think of this internal format as an immutable structure, only created by the import action. In the future it may become interesting to interactively add annotations, e.g., by using a quickfix. At that point it will become relevant to think of reverse translating internal format to something sharable.
This is being moved to the next release as I expect some significant challenges in scheduling sufficient resources for support functions such as design and implementation reviews, white box testing, verification etc. The JDT/UI team also expressed difficulties in scheduling and accommodating their portion of the work in the 3.8 time frame. Design discussions and implementation work need not be halted altogther on account of this. To the extent possible, steady progress can be made and an early milestone of the next release could be used as the release vehicle.
What is the status? Is there already a file format? Can we contribute on define the null behavior of the JDK self?
(In reply to comment #9) > What is the status? Is there already a file format? Thanks for your interest. The overall status of annotation based null analysis comes in three stages: - basic support: released for Juno (JDT 3.8) - null annotations for fields (bug 331649): to be uploaded as an experimental patch feature (date of publication close to the Juno release) - nullity profiles for libraries: currently unscheduled > Can we contribute on define the null behavior of the JDK self? That would be very much appreciated! In order to make parallel development possible, I'm willing to promise that we will be able to read .class file format, either directly from the compiler or by an offline transformation towards the internal format (should we define one). Given that the annotation format mentioned by Michael (comment 2) can be transformed to .class we should be able to easily connect to their results, too. When collecting a nullity profile - say - for the JDK by a community effort, we'd have to define some process to ensure that the result is reliable. As you speak of contributing, what method of finding null annotations are you thinking of?
> When collecting a nullity profile - say - for the JDK by a community effort, > we'd have to define some process to ensure that the result is reliable. Does we need it for the first version really? Some mistake on beginning are better as no definition. People which will contribute can attached annotated source files to this ticket (or another if there exists). You or other people with commit rights can commit it. I expected that the people that checkin it take a short look that it is a valid file. > As you speak of contributing, what method of finding null annotations > are you thinking of? This can only be possible through reviewing the API documentation. If the documentation is not clear then also a look in the source code is needed. For example the return values of the 3 methods getClass, toString and clone from java.lang.Object are Nonnull.
(In reply to comment #11) > > When collecting a nullity profile - say - for the JDK by a community effort, > > we'd have to define some process to ensure that the result is reliable. > > Does we need it for the first version really? Some mistake on beginning are > better as no definition. Frankly, I'm not sure about that. I'd really like these profiles to be reliable. But maybe the process will define different levels of certainty. So people with lower expectations can share best-efforts results, while others will only use a "verified" version... > > As you speak of contributing, what method of finding null annotations > > are you thinking of? > > This can only be possible through reviewing the API documentation. If the > documentation is not clear then also a look in the source code is needed. Comment 2 lists a few tools that automatically infer null information. That's an attractive alternative. > For > example the return values of the 3 methods getClass, toString and clone from > java.lang.Object are Nonnull. One caveat: for non-final library classes, specifying a @NonNull return requires all overrides to also ensure non-null return values.
> Frankly, I'm not sure about that. I'd really like these profiles to be > reliable. But maybe the process will define different levels of certainty. So > people with lower expectations can share best-efforts results, while others > will > only use a "verified" version. You are sure that we need different annotation versions of the JDK? Can you tell a sample for a specific method? An method parameter accept null values or not. If a method does not return a null value then it should never return null values. > Comment 2 lists a few tools that automatically infer null information. That's > an attractive alternative. Which do you means? My understand was that this tools use null annotation. Not that it detect the null behavior automatically. > One caveat: for non-final library classes, specifying a @NonNull return > requires all overrides to also ensure non-null return values. This is right. If this is not the case then it is a bug. It will be interesting to check the source code of JDK self.
(In reply to comment #13) > > Frankly, I'm not sure about that. I'd really like these profiles to be > > reliable. But maybe the process will define different levels of certainty. So > > people with lower expectations can share best-efforts results, while others > > will > > only use a "verified" version. > > You are sure that we need different annotation versions of the JDK? Can you > tell a sample for a specific method? An method parameter accept null values or > not. If a method does not return a null value then it should never return null > values. I'm not sure we speak of the same thing here. My concern is not just technical, but a matter of trust: if someone submits an annotation asserting that a given JDK method never returns null, do I *trust* that the submitter has carefully analyzed all possible execution paths? If his manual analysis was wrong, I may create bugs in my application based on that annotation, which can hurt. That's a social dependency that must be taken care of by the process of collecting library annotations. > > Comment 2 lists a few tools that automatically infer null information. That's > > an attractive alternative. > > Which do you means? My understand was that this tools use null annotation. Not > that it detect the null behavior automatically. There are both kinds of tools. > > One caveat: for non-final library classes, specifying a @NonNull return > > requires all overrides to also ensure non-null return values. > > This is right. If this is not the case then it is a bug. It will be interesting > to check the source code of JDK self. But if a sub-class already exists and the implementor wasn't aware of the non-null behavior (because it wasn't specified in the super-class) adding that contract after the fact bears some danger.
> I'm not sure we speak of the same thing ... We speak from the same things but we have a different view. You want analyze the completely JDK and find the 100% right annotation. You have a problem to *trust* external boys that all was analyze right. The result is that you and some boys must review the completely JDK self. This will need years I think if 95% is right for a first version then it is ok. The wrong annotations can be find fast and can be corrected like bugs. This will need only weeks or months. > But if a sub-class already exists and the implementor wasn't aware of the > non-null behavior (because it wasn't specified in the super-class) adding that > contract after the fact bears some danger. If there is any implementation of toString() that return a null in the JDK then THIS is the bug. We should not change the annotation to Nullable. With our annotations we does not change the contract. We create only a test. The danger exists already because the behavior is not clear. I does not expect that the test can find 100% of NPEs.
(In reply to comment #15) > > I'm not sure we speak of the same thing ... > > We speak from the same things but we have a different view. You want analyze > the completely JDK and find the 100% right annotation. You have a problem to > *trust* external boys that all was analyze right. I personally wouldn't have a problem trusting some folks whom I know :) But when I collect annotations from several contributors and recommend the result to a potentially large group of users, at least I want to be able to make a statement about how reliable these annotations are. If users eliminate null-checks based on the annotations and if they find the hard way that this introduced new bugs because the annotation was wrong, then the entire approach causes more harm than benefits. > The result is that you and > some boys must review the completely JDK self. This will need years Note, that I haven't yet specified what kind of QA process I'm thinking of. Maybe there are shades between no verification and s.t. that takes years. Certainly tools can help in finding correct annotations. > I think if 95% is right for a first version then it is ok. The wrong > annotations can be find fast and can be corrected like bugs. This will need > only weeks or months. I'm not convinced by this statement. Maybe, 95% right isn't actually an improvement? Maybe current code is already 95% correct wrt NPE? Maybe it's exactly the added precision that is needed to eliminate NPE? Why would finding 5% wrong annotations be easier than finding 5% of statements that potentially raise NPE? > > But if a sub-class already exists and the implementor wasn't aware of the > > non-null behavior (because it wasn't specified in the super-class) adding that > > contract after the fact bears some danger. > > If there is any implementation of toString() that return a null in the JDK then > THIS is the bug. Who says? Based on common sense I may agree, but I don't see this in any specification. In other cases there will be even more doubt. > We should not change the annotation to Nullable. > With our annotations we does not change the contract. Yes we do change the contract: From no contract to requiring/promising non-null return. > We create only a test. The other side is: we encourage clients of that method to remove any existing null-checks. > The danger exists already because the behavior is not clear. > I does not expect that the test can find 100% of NPEs. I agree for a short term perspective, but I want to provide a solution that is open for gradually tightening the rules until we *will* have certified 100% NPE-free code (modulo reflection and that sort of tricks). Sorry, if this sounds overly sceptic. I don't mean to discourage anybody. I just want to avoid bad surprises down the road. One way to approach this is simply: Place annotations in different pots: - one collection for annotations derived by automatic analysis - one collection for annotations derived from javadoc - one collection for annotations based on manual code inspection - one collection based on common sense On the one hand, this complicates the issue, but OTOH it allows people to pick exactly their desired level of safety. Yet in other words: I'm not advocating any specific rigid process, I'm just advocating that we should have *some* defined and transparent process.
(In reply to comment #16) > Yet in other words: I'm not advocating any specific rigid process, I'm > just advocating that we should have *some* defined and transparent process. +1. "Trust, but verify" is the way to go, IMHO. We are in this business where despite double and triple checking, design reviews, code reviews, black box testing, white box testing and what not, errors creep into the system. Complexity without controls in place is very unlikely to produce a reliable solution.
(In reply to comment #16) Thanks for the summary, I completely agree. To make nullity profiles comparable / verifiable, the base format should be text-based (i.e. comment 2). To improve performance, JDT/Core is free to use a different format internally (similar situation as the search engine index).
(In reply to comment #16) > Sorry, if this sounds overly sceptic. I don't mean to discourage anybody. > I just want to avoid bad surprises down the road. > > One way to approach this is simply: Place annotations in different pots: > - one collection for annotations derived by automatic analysis > - one collection for annotations derived from javadoc > - one collection for annotations based on manual code inspection > - one collection based on common sense I think you are making too much of this. I have just starting using the Juno code and have wrapped a nonNullXXX(z) around any z that is not analyzed as @NonNull. I found relatively few places where I needed to place nonNullJava(z) for inadequate Java library anntation. I think this is because I'm using my own code and EMF rather than Java libraries and Java libraries are mostly not inherrently null. I certainly need more annotations for EMF than Java. So I would be happy to provide files in which I declare my pragmatic Java library annotations. Once it comes to sharing, the time spent on manual analysis would be much better spent in annotating the primary codebase.
Just being curious: is there any progress on this issue? For me, and undoubtedly for many others, this would be a very welcome feature. What is your estimate on the "completeness rate" of an automated tool to extract nullity information form a library? In other words, if you would unleash it to e.g. the JDK, what percentage of the public API would it be able to analyze reliably?
*** Bug 415758 has been marked as a duplicate of this bug. ***
As the implementation in bug 392099 (null analysis with JSR 308 annotations) is making progress, I made a quick and dirty experiment whether we could provide also a bare minimal solution for this bug maybe during the Luna cycle. The idea is to use only functionality which we already have, i.e., no new file format, no editing support for external annotations, just support for using two separate jars per library: - a stub jar with null annotations but without method bodies - to be provided by 3rd party contributors (using whatever tools they choose) - to be used during compilation to enable annotation based null analysis - the regular jar without null annotations - to be used for run/debug I'm achieving this split by classpath entries of this shape: <classpathentry kind="lib" path="lib/unannotated.jar" compilepath="lib/annotated.jar"/> With this as the only user visible change my patch actually works so far that - compiling & null analysis use lib/annotated.jar - everything else uses lib/unannotated.jar While it is clear which jars compiler and run/debug should use respectively, the Java Model could theoretically be built on either one. I currently chose to use the regular jar for the Java Model, so what you are browsing is the real thing. I'm posting this as to ask: - @interested parties: would this be a useful addition already? Is anyone willing to provide annotated stub libraries? Would tools like http://types.cs.washington.edu/annotation-file-utilities/ work for now for creating such stub libraries, even if they're not integrated in JDT? Is anyone interested in providing support functions, like validation that a stub library actually has the same signatures as the real library etc.? - @JDT team: do you think this concept could fly without disturbing JDT? Would JDT/UI accept a patch that adds support into the Libraries property page, so that stub libraries could be entered similar to source attachment? Any other thoughts?
Created attachment 235064 [details] Experimental patch The quick-and-dirty patch to sketch my idea. Created on top of BETA_JAVA8
Stephan, I think this is not a good solution. My understand is that both jar files must be synchrony. Else you receive NoSuchMethodErrors at runtime. This can be very difficult and error-prone. This required a different file for every version. The first library that need to annotated is the JDK self. The different versions can be problematic.
(In reply to Volker Berlin from comment #24) > Stephan, I think this is not a good solution. My understand is that both jar > files must be synchrony. Else you receive NoSuchMethodErrors at runtime. > This can be very difficult and error-prone. > > This required a different file for every version. That's why I asked... (In comment #22) > Is anyone interested in providing support functions, like validation that a stub > library actually has the same signatures as the real library etc.? I guess such validation will always be needed when using any form of supplement to carry the null annotations, although I admit that when compiling solely against the stub library the danger with incompatible files is greater than when the bare signatures are still taken from the original jar. I was just wondering, if a solution exists that is even simple enough to be implemented during the Luna cycle, so that users could start contributing annotation stubs for various libraries... I'll wait to see if others want to weigh in, too.
I would welcome your suggested progress very much. I think it's key to make the process of providing, validating, maintaining, etc the stub libraries as simple as possible, to allow many people to contribute. Do you already have any thoughts on what this process and the required infrastructure would look like? E.g., where will the stub libraries be stored (centrally?), how can you submit them, and how can they be retrieved? As mentioned before, versioning is an important aspect, and must be done right. I'm thinking of a maven-style repository containing annotated stub libraries for versioned jars. Anyway, the more accessible this is, the more people will contribute by providing the stub libraries or other other utilities. You'll have my support anyway :)
Sounds good! One thing to note, though. The annotated stub libraries that are currently available are, let's say ... optimized. Meaning they may contain only the subset of the original library's API for which new annotations have been deduced. So to speed up adoption, there should be an automatic fallback to the original library. This shouldn't be too difficult, just making sure the feature works like <classpathentry kind="lib" path="lib/unannotated.jar" compilepath="lib/annotated.jar:lib/unannotated.jar"/> or so. (In reply to comment #26) > Do you already have any thoughts on what this process and the required > infrastructure would look like? E.g., where will the stub libraries be stored > (centrally?), how can you submit them, and how can they be retrieved? > > As mentioned before, versioning is an important aspect, and must be done right. > I'm thinking of a maven-style repository containing annotated stub libraries for > versioned jars. I'd like to see as little infrastructure to start with as possible. So that means stub-libs could be downloaded from github, a random person's website, or - like you mentioned - as maven dependencies from central. Remember, these stub-libs are just a (hopefully) temporary workaround for libraries not providing the right annotations themselves on day 1. Going forward there should be less and less need for additional stubs as the annotations would be included in the original libraries. > Anyway, the more accessible this is, the more people will contribute by > providing the stub libraries or other other utilities. You'll have my support > anyway :) Hear hear!
(In reply to Timo Kinnunen from comment #27) > Remember, > these stub-libs are just a (hopefully) temporary workaround for libraries > not providing the right annotations themselves on day 1. Going forward there > should be less and less need for additional stubs as the annotations would > be included in the original libraries. That would indeed be the optimal situation. One important precondition is to reach consensus on which annotations to use for null analysis throughout the java community -- to my knowledge this is not decided yet. org.eclipse.jdt.annotation.NonNull will not likely be accepted as a community-wide annotation, I think.
That would be great. Nullity checking without support for a library annotation mechanism is painful. Is there an intention to provide some stub libraries with the release, and if so, which nullity annotations would be used? (Probably best to avoid that tar pit, but I still wanted to ask). Or will you simply make the feature available and let the users create the stub libraries? We could probably find a way to generate such a library sub from the "large" collection of JML annotated spec files.
(In reply to Tom van den Berge from comment #28) > One important precondition is to reach consensus on which > annotations to use for null analysis throughout the > java community -- to my knowledge this is not decided yet. I think this would be counter-productive. Consensus will take a long time (especially since there are new changes in Java 8), and the variety of annotations in use during that time will further slow adoption, and even once consensus is “reached” there will still be much existing code using different annotations. Eclipse should accept several synonyms for annotations, at least until such consensus is reached and permeates the ecosystem. I am working on a project right now which depends on libraries that use three different sets of annotations. Allowing synonyms for null annotations would make those useful instantly. Waiting for consensus means at least a couple of years of sprinkling x!=null assertions everywhere.
(In reply to Bogdan Butnaru from comment #30) Bogdan, you make very good points. > Eclipse should accept several synonyms for annotations, ... Agreed. > ... libraries that use three different sets of annotations Also developers creating libraries, will face an equivalent dilemma of choosing the "right" annotation for their user base. In light of these points, supporting annotation synonyms does seem like a feature that will be necessary for at least a transition / stabilization period (that is likely to span a period of a few years).
(In reply to Bogdan Butnaru from comment #30) That is certainly a good idea. I don't have any experience with annotations other that those of Eclipse, but is it safe to assume that other annotations are semantically equivalent to Eclipse's annotations? Also, it looks like IntelliJ already supports configuration of multiple sets of annotations (http://blog.jetbrains.com/idea/2011/03/more-flexible-and-configurable-nullublenotnull-annotations/).
A list of annotations for nullness appears in section 3.7 of the Checker Framework manual: http://types.cs.washington.edu/checker-framework/current/checkers-manual.html#nullness-related-work > is it safe to assume that other annotations are semantically equivalent to Eclipse's annotations? Most are. A notable exception is the annotations of FindBugs (and the long-abandoned JSR 305 proposal, which is essentially the same as FindBugs), which differ from every other tool for nullness analysis. Details appear in section 3.7.2 of the Checker Framework manual: http://types.cs.washington.edu/checker-framework/current/checkers-manual.html#findbugs-nullable
I have been thinking about this. We use nullity annotations a lot, and not having for instance the JDK annotated means we still have lots of places where we have unchecked nulls. == Tools == There are quite some tools that try to detect nullity information from existing code. One that to me looks quite promising is KAnnotator (https://github.com/jetbrains/kannotator), made by the Jetbrains people. It is used by their IDE to detect nullity information for existing libraries. We should somehow be able to use the output from tools like this, perhaps not directly (through conversion to a common format) but having choice seems important here. == External annotations format == The nullity information should be in some file that can be "attached" to any binary node in the IDE. Much like what is done with Javadoc and source -there should be an extra thing like that for external annotations. We can add "autodiscovery" of such a file too if we have some naming convention. In my opinion it is very important to have this file be a text file, not a binary one. It allows easy maintenance and allows the file to be part of version control, with changes to it easily trackable. KAnnotator uses a simple XML format. We can internally "preparse" such a file and use any fast-to-use format internally and just reparse the original source if it changed. This prevents a lengthy "xml parse" per compile. Perhaps that defining a simple and fast-to-parse format in text might even be fast enough to skip the preparse/cache thing completely. Also, the file should not contain actual annotations at all: it should contain nullity information in a format that is independent of any annotation used. We do not have a single standard for these annotations and we will not have that for years. We can use the information in the file to "fake" the actual annotations used in the project that requires the nullity information. I see in the compiler code that those configured annotations are changed very early in the process into "flag" types that no longer depend on the actual annotations. It might be enough to just extend implementations of IBinaryType to read the nullity file data and "add" them to the type's data set using those flag types. By hiding the actual annotation used we completely sidestep the problem that different libraries use different annotations. I do not like the "jars with annotated signatures" proposal much, although it is a clever idea. The jar is binary and as others commented keeping it in sync with the original is hard and error-prone. Also, changes inside it are not trackable by version control. It seems better to keep the "original" jar and just "overlay" the annotations. If anything here sounds reasonable I might be able to help with parts of it, especially if someone can point me to whatever documentation there is on the compiler's internals...
No idea if it's useful, but I started to work on this - if only to learn ;-) Code is at https://github.com/fjalvingh/eclipse.jdt.core
As I do not want to keep "spamming" this bug- progress on this can be followed on: http://domui.org/wiki/bin/view/Main/EclipseAddExternalAnnotations for those interested. Today I added the UI to add an external annotations file to jars from a jdk, like the "source attachments" UI that was already present. I also extended the classpath instances to properly hold and save/load that property. I'll report back here once code is usable. As it's suspiciously silent on this bug, please do let me know if this is useful at all or if it's not - no use wasting time..
(In reply to Frits Jalvingh from comment #36) > As it's suspiciously silent on > this bug, please do let me know if this is useful at all or if it's not - no > use wasting time.. Sorry, for my part the silence is fully explained by the efforts to complete Luna. I appreciate your efforts and will re-join the discussion when Luna has sailed.
(In reply to Frits Jalvingh from comment #36) > As it's suspiciously silent on > this bug, please do let me know if this is useful at all or if it's not - no > use wasting time.. There are 26 CC's for this bug. I suspect that many of us are waiting patiently. Please remember that many of us must maintain a BREE of less than 8 so please ensure that the nullity profiles are available for Java 6/7. (NB Only 2 out of many thousands of Eclipse plugins have a BREE of 8. See http://build.eclipse.org/simrel/luna/reporeports/reports/breedata.txt.)
My excuses, I realize I'm whining for attention above; it was not meant to be that 8-/ Of course I understand everyone's busy... In reply to Ed Willink from comment# 38: Actually, the current proof-of-concept only works for versions before 8 ;-) It was the easiest, and I'm still trying to get my head around the Java 8 changes. But the goal is definitely to support both type and declaration style of annotations. The thing should also be flexible enough to extend later for other kinds of compiler extensions like @NullContract or whatever.
*** Bug 437881 has been marked as a duplicate of this bug. ***
When visiting Zurüch for the Luna Demo Camp I had some minutes with Markus to discuss this feature. He, too, emphasized the importance of having a textual format. When brainstorming possible formats, parsers, standards, licences, we identified a likely candidate: Java signatures as used in class files and as supported by org.eclipse.jdt.core.Signature. In addition to being textual this format has a dual advantage: - it's a standard - we already have a parser and other facilities for its manipulation Currently, even class Signature is not part of the ecj batch compiler, but including this class is easily doable as it does not pull in any further dependencies. OTOH, perhaps SignatureWrapper and methods like LookupEnvironment.getTypeFromTypeSignature() are even more relevant for the task at hand. In order to add null annotations to this format we should indeed use tokens that are independent of any concrete annotation type. Unfortunately, a lot of special characters are already in use for signatures, but it looks like '0' and '1' could be available without any ambiguities, to mean @Nullable and @NonNull respectively. In analogy to other languages '?' and '!' would be preferred choices, but '!' is already in use - perhaps '0' and '1' are even somewhat safe for the future (should Java add more tokens to the signature grammar). To identify a good position in the signature I'd suggest to put the annotation token after the first token of a signature element. Examples: Class: @NonNull String = L1java.lang.String; Type argument: List<@Nullable String> = Ljava.util.List<L0java.lang.String;>; Type variable: @Nullable E = T0E; Type parameter: Cx<@NonNull T> = <1T:>LCx; Array dimensions: int @NonNull[] @Nullable[] = [1[0I This looks promising as to encode null annotations in all locations needed for old and new annotations (Java 5 vs. 8). Alternatively, we could use a postfix notation, where the annotation token would precede the trailing ';', but for Java devs the prefix variant should be easier to handle, I suppose. These token pairs are easy to recognize as prefixes to the type name: L0 : nullable class/interface T0 : nullable type variable [0 : nullable array dimension <0 : nullable type parameter Next question regards the encoding of @NonNullByDefault. We _could_ argue that the format should mainly be processed by tools, which can easily make all annotations explicit, or, we could even say that declaring a default bears to much risk of claiming nonnullness without having this verified (remember that ecj does not validate the bytecode). If we still want to support @NonNullByDefault in external annotations, I would support only the argument-less form. But for starters, let's go without @NonNullByDefault.
(In reply to comment #41) > When visiting Zurüch for the Luna Demo Camp I had some minutes with Markus to > discuss this feature. > > He, too, emphasized the importance of having a textual format. When > brainstorming possible formats, parsers, standards, licences, we identified a > likely candidate: Java signatures as used in class files and as supported by > org.eclipse.jdt.core.Signature. In addition to being textual this format has a > dual advantage: > - it's a standard > - we already have a parser and other facilities for its manipulation > > Currently, even class Signature is not part of the ecj batch compiler, but > including this class is easily doable as it does not pull in any further > dependencies. OTOH, perhaps SignatureWrapper and methods like > LookupEnvironment.getTypeFromTypeSignature() are even more relevant for the task > at hand. > > > In order to add null annotations to this format we should indeed use tokens that > are independent of any concrete annotation type. Unfortunately, a lot of special > characters are already in use for signatures, but it looks like '0' and '1' > could be available without any ambiguities, to mean @Nullable and @NonNull > respectively. > In analogy to other languages '?' and '!' would be preferred choices, but '!' is > already in use - perhaps '0' and '1' are even somewhat safe for the future > (should Java add more tokens to the signature grammar). Oh, please don't. First of all, the Java class name format you are using as a base is pretty much ungoogleable in itself. Then there's the fact that it has several variations that are slightly different and it's far from obvious what's what. And finally the format is in itself little better than a binary protocol around UTF8 text strings with all the control markers just chosen from printable ASCII characters. It's not very user-friendly, is what I'm saying. I also don't see what sort of work-flow one would use to produce this format correctly using the Eclipse IDE. Creating a skeleton of a class, then annotating that is easy. How do you go from there to produce the corresponding encoded version without introducing errors? What is there to prevent someone from using 0 when they meant NotNull or 1 when they meant not Nullable? > To identify a good position in the signature I'd suggest to put the annotation > token after the first token of a signature element. Examples: > Class: > @NonNull String = L1java.lang.String; > Type argument: > List<@Nullable String> = Ljava.util.List<L0java.lang.String;>; > Type variable: > @Nullable E = T0E; > Type parameter: > Cx<@NonNull T> = <1T:>LCx; > Array dimensions: > int @NonNull[] @Nullable[] = [1[0I What's wrong with simply doing: Class: @NonNull String Type argument: List<@Nullable String> Type variable: @Nullable E Type parameter: Cx<@NonNull T> Array dimensions: int @NonNull[] @Nullable[] without specifying the package for the null annotations and accepting a few alternate spellings (NotNull and NonNull, etc.)?
If the text format is meant as something that real people would manipulate then signatures are i.m.o. a very bad idea: they are completely unreadable. I also think it is a bad idea to "take unused chars" in them and (ab)use them for these means: new versions of Java will probably start using more and more of the leftovers, and this could mean that our collected data is now clashing with the JDK's definition. It might be acceptable to "extend" signatures in an internal format used only by the code, but exposing it as an external, long-lived format (because people would create these files and maintain them for a long time) is i.m.o. a bad idea. And finally, I dislike that this is limited to just the nullity annotations. I would like it better to have something that is at least suitable to hold other annotations too, like @NullContract (which would hold real values too) or other things we might think of to extend the compiler's native checks. Having signatures does not support that. Take a look at what Intellij Idea does with @Contract for it's nullity checks - I think it is quite nice to be able to do something like that too. Another thing to consider is how we would obtain these files. I think in most cases they will start as something generated from a tool. Manually annotating existing libraries is just too much work. If Eclipse defines an "Eclipse external annotations format" then existing tools that detect nullity (like KAnnotator) need to have their output "converted" to Eclipse's format. Someone needs to write a conversion tool (probably outside Eclipse) which takes such output and generates Eclipse's format. Advantage is that creating those converters should be easy, and maintaining/writing them can be done by others. The alternative (which I currently use in the code I wrote to support this) is to have different Eclipse "plugins" for each format. Advantage is that this would make support "out of the box" for tools easier without needing an extra (forgettable) "convert output" step - at the cost of maintaining these plugins. For that solution the file format is at least moot for the supported tools: these all have a format of their own. We can even combine the approaches of course: Eclipse's "native format" would just be the default plugin ;-) This would work fine with the code I wrote so far. If Eclipse wants to define a "native" format for external annotations then I think the best way to do it is as method declarations themselves. This makes the format completely clear for anyone maintaining them, and it's not hard to parse it into something else. It should also allow multiple files to be used: having a 40mb text file with all the JDK's annotations is not maintainable. Which means the real format would be something like a jar of these files, in package structure or something like that. IMO the text format should be geared to readability and maintainability, and not be made "technical" to handle performance or how easy it is to write the code for it. Parsing is simple, and we can always create some very fast intermediary format if that is needed for compilation.
Alright, if everybody wants to participate in the design, I should've started with making the goals and non-goals explicit together with some constraints. First: I, too, have been thinking about an extension point for enabling pluggable annotation providers, BUT, I want external annotations be available also in the batch compiler, and that beast cannot use any extension point magic. It's a plain runnable jar. Let's not get carried away and end up with nothing: The batch compiler will use exactly one format for external annotations. Second: What are the reasons for choosing a textual format? Some say: readability, which for me is not a high priority at all. I was convinced for textual formats by considerations about version control, comparison, merging - that sort of things. From this perspective, a beautiful representation is a non-goal. Being textual is a goal, being universal and unambiguous is another goal. Allowing fast parsing is a goal. Easy matching against the real class files is a goal. Third: How shall external annotations be manipulated? It has been mentioned that initial annotations should be obtained from an analysis tool. This is relevant for two reasons: convenience and reliability (derived annotations should be correct-by-definition). Another use case is during inspection of the library's source code. Now the user is looking at that source code and wants to add individual annotations. It should be a goal to hide the external file for this use case and provide gestures like quick assist that perform the work. Forth: How shall external annotations we inspected? Browsing the external file on it's own will never be a pleasant experience, users should be able to see the effect right on the library (source code or outline etc.). Once JDT's text hovers are able to show type annotations that's one place where annotations can be conveniently browsed. Perhaps views like the Outline can render null annotations as decorations (here I'm thinking of '?' vs. '!' indeed). (Who looks at the HTML source of javadoc? :) ). Fifth: What set of annotations should be supported. It is a goal to be open for more null-related annotations. We have @LazyNonNull in the pipeline, but frankly I don't see anything beyond that going into JDT, realistically. Even @LazyNonNull may turn out to be irrelevant for external annotations. Remember that we are talking about non-authors annotating third-party code. I don't see a priority in supporting a vastly expressive annotation language for this task. If a library requires such sophistication it should be properly annotated by its authors, IMHO. ****************************************************************************** At this point I'd particularly ask the JDT team if the above goals / non-goals are consensual? ******************************************************************************
Why reinvent the wheel instead of using existing formats (intellij's and/or jaif) ?
(In reply to Marcin Wisnicki from comment #45) > Why reinvent the wheel instead of using existing formats (intellij's and/or > jaif) ? Whatever format we choose, we need a parser that can be made part of the JDT code base, which requires that it is made available under the EPL and passes the IP due diligence of the Eclipse Foundation. If you can point me to a parser that matches these criteria, for a format that can be considered as a de-facto standard, that could be worth considering. Otherwise I'd say: why implement yet another parser if we already have all that we need, based on a standard actually? From a technical p.o.v. there's a big advantage in using signatures: when reading overloaded methods from a library class file, we need to match annotations to methods. In this situation all we have for discriminating overloaded methods are their signatures. Why should we store annotations in a different format if we need the signatures anyway? As for interchange with annotation files in other formats: I don't see this as a responsibility of the compiler, but as a field where contributed Eclipse plugins may provide functionality for transforming, comparing, merging annotation files in different formats. Here we don't have the strict EPL restriction.
*** Bug 440377 has been marked as a duplicate of this bug. ***
(In reply to Stephan Herrmann from comment #44) Sorry about being a pain 8-/ #1: Yes, it's vital that the batch compiler can do everything that the embedded one can. So if it is only acceptable for it to accept a single format- then that decides that ;-) If we want to make life easier in the IDE we can add stuff to "convert" formats to Eclipse's native one in there. Based on plugins. > Second: What are the reasons for choosing a textual format? For version control /alone/ having a good representation is important /only/ if you want to be able to do proper merging and comparing (understanding what conflicts or what changed). If the format is textual "gibberish" then any comparison will not make much sense. If that is not a goal binary suits fine for vcs'es too. Universal is not a goal. Unambiguous I would say is? Fast parsing is a goal considering the batch compiler issue (but if we consider an "unreadable" format then why not make it binary in an optimized format?) Since we have to parse the format anyway easy matching is done by creating an optimal parse result, not from the file, per se. I do that in the code I wrote: I parse the KAnnotator format which is XML with javaish function prototypes. After parsing the memory representation is Eclipse signatures so that lookup is instantaneous. > Third: How shall external annotations be manipulated? IDE-assisted manipulation of the file through quick fixes is a good plan, it's also how Intellij does it. Tools however will do the bulk of the initial work. This also relates to the format: if the format is solely the result of repeatable conversion then the value of file in "eclipse format" is null: it can always be derived. But after manipulation it becomes valuable meaning it needs to be "future proof": the next Eclipse should not require a new format. So for me being "future proof" would be a requirement too. And this "future proof" requirement for me means I do not like "modified" Signatures: when Oracle releases Java 9 some 6 years from now our signatures might clash with theirs - causing trouble in all those files. Why take that risk? We could use signatures if we do not *extend* them. So we would use a signature to /match/ a method but something outside it to specify the annotations applied to it? > Forth: How shall external annotations we inspected? Would be nice. But even without this the solution would work quite well, as a save would immediately tell you that you sinned ;-) > Fifth: What set of annotations should be supported. If one knows for sure that nothing will ever be added then yes, making it extensible would be useless. But can we really say that? Even though we might now think "nothing new will happen here"? Idea already has that @Contract annotation to specify things like "the return value is null if the 1st parameter is null". Is it too far-fetched that Eclipse could ever have that? And yes- it can be added by a non-creator of code too ;-) And we do not *need* a "vastly expressive annotation language" at all. Just one that is extensible - meaning we have a format that can be easily extended, not a format that is capable at the start to express "all".
What created all this angst about a "gibberish" format? I find jaif painful to read, and I strongly dislike any XML. If we were to vote on this issue I'm sure there's only one syntax that everybody reading this would tolerate: Java source stubs. We do have a parser for that :), but it produces the wrong output: AST, which is of little use for the purpose at hand. BTW, does IntelliJ's XML format support type annotations? BTW2: at a second look jaif seems to be over the top for our purpose, because that format is geared at capturing all locations that can be annotated using JSR 308, including method bodies. Method bodies are not on the agenda in this bug.
(In reply to Stephan Herrmann from comment #49) > What created all this angst about a "gibberish" format? Hmmmm, my /interest/ in creating a readable and future-proof format is caused by me working on this thing, and using the existing @Nullable support for Eclipse as much as possible. The readable is important /if/ the format is maintained by users, because then it should be possible to see what changes were made and what they mean, for instance during review. Readable is not important if the file is only a product of conversion from some other format and after that immutable. I do not like jaif either and see it as way too complex, and I share your dislike of XML. As far as readability and expandability is concerned I think Java stubs are the best, and they are quite future-proof too ;) Since we only have to parse a tiny bit of Java's grammar - and since we do not need a "compile grade" parser I think handwriting a tiny and fast parser should not be hard - actually the example code I wrote needs to parse Intellij's format (see below) so it already contains something very basic. > BTW, does IntelliJ's XML format support type annotations? Not really. The format looks like: <item name='java.util.AbstractCollection T[] toArray(T[]) 0'> <annotation name='org.jetbrains.annotations.NotNull'/> </item> <item name='java.util.AbstractCollection T[] toArray(T[])'> <annotation name='org.jetbrains.annotations.NotNull'/> <annotation name='org.jetbrains.kannotator.runtime.annotations.Propagated'> <val name="value" val="{org.jetbrains.kannotator.controlFlow.builder.analysis.NullabilityKey.class}"/> </annotation> </item> The format is xml and also requires parsing of the java fragment. While this is text it's not that readable.
To get things going I started a new feature branch at http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/log/?h=sherrmann/ExternalAnnotations Current focus is on bug 440477
(In reply to comment #49) > I find jaif painful to read, and I strongly dislike any XML. If we were to vote > on this issue I'm sure there's only one syntax that everybody reading this would > tolerate: Java source stubs. This is an example stub from an old project of mine from a couple of years ago, file Vector3f.stub: import checkers.nullness.quals.Nullable; package org.lwjgl.util.vector; class Vector3f { public Vector3f normalise(@Nullable Vector3f dest) {} } Apart from the order of import and package statements it could be compilable Java source code. There's the equivalent of an implicit @NonNullByDefault in effect, so only the methods not matching that need external annotations, in my case just the method normalise. This is it for the LWJGL library, at least the parts that my code used. I don't have any desire to annotate the whole LWJGL library but I'll gladly take the two minutes it takes to decide and write the stub class when needed rather than adding a bunch of @SuppressWarnings annotations in my code. So that's where I'm coming from. > We do have a parser for that :), but it produces the wrong output: AST, which is > of little use for the purpose at hand. Going from an AST to method signatures shouldn't be difficult, though.
I just created bug 440629 for some performance experiments. Contributions in that area would be highly appreciated!
I wrote a bare-bones fast parser for java format signatures. An example (generated from that KAnnotator dataset) would be: package java.util { class SimpleTimeZone { @Nonnull java.lang.Object clone(); boolean equals(@Nonnull java.lang.Object); boolean inDaylightTime(@Nonnull java.util.Date); @Nonnull java.lang.String toString(); } class List<E> { void add(int,@Nonnull E); boolean add(@Nonnull E); boolean addAll(@Nonnull java.util.Collection<? extends E>); boolean addAll(int,@Nonnull java.util.Collection<? extends E>); boolean contains(@Nonnull java.lang.Object); boolean containsAll(@Nonnull java.util.Collection<?>); boolean equals(@Nonnull java.lang.Object); int indexOf(@Nonnull java.lang.Object); @Nonnull java.util.Iterator<E> iterator(); int lastIndexOf(@Nonnull java.lang.Object); @Nonnull java.util.ListIterator<E> listIterator(); @Nonnull java.util.ListIterator<E> listIterator(int); boolean remove(@Nonnull java.lang.Object); boolean removeAll(@Nonnull java.util.Collection<?>); E set(int,@Nonnull E); @Nonnull java.util.List<E> subList(int,int); @Nonnull java.lang.Object[] toArray(); @Nonnull T[] toArray(@Nonnull T[]); } } Using the same KAnnotator dataset we played with earlier generates a single file of 3.3MB in this format for the entire JDK. The parser at least parses that file quite fast: in 50..60 milliseconds on my PC. I do not expect this to get much faster: the lexer and parser have already been coded for best performance. Now of course this is just parsing: it runs through a hand-written recursive-descend parser which just checks syntax for now, but at least lexing, tokenizing and parsing does not seem too slow to use full prototype signatures. Next step would be to try to get the parser to generate signatures in the format you require inside the annotation walker. I have no idea how that will impact performance, and it will take some time because those signatures give me a bit of a headache still ;-)
(In reply to Frits Jalvingh from comment #54) answered in bug 440474 comment 20
********************** * State of the Union * ********************** :) I have summarized what we have in this bug and its friends: https://wiki.eclipse.org/JDT_Core/Null_Analysis/External_Annotations I would like to move the essential parts from my feature branch into master during the next 2-3 weeks. What's needed for this to happen? - Consent from JDT leaders - In particular, agreement about added API - Hopefully the UI contribution in bug 440815 (relevance for a CQ is being checked) - Filling a few gaps outlined in the wiki page - If anyone advocates a file format other than my proposal, we'd need a proof-of-concept implementation to demonstrate feasibility in terms of implementation complexity and specifically in terms of performance. - Anything else? Another pending discussion, which I wouldn't consider essential for initial release: - Should we support more packagings than "directory tree" and "zip"? (see https://wiki.eclipse.org/JDT_Core/Null_Analysis/External_Annotations#packaging ) Either of "big file" or "combined" can easily be added later, (perhaps well in time for Mars, still). More functionality that would be good to have in Mars: - Basic support for creating external annotations via quick fix / assist. See bug 458186 and bug 458200. - Visualization of (type) annotations in hovers Beyond the essentials there are several nice-to-haves that have been proposed in the bugs. Given the limited capability of the JDT team to maintain new features for the years to come, I've been thinking about minimal API in JDT that would allow external plug-ins to provide such desirable functionality. You'll find more on this in the wiki page under these headings: - Additional operations - Facilitate crowd-sourcing I'll spell out the API proposed so far in a subsequent comment. To give a home to a new plug-in for advanced support for external annotations, we may think of creating a new project, here at Eclipse.org or informally on github or such. IF we opt for an Eclipse project, which I believe could start as an incubator either in the vicinity of JDT or under "tools", who of the valued subscribers of this bug would like to participate? As hinted by the paragraphs in the wiki, I see several desirable areas of engagement for such a project: - more complete/convenient support for working with external annotations - technology, process etc. towards a community repository of external annotations. - integration with build tools like Maven & m2e I'll certainly be available to help, but a new project would definitely need more than one person :) By drawing a line between essentials and nice-to-have, I'm trynig to keep the bar low for JDT to be able to accept this new feature as a significant item for its Mars New&Noteworthy.
New API proposed in this cluster of bugs consists of three elements: 1.: One new constant in IClasspathAttribute, see http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/tree/org.eclipse.jdt.core/model/org/eclipse/jdt/core/IClasspathAttribute.java?h=sherrmann/ExternalAnnotations#n117 The xml representation of the same in .classpath should perhaps be considered as API, too, if it's OK for other plug-ins to edit .classpath (I'm not sure, what's the policy here). I was wondering whether the different options for specifying a path should be documented (cf. https://wiki.eclipse.org/JDT_Core/Null_Analysis/External_Annotations#shapes ) but I couldn't find such documentation on similar constants. Should I still add it for the new constant? 2.: A new utility class, whose design was driven by work on quick fixes (bug 458186): http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/tree/org.eclipse.jdt.core/model/org/eclipse/jdt/core/util/ExternalAnnotationUtil.java?h=sherrmann/ExternalAnnotations 3.: The file format as described in https://wiki.eclipse.org/JDT_Core/Null_Analysis/External_Annotations#File_format For the sake of bug 458201 I will probably propose one more API, by which an ICompilationUnit can be obtained from an IClassFile, even if there's no underlying IResource. From experiments this might be as easy as providing external access to the constructor of ClassFileWorkingCopy.
(In reply to Stephan Herrmann from comment #57) > For the sake of bug 458201 I will probably propose one more API, by which an > ICompilationUnit can be obtained from an IClassFile, even if there's no > underlying IResource. From experiments this might be as easy as providing > external access to the constructor of ClassFileWorkingCopy. Withdrawn: classFile.getWorkingCopy((WorkingCopyOwner)null, null) should be good enough, we just need to fix a few bugs resulting from it (bug 458577).
Created attachment 250415 [details] example project Here's a tiny example project facilitating to play with this feature. ==Preparing a runtime workbench== * The following assumes a 1.8 JRE (although 1.7- is supported, too, 1.8 just provides more options). * Core functionality can be tried from JDT/Core branch sherrmann/ExternalAnnotations * Minimal quick fixes can be tried using JDT/UI with patch from bug 458186 * Experimental quick assists additionally require the patches from bug 458201 and bug 458200 (in that order). ==Attaching an existing annotation zip== In the runtime workbench import the attached project => you should see a bunch of null-related errors (illegal redefinitions). Open the source file in an editor to see details. Edit .classpath changing this line (currently pointing to nowhere): <attribute name="annotationpath" value="annot"/> to this (using an existing zip file): <attribute name="annotationpath" value="annot.zip"/> => all but one errors should go away (editor should react immediately, Problems view needs an explicit rebuild). Place editors for MyMap.java and .classpath side by side, then remove / re-add the annotationpath attribute, while watching the java editor. => compiler errors are toggled. ==Using quick fix to incrementally add external annotations== Change .classpath back to "annot" to prepare for using annotation files in an open directory structure (directory doesn't need to exist yet). If you have JDT/UI patched for quick fixes: On one of the errors select a quick fix, variant mentioning "(using external annotation)". => file annot/java/util/Map.eea is created, selected error disappears. Open the new file, move it to a side so you can see .java and .eea editors side by side. Invoke more quick fixes. => observe .eea filling with more entries, while errors disappear. => in case of multiple errors on the same method, .eea entries are incrementally filled with details. (sorry: the offered multi-fix is not operational, we probably don't want this anyway). ==Expermental quick assists directly on a library class== If you have JDT/UI patched for experimental quick assist: Open interface Map in a third editor (ideally showing three files simultaneously now). On any non-primitive method return type hit Ctrl-1 to explicitly toggle to @NonNull or @Nullable as desired. Third option removes an existing annotation. => watch changes occurring in Map.eea and effects in MyMap.java (when using the mouse, focus may have to be set explicitly; when using the keyboard, reaction is even more immediate). Find a method with parameterized return type, select type argument or even type bounds to invoke quick assist for this given detail (Collectors.java has great generics :) ): => watch external annotations appearing on any relevant type detail you choose. This experimental implementation demonstrates quick assists only on return types. Quick assists for parameters to be added if the general approach is approved.
(In reply to Stephan Herrmann from comment #57) > New API proposed in this cluster of bugs consists of three elements: > > > 1.: > One new constant in IClasspathAttribute, see > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/tree/org.eclipse.jdt.core/ > model/org/eclipse/jdt/core/IClasspathAttribute.java?h=sherrmann/ > ExternalAnnotations#n117 .... Looks good to me. > I was wondering whether the different options for specifying a path should > be documented (cf. This is pretty much like other path attributes associated with classpath, which are documented to some extend in IPath. I think that should suffice. > 2.: > A new utility class, whose design was driven by work on quick fixes (bug > 458186): I haven't looked at how this is getting used, but assuming this comes into picture only when external annotations are present and analysis is enabled, looks alright to me. I have not looked at the file format yet. I have one general question though. With Java 9 (read Jigsaw) around the corner aiming to replace classpath based dependencies, have you thought about how this can be reused in future? Just wondering.
Thanks, Jay, for commenting (In reply to Jay Arthanareeswaran from comment #60) > > 2.: > > A new utility class, whose design was driven by work on quick fixes (bug > > 458186): > > I haven't looked at how this is getting used, but assuming this comes into > picture only when external annotations are present and analysis is enabled, > looks alright to me. Your right. JDT/Core only uses the constants NULLABLE & NONNULL, the rest is used by proposed quick fixes/assists that are only offered in projects configured for annotation based null analysis. Minor correction: the API also supports *creating* external annotations in the first place. Other plugins interested in null annotations may benefit from this API, too. > I have not looked at the file format yet. The discussion in the other bug may be a bit convoluted, I hope the description in the wiki is clear. > I have one general question though. With Java 9 (read Jigsaw) around the > corner aiming to replace classpath based dependencies, have you thought > about how this can be reused in future? Just wondering. Very good question, I haven't thought about this, yet. Do we have a bug for questions like whether JDT's classpath containers can be made to model a Jigsaw module (akin to RequiredPluginsClasspathContainer)? Reminds me that I should probably think of a solution for OSGi, before jumping at Jigsaw. I've made some experiments how PDE could support external annotations. I'll report about those in bug 440477 in a minute.
As of today I dare calling this thing feature complete: UI has been contributed that supports configuring a path to external annotations for configured JREs (per workspace) as well as for regular libraries (per project). When browsing library source code using the class file editor (the one that shows sources attached to a library - readonly), a new command "Annotate" is available (via context menu and conveniently bound to Ctrl-1), that offers to attach a @NonNull or @Nullable annotation to any type in a signature that you are pointing to (granularity depending on whether or not you're using Java 8). The compiler will immediately respect this additional information for its analysis. I hope that by M7 we will also be able to blend the external annotations into javadoc hovers and javadoc view (prototype exists). Advanced support features will be tracked via their own bugs, see also the wiki page [1], which I'll continue to update every now and then. At this point let me thank, in particular Frits for contributing the configuration UI, and also all others whose comments have helped fledging out the approach. Now, it's your turn: PLEASE start testing the new functionality. Please file bugs so we have a good chance to iron out any obvious quirks before M7. To start testing please pick up any build later than now :), or, if you're not into using I-builds, please fetch M6 as soon as it is available (SDK end of this week, Eclipse packages the week after). [1] https://wiki.eclipse.org/JDT_Core/Null_Analysis/External_Annotations
Verified for 4.5 M6 with build I20150317-2000
(In reply to Stephan Herrmann from comment #62) > Now, it's your turn: PLEASE start testing the new functionality. Please file > bugs so we have a good chance to iron out any obvious quirks before M7. > > To start testing please pick up any build later than now :), or, if you're > not into using I-builds, please fetch M6 as soon as it is available (SDK end > of this week, Eclipse packages the week after). I've downloaded the M6 SDK (for OSX), and I can see the Annotate command in the file viewer. However, nothing happens when I click on it (or press Cmd-1), except a "boink" sound can be heard that normally is played when you do something illegal. I've tried placing my cursor at all parts of the signature, but it made no difference.
(In reply to Tom van den Berge from comment #64) > (In reply to Stephan Herrmann from comment #62) > > > Now, it's your turn: PLEASE start testing the new functionality. Please file > > bugs so we have a good chance to iron out any obvious quirks before M7. > > > > To start testing please pick up any build later than now :), or, if you're > > not into using I-builds, please fetch M6 as soon as it is available (SDK end > > of this week, Eclipse packages the week after). > > I've downloaded the M6 SDK (for OSX), and I can see the Annotate command in > the file viewer. However, nothing happens when I click on it (or press > Cmd-1), except a "boink" sound can be heard that normally is played when you > do something illegal. I've tried placing my cursor at all parts of the > signature, but it made no difference. Thanks for trying, meanwhile we have at least a N&N entry for this feature [1] There I mention: " Two pre-requisites must be met for using this command: * The project is configured to use annotation-based null analysis. * An existing workspace folder has been specified as the external annotation location for a given library." Can you please checks if both conditions were met when you tried? [1] https://www.eclipse.org/eclipse/news/4.5/M6/#annotate-command
(In reply to Stephan Herrmann from comment #65) > " Two pre-requisites must be met for using this command: > > * The project is configured to use annotation-based null analysis. > * An existing workspace folder has been specified as the external > annotation location for a given library." > > Can you please checks if both conditions were met when you tried? The 2nd one wasn't! However, after specifying an existing directory in the project for rt.jar (like in the example), still nothing happens when selecting "Annotate" :-( Should a file be present in the directory? It's an empty dir now.
(In reply to Tom van den Berge from comment #66) > (In reply to Stephan Herrmann from comment #65) > > > " Two pre-requisites must be met for using this command: > > > > * The project is configured to use annotation-based null analysis. > > * An existing workspace folder has been specified as the external > > annotation location for a given library." > > > > Can you please checks if both conditions were met when you tried? > > The 2nd one wasn't! However, after specifying an existing directory in the > project for rt.jar (like in the example), still nothing happens when > selecting "Annotate" :-( Have you specified the location via the JRE Definition? When you re-check the configuration, be noted that "Cancel" in the detail dialog accidentally deletes the value (bug was detected too late for M6). "OK" works. You're sure the file you are looking at is from this exact rt.jar? Can you see the annotation path also via the Build Path > Libraries properties of your project. Any exception logged? > Should a file be present in the directory? It's an empty dir now. Empty dir is fine.
(In reply to Stephan Herrmann from comment #67) It's working now; I didn't change anything. I was trying to add an annotation to String.toString(), but I guess my cursor was not at the return type of the method. It looks very good. I've been looking forward to external annotations, and I will continue testing it. Thanks!
(In reply to Tom van den Berge from comment #68) > (In reply to Stephan Herrmann from comment #67) > > It's working now; I didn't change anything. I was trying to add an > annotation to String.toString(), but I guess my cursor was not at the return > type of the method. Thanks for confirming. What's left to do: - We need a good help page. - If any of your initial problems are repeatable, it needs fixing.
When I configure a directory as an "external location", instead of a "workspace location", the "annotate" command does not work anymore (same problem as in #64).
(In reply to Tom van den Berge from comment #70) > When I configure a directory as an "external location", instead of a > "workspace location", the "annotate" command does not work anymore (same > problem as in #64). This is by design. The Annotate command can only modify workspace resources. If we'd support modifying external resources it would be much more difficult to keep in sync. External locations are mainly intended for consuming pre-built external annotations, i.e., the "normal" format would be external zip/jar, not external directory. We might want to issue a warning when a user configures an external directory for external annotations. We could say s.t. like: "The Annotate command will not be available for this locations, please consider using a workspace location instead". Would that make sense / help?
(In reply to Stephan Herrmann from comment #71) > This is by design. The Annotate command can only modify workspace resources. > If we'd support modifying external resources it would be much more difficult > to keep in sync. External locations are mainly intended for consuming > pre-built external annotations, i.e., the "normal" format would be external > zip/jar, not external directory. Ok, that makes sense. > We might want to issue a warning when a user configures an external > directory for external annotations. We could say s.t. like: > "The Annotate command will not be available for this locations, > please consider using a workspace location instead". > Would that make sense / help? The reason I was trying to use an external location is that I have many different projects in my workspace, and many share the same libraries. Obviously I want to share the annotation files with all of them. Wouldn't it be possible to somehow configure a directory for the entire workspace, e.g. in "/annotations"? I tried creating a directory in the root of the workspace, but it is not seen by eclipse, probably because it's not a project. Or would a directory at that location give the same problems you mentioned?
(In reply to Tom van den Berge from comment #72) > Wouldn't it be possible to somehow configure a directory for the entire > workspace, e.g. in "/annotations"? I tried creating a directory in the root > of the workspace, but it is not seen by eclipse, probably because it's not a > project. Or would a directory at that location give the same problems you > mentioned? Top-level containers in a workspace are projects. There's no way to shortcut this. I tried if a linked folder could help, but that currently causes bug 462768. I'll check if that can be made to work. So please for now put all your external annotations into a dedicated project if you want to share them among several projects.
When configuring a workspace location (directory) for a Maven dependency (Java Build Path -> Libraries -> Maven Dependencies), it does not seem to be stored. When the dialog is reopened, the configured location is gone. Is this bug ok for this issue, or would you like me to file a new bug?
(In reply to Tom van den Berge from comment #74) > When configuring a workspace location (directory) for a Maven dependency > (Java Build Path -> Libraries -> Maven Dependencies), it does not seem to be > stored. When the dialog is reopened, the configured location is gone. > > Is this bug ok for this issue, or would you like me to file a new bug? That's due to the nature of the "Maven Dependencies" classpath container: the list of jars provided via this container is computed at runtime, and hence JDT has no location where we can store annotation paths per jar. Of course you should be warned about this beforehand ... One train of thought to improve this has been started in bug 459753. For now this leaves us with only one annotation location for the entire container. The UI currently doesn't offer to create this (see bug 462412 2nd paragraph), but as a workaround you can manually update your .classpath: <classpathentry kind="con" path="org.eclipse.m2e.MAVEN2_CLASSPATH_CONTAINER"> <attributes> <attribute name="annotationpath" value="/MyPrj/annotations"/> </attributes> </classpathentry>
(In reply to Stephan Herrmann from comment #62) > [1] https://wiki.eclipse.org/JDT_Core/Null_Analysis/External_Annotations Finally popped my stack to look at this. It looks really encouraging, especially Java 7 support too, but I'm defeated at the first hurdle. How do I add an annotationPath? (Is there a danger of confusing between null-annotations and generic annotation capabilities?) It would be great to provide a zipped example project that demonstrates how to add my favourite missing @NonNull. StringBuilder.toString().
(In reply to Ed Willink from comment #76) > It looks really encouraging, especially Java 7 support too, but I'm defeated > at the first hurdle. How do I add an annotationPath? The 'new & noteworthy' page for M6 has a section on configuring and using external annotations: https://www.eclipse.org/eclipse/news/4.5/M6/#JDT
(In reply to Tom van den Berge from comment #77) > > It looks really encouraging, especially Java 7 support too, but I'm defeated > > at the first hurdle. How do I add an annotationPath? Ah!. My guess was completely wrong. I thought this was some kind of ".classpath" file extension. Providing it as a workspace-wide facility is a problem. If I have extra annotations for my bundle, and also load your bundle, then the analysis of your bundle is changed. This seems like a bad idea. Surely annotationPath should be a per-bundle configuration? And absolutely it must be under GIT management.
(In reply to Ed Willink from comment #78) > Providing it as a workspace-wide facility is a problem. The external annotations for the JDK are configured workspace-wide, but for the libraries used by your projects, you configure them per library per project. > If I have extra annotations for my bundle, and also load your bundle, then > the analysis of your bundle is changed. This seems like a bad idea. Surely > annotationPath should be a per-bundle configuration? I'm not sure what you mean with 'bundle'? The idea is to have one place (a directory, or zip/jar) where all external annotations for a specific jar go. You can obtain an external annotations and use it, or start composing you own, e.g. for the JDK's StringBuilder.toString(). The compiler will simply use the available external annotations. > And absolutely it must be under GIT management. You would have to store your eclipse project files in git to do that. For the JDK external annotations I wouldn't know how to do that.
(In reply to Ed Willink from comment #78) > (In reply to Tom van den Berge from comment #77) > > > It looks really encouraging, especially Java 7 support too, but I'm defeated > > > at the first hurdle. How do I add an annotationPath? > > Ah!. My guess was completely wrong. I thought this was some kind of > ".classpath" file extension. You can actually do all this by editing .classpath. But of course the UI is the preferred approach. > Providing it as a workspace-wide facility is a problem. As Tom mentioned this is mainly targeted at attaching annotations to a JRE. To configure per-project annotationPath for any container like requiredPlugins is tracked via bug 462412. > If I have extra annotations for my bundle, and also load your bundle, then > the analysis of your bundle is changed. This seems like a bad idea. Surely > annotationPath should be a per-bundle configuration? You want bug 459822. > And absolutely it must be under GIT management. Everything you do per-project will be stored in .classpath and thus can be subjected to Git. Just one caveat: when you edit details of any individual jar in a JRE container, these changes will write-through to the workspace preference.
(In reply to Stephan Herrmann from comment #80) > To configure per-project annotationPath for any container like > requiredPlugins is tracked via bug 462412. Update: this part has been resolved via bug 465293 (while bug 462412 remains open for other options like using a classpath variable).
FYI: I've written a first version of the user documentation for this feature, visible at - http://git.eclipse.org/c/platform/eclipse.platform.common.git/plain/bundles/org.eclipse.jdt.doc.user/tasks/task-using_external_null_annotations.htm or even better: - by downloading RC3 from http://download.eclipse.org/eclipse/downloads/drops4/I20150527-2010/ (or via update) and navigating to the new help page Java development user guide - Tasks - Improving Java code quality - Using external null annotations I'd appreciate if folks proof-read, and report into bug 461300 anything that looks bogus or unclear. TIA.
(In reply to Stephan Herrmann from comment #82) > I'd appreciate if folks proof-read, and report into bug 461300 anything that > looks bogus or unclear. > TIA. Hi Stephan. Sarika will make a pass over it.
Hi Stephan, Document looks great, You may want to go through these sentences once : 1. If inheritance of null annotations is not enabled one particular situation is safe from a type theory point of view but may still indicate a problem. (Something is missing depending on what you intend to say) 2. Therefore, it is again strongly recommended to leave this problems configured as errors and not to ignore these messages. (this -these ?)
Thanks to Aaron and Sarika for reviewing the docs. Details are in bug 461300.
*** Bug 470953 has been marked as a duplicate of this bug. ***
*** Bug 481919 has been marked as a duplicate of this bug. ***
*** Bug 492591 has been marked as a duplicate of this bug. ***
