hide some ClassFile API #330
Conversation
|
Probably best to review commits in isolation, even though the second one is quite small. |
| this.number = number; | ||
| } | ||
|
|
||
| public int number() { |
There was a problem hiding this comment.
| public int number() { | |
| /** | |
| * {@return the Java version as an integer} | |
| */ | |
| public int number() { |
There was a problem hiding this comment.
Good point, will do.
There was a problem hiding this comment.
Done, removed the method.
|
|
||
| public void withVersion(final ClassVersion version) { | ||
| checkNotNullParam("version", version); | ||
| this.version = ClassFileFormatVersion.valueOf("RELEASE_" + version.number()); |
There was a problem hiding this comment.
This is a little grisly. :-)
Maybe it would be slightly cleaner to use an enum switch expression?
There was a problem hiding this comment.
That would actually allow removing the number() method from ClassVersion, which makes a lot of sense. Will do.
| ClassLoader cl = Thread.currentThread().getContextClassLoader(); | ||
| if (cl == null) { | ||
| cl = AnnotationCreator.class.getClassLoader(); | ||
| } | ||
| clazz = (Class<S>) Class.forName(annotationClass, false, cl); |
There was a problem hiding this comment.
I think we should consider requiring that a Class<S> be passed in instead of trying to do class loader magic. TCCL is almost certainly the wrong one to use at any rate.
There was a problem hiding this comment.
We can do that, I was just thinking that the variant that accepts the annotation element as a method reference is supposed to be most user friendly and hence should figure out the nested annotation class on its own.
There was a problem hiding this comment.
Ideally yes, but in practice I can't think of a totally safe/totally correct way to do it automatically. The lambda instance does not seem to have a concrete generic type, so we can't probe it that way. AFAICT we can't know for sure what class loader originated the type of the nested annotation.
There was a problem hiding this comment.
Actually, I'm being silly. We can examine the method on the enclosing annotation type.
There was a problem hiding this comment.
Ha, good point! I'll try.
The methods to add nested annotation[s] no longer attempt to load the class of the nested annotation from a classloader. Instead, they take it from the return type of the annotation element. Additionally, the annotation creation API now allows creating annotations based on a `ClassDesc` description of the annotation type, not only based on the `Class<? extends Annotation>` description.
This is our public API for specifying the desired class file version. The `ClassFileFormatVersion` enum from the ClassFile API no longer leaks from the Gizmo public API.
7638669 to
2417a87
Compare
ClassVersionenumFixes #331
Fixes #332