ClassLoader usage in AnnotationCreator::createAnnotation

[alesj]

While fixing JBMDR-45, I saw this usage (MDR / AnnotationCreator):

 public static Object createAnnotation(ASTAnnotation node, Class<?> annotation, ClassLoader cl) throws Exception
 {
 HashMap<String, Object> map = new HashMap<String, Object>();
 ClassLoader loader = (cl != null) ? cl : Thread.currentThread().getContextClassLoader();
 if (annotation == null)
 {
 annotation = loader.loadClass(node.getIdentifier());
 }

My question is, why don't we rather take loader from annotation class if it's not null, then use TCCL?

[kabirkhan]

Good point, let's go with that

[alesj]

"alesj" wrote:

My question is, why don't we rather take loader from annotation class if it's not null, then use TCCL?

 ClassLoader loader;
 if (cl != null)
 {
 loader = cl;
 }
 else if (annotation != null)
 {
 loader = annotation.getClassLoader();
 }
 else
 {
 loader = Thread.currentThread().getContextClassLoader();
 }

[kabirkhan]

I'd rather do

 ClassLoader loader;
 if (cl != null)
 {
 loader = cl;
 }
 else if (annotation != null)
 {
 loader = annotation.getClassLoader();
 }
 if (loader == null)
 {
 loader = Thread.currentThread().getContextClassLoader();
 }

Since if the annotation class is loaded by the bootstrap classloader, annotation.getClassLoader() might be null.

[adrian.brock]

I'd rather throw an error if there is no classloader provided.
Using the annotation's classloader fails if you have something like:

@JMX(interface=some.thing.from.an.Application.class)

[alesj]

I'll leave it as it was and add a WARN log.
I don't wanna break all sorts of things just before we go 2.0.0.GA.
If it eventually proves that we don't get any warnings,
I'll change it for 2.0.0.GA - throwing an exception.
Sounds reasonable?