Read annotations directly from the .class files#62
Conversation
This commit brings the ByteCodeAnalyzer from imagej-updater into SciJava common. The idea is not so much to move it here, but to modify it for use with the annotation indexer. To this end, the ByteCodeAnalyzer's fields, methods and inner classes were all restricted as much as possible, and the class itself was made package-local, to make clear that this class is not meant to be part of the official API of SciJava common. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The result is a map suitable for use by the annotation indexer. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Instead of loading the classes, we read the annotations directly from the .class files. This has two advantages over the previous method: 1. We avoid polluting the ClassLoader with loaded classes (which can be problematic e.g. when ij1-patcher wants to patch the classes before loading them at a later stage). 2. It is no longer necessary for the dependencies of the classes to be on the class path (though that is only true for the classes, not for the annotations). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
These lines were identified by Eclipse as unneeded. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
As far as the annotation indexer is concerned, we need not parse interfaces, fields nor methods. So let's skip that part and cut to the chase. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is a subtle issue: if an annotation for a given class does not specify a particular value, the developer obviously intended the default value to be used. Now, if the default value ever changes, it should be reflected in the values returned by the annotation index. That implies that we must not write out unspecified settings into the serialized annotations, but always infer them at runtime from the annotation class itself. Happily, our annotation processor does The Right Thing (but the EclipseHelper in its current form does not). Keep it that way. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The switch to reading annotations directly from .class files in the EclipseHelper fixed a subtle bug: default values for annotation parameters must not be serialized into the annotation index, but instead be inferred at runtime (in case the default values in the annotation class itself changed). Keep this bug fixed by testing for it in the unit tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
So after all, it looks as if this branch is not only a "nice to have" but actually fixes a subtle bug in the EclipseHelper: it used to serialize annotation values including the default values (when we want to make sure that changes of the default values in the annotation class are picked up unless overridden). @ctrueden could you give it a good shake (i.e. install it locally and use it to test your favorite SciJava common-based project)? |
|
I tried a brief test in Eclipse:
All seems good to me. Anything else you want me to check? |
|
Sorry, I ran out of time and did not do a good job of describing things... :-( The changes affect only the The test I had in mind (and that I will perform before merging) is to move all the Will keep you posted! |
|
Okay, I did not try this with all of ImageJ, but I tested it with I verified this by
The end result is that the annotation index written out by the annotation processor and the one written out by the EclipseHelper are identical (module serialization order, of course). There is only one more thing I'd like to fix: the |
We recently switched to a more thorough indexing when the EclipseHelper is called as a main class, including inspecting the Class-Path entry of the manifest of .jar files. However, said entries might not refer to existing files, in which case we need to ignore them. While at it, this fixes the bug that .jar files on the class path would not have been accessed properly if their paths contained spaces. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Read annotations directly from the .class files
|
Now I am happy. |
2 participants
This fixes #60.