Add AnnotationValidator, and validation checks for Category (fix for issue #93)

src/main/java/org/junit/experimental/categories/Category.java

@@ -4,6 +4,8 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
+import org.junit.validator.ValidateWith;
+
 /**
  * Marks a test class or test method as belonging to one or more categories of tests.
  * The value is an array of arbitrary classes.
@@ -40,6 +42,7 @@
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Inherited
+@ValidateWith(CategoryValidator.class)
 public @interface Category {
     Class<?>[] value();
 }

src/main/java/org/junit/experimental/categories/CategoryValidator.java

@@ -0,0 +1,61 @@
+package org.junit.experimental.categories;
+
+import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.unmodifiableSet;
+import static java.util.Arrays.asList;
+
+import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.runners.model.FrameworkMethod;
+import org.junit.validator.AnnotationValidator;
+
+/**
+ * Validates that there are no errors in the use of the {@code Category}
+ * annotation. If there is, a {@code Throwable} object will be added to the list
+ * of errors.
+ *
+ * @since 4.12
+ */
+public final class CategoryValidator extends AnnotationValidator {
+
+    private static final Set<Class<? extends Annotation>> INCOMPATIBLE_ANNOTATIONS = unmodifiableSet(new HashSet<Class<? extends Annotation>>(
+            asList(BeforeClass.class, AfterClass.class, Before.class, After.class)));
+
+    /**
+     * Adds to {@code errors} a throwable for each problem detected. Looks for
+     * {@code BeforeClass}, {@code AfterClass}, {@code Before} and {@code After}
+     * annotations.
+     *
+     * @param method the method that is being validated
+     * @return A list of exceptions detected
+     *
+     * @since 4.12
+     */
+    @Override
+    public List<Exception> validateAnnotatedMethod(FrameworkMethod method) {
+        List<Exception> errors = new ArrayList<Exception>();
+        Annotation[] annotations = method.getAnnotations();
+        for (Annotation annotation : annotations) {
+            for (Class clazz : INCOMPATIBLE_ANNOTATIONS) {
+                if (annotation.annotationType().isAssignableFrom(clazz)) {
+                    addErrorMessage(errors, clazz);
+                }
+            }
+        }
+        return unmodifiableList(errors);
+    }
+
+    private void addErrorMessage(List<Exception> errors, Class clazz) {
+        String message = String.format("@%s can not be combined with @Category",
+                clazz.getSimpleName());
+        errors.add(new Exception(message));
+    }
+}

src/main/java/org/junit/runners/ParentRunner.java

@@ -11,11 +11,15 @@
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Rule;
+import org.junit.validator.AnnotationValidator;
+import org.junit.validator.AnnotationValidatorFactory;
+import org.junit.validator.ValidateWith;
 import org.junit.internal.AssumptionViolatedException;
 import org.junit.internal.runners.model.EachTestNotifier;
 import org.junit.internal.runners.statements.RunAfters;
@@ -31,9 +35,9 @@
 import org.junit.runner.manipulation.Sorter;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runner.notification.StoppedByUserException;
+import org.junit.runners.model.FrameworkField;
 import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.InitializationError;
-import org.junit.runners.model.MultipleFailureException;
 import org.junit.runners.model.RunnerScheduler;
 import org.junit.runners.model.Statement;
 import org.junit.runners.model.TestClass;
@@ -59,6 +63,9 @@ public abstract class ParentRunner<T> extends Runner implements Filterable,
     // Guarded by fChildrenLock
     private volatile Collection<T> fFilteredChildren = null;
 
+    private final AnnotationValidatorFactory fAnnotationValidatorFactory =
+            new AnnotationValidatorFactory();
+
     private volatile RunnerScheduler fScheduler = new RunnerScheduler() {
         public void schedule(Runnable childStatement) {
             childStatement.run();
@@ -114,6 +121,54 @@ protected void collectInitializationErrors(List<Throwable> errors) {
         validatePublicVoidNoArgMethods(BeforeClass.class, true, errors);
         validatePublicVoidNoArgMethods(AfterClass.class, true, errors);
         validateClassRules(errors);
+        invokeValidators(errors);
+    }
+
+    private void invokeValidators(List<Throwable> errors) {
+        invokeValidatorsOnClass(errors);
+        invokeValidatorsOnMethods(errors);
+        invokeValidatorsOnFields(errors);
+    }
+
+    private void invokeValidatorsOnClass(List<Throwable> errors) {
+        Annotation[] annotations = getTestClass().getAnnotations();
+        for (Annotation annotation : annotations) {
+            Class<? extends Annotation> annotationType = annotation.annotationType();
+            ValidateWith validateWithAnnotation = annotationType.getAnnotation(ValidateWith.class);
+            if (validateWithAnnotation != null) {
+                AnnotationValidator annotationValidator =
+                        fAnnotationValidatorFactory.createAnnotationValidator(validateWithAnnotation);
+                errors.addAll(annotationValidator.validateAnnotatedClass(getTestClass()));
+            }
+        }
+    }
+
+    private void invokeValidatorsOnMethods(List<Throwable> errors) {
+        Map<Class<? extends Annotation>, List<FrameworkMethod>> annotationMap = getTestClass().getAnnotationToMethods();
+        for (Class<? extends Annotation> annotationType : annotationMap.keySet()) {
+            ValidateWith validateWithAnnotation = annotationType.getAnnotation(ValidateWith.class);
+            if (validateWithAnnotation != null) {
+                for (FrameworkMethod frameworkMethod : annotationMap.get(annotationType)) {
+                    AnnotationValidator annotationValidator =
+                            fAnnotationValidatorFactory.createAnnotationValidator(validateWithAnnotation);
+                    errors.addAll(annotationValidator.validateAnnotatedMethod(frameworkMethod));
+                }
+            }
+        }
+    }
+
+    private void invokeValidatorsOnFields(List<Throwable> errors) {
+        Map<Class<? extends Annotation>, List<FrameworkField>> annotationMap = getTestClass().getAnnotationToFields();
+        for (Class<? extends Annotation> annotationType : annotationMap.keySet()) {
+            ValidateWith validateWithAnnotation = annotationType.getAnnotation(ValidateWith.class);
+            if (validateWithAnnotation != null) {
+                for (FrameworkField frameworkField : annotationMap.get(annotationType)) {
+                    AnnotationValidator annotationValidator =
+                            fAnnotationValidatorFactory.createAnnotationValidator(validateWithAnnotation);
+                    errors.addAll(annotationValidator.validateAnnotatedField(frameworkField));
+                }
+            }
+        }
     }
 
     /**

src/main/java/org/junit/runners/model/TestClass.java

@@ -7,8 +7,10 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -24,8 +26,8 @@
  */
 public class TestClass {
     private final Class<?> fClass;
-    private final Map<Class<?>, List<FrameworkMethod>> fMethodsForAnnotations;
-    private final Map<Class<?>, List<FrameworkField>> fFieldsForAnnotations;
+    private final Map<Class<? extends Annotation>, List<FrameworkMethod>> fMethodsForAnnotations;
+    private final Map<Class<? extends Annotation>, List<FrameworkField>> fFieldsForAnnotations;
 
     /**
      * Creates a {@code TestClass} wrapping {@code klass}. Each time this
@@ -40,22 +42,38 @@ public TestClass(Class<?> klass) {
                     "Test class can only have one constructor");
         }
 
-        Map<Class<?>, List<FrameworkMethod>> methodsForAnnotations = new HashMap<Class<?>, List<FrameworkMethod>>();
-        Map<Class<?>, List<FrameworkField>> fieldsForAnnotations = new HashMap<Class<?>, List<FrameworkField>>();
+        Map<Class<? extends Annotation>, List<FrameworkMethod>> methodsForAnnotations =
+                new LinkedHashMap<Class<? extends Annotation>, List<FrameworkMethod>>();
+        Map<Class<? extends Annotation>, List<FrameworkField>> fieldsForAnnotations =
+                new LinkedHashMap<Class<? extends Annotation>, List<FrameworkField>>();
+
         for (Class<?> eachClass : getSuperClasses(fClass)) {
             for (Method eachMethod : MethodSorter.getDeclaredMethods(eachClass)) {
                 addToAnnotationLists(new FrameworkMethod(eachMethod), methodsForAnnotations);
             }
-            for (Field eachField : eachClass.getDeclaredFields()) {
+            // ensuring fields are sorted to make sure that entries are inserted
+            // and read from fieldForAnnotations in a deterministic order
+            for (Field eachField : getSortedDeclaredFields(eachClass)) {
                 addToAnnotationLists(new FrameworkField(eachField), fieldsForAnnotations);
             }
         }
-        fMethodsForAnnotations = Collections.unmodifiableMap(methodsForAnnotations);
-        fFieldsForAnnotations = Collections.unmodifiableMap(fieldsForAnnotations);
+
+        fMethodsForAnnotations = makeDeeplyUnmodifiable(methodsForAnnotations);
+        fFieldsForAnnotations = makeDeeplyUnmodifiable(fieldsForAnnotations);
+    }
+
+    private static Field[] getSortedDeclaredFields(Class<?> clazz) {
+        Field[] declaredFields = clazz.getDeclaredFields();
+        Arrays.sort(declaredFields, new Comparator<Field>() {
+            public int compare(Field field1, Field field2) {
+                return field1.getName().compareTo(field2.getName());
+            }
+        });
+        return declaredFields;
     }
 
     private static <T extends FrameworkMember<T>> void addToAnnotationLists(T member,
-            Map<Class<?>, List<T>> map) {
+            Map<Class<? extends Annotation>, List<T>> map) {
         for (Annotation each : member.getAnnotations()) {
             Class<? extends Annotation> type = each.annotationType();
             List<T> members = getAnnotatedMembers(map, type, true);
@@ -70,6 +88,17 @@ private static <T extends FrameworkMember<T>> void addToAnnotationLists(T member
         }
     }
 
+    private static <T extends FrameworkMember<T>> Map<Class<? extends Annotation>, List<T>>
+            makeDeeplyUnmodifiable(Map<Class<? extends Annotation>, List<T>> source) {
+        LinkedHashMap<Class<? extends Annotation>, List<T>> copy =
+                new LinkedHashMap<Class<? extends Annotation>, List<T>>();
+        for (Map.Entry<Class<? extends Annotation>, List<T>> entry : source.entrySet()) {
+            copy.put(entry.getKey(), Collections.unmodifiableList(entry.getValue()));
+        }
+        return Collections.unmodifiableMap(copy);
+    }
+
+
     /**
      * Returns, efficiently, all the non-overridden methods in this class and
      * its superclasses that are annotated with {@code annotationClass}.
@@ -88,7 +117,27 @@ public List<FrameworkField> getAnnotatedFields(
         return Collections.unmodifiableList(getAnnotatedMembers(fFieldsForAnnotations, annotationClass, false));
     }
 
-    private static <T> List<T> getAnnotatedMembers(Map<Class<?>, List<T>> map,
+    /**
+     * Gets a {@code Map} between annotations and methods that have
+     * the annotation in this class or its superclasses.
+     *
+     * @since 4.12
+     */
+    public Map<Class<? extends Annotation>, List<FrameworkMethod>> getAnnotationToMethods() {
+        return fMethodsForAnnotations;
+    }
+
+    /**
+     * Gets a {@code Map} between annotations and fields that have
+     * the annotation in this class or its superclasses.
+     *
+     * @since 4.12
+     */
+    public Map<Class<? extends Annotation>, List<FrameworkField>> getAnnotationToFields() {
+        return fFieldsForAnnotations;
+    }
+
+    private static <T> List<T> getAnnotatedMembers(Map<Class<? extends Annotation>, List<T>> map,
             Class<? extends Annotation> type, boolean fillIfAbsent) {
         if (!map.containsKey(type) && fillIfAbsent) {
             map.put(type, new ArrayList<T>());

src/main/java/org/junit/validator/AnnotationValidator.java

@@ -0,0 +1,60 @@
+package org.junit.validator;
+
+import org.junit.runners.model.FrameworkField;
+import org.junit.runners.model.FrameworkMethod;
+import org.junit.runners.model.TestClass;
+
+import static java.util.Collections.emptyList;
+
+import java.util.List;
+
+/**
+ * Validates annotations on classes and methods. To be validated,
+ * an annotation should be annotated with {@link ValidateWith}
+ *
+ * Instances of this class are shared by multiple test runners, so they should
+ * be immutable and thread-safe.
+ *
+ * @since 4.12
+ */
+public abstract class AnnotationValidator {
+
+    private static final List<Exception> NO_VALIDATION_ERRORS = emptyList();
+
+    /**
+     * Validates annotation on the given class.
+     *
+     * @param testClass that is being validated
+     * @return A list of exceptions. Default behavior is to return an empty list.
+     *
+     * @since 4.12
+     */
+    public List<Exception> validateAnnotatedClass(TestClass testClass) {
+        return NO_VALIDATION_ERRORS;
+    }
+
+    /**
+     * Validates annotation on the given field.
+     *
+     * @param field that is being validated
+     * @return A list of exceptions. Default behavior is to return an empty list.
+     *
+     * @since 4.12
+     */
+    public List<Exception> validateAnnotatedField(FrameworkField field) {
+        return NO_VALIDATION_ERRORS;
+
+    }
+
+    /**
+     * Validates annotation on the given method.
+     *
+     * @param method that is being validated
+     * @return A list of exceptions. Default behavior is to return an empty list.
+     *
+     * @since 4.12
+     */
+    public List<Exception> validateAnnotatedMethod(FrameworkMethod method) {
+        return NO_VALIDATION_ERRORS;
+    }
+}

src/main/java/org/junit/validator/AnnotationValidatorFactory.java

@@ -0,0 +1,44 @@
+package org.junit.validator;
+
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Creates instances of Annotation Validators.
+ *
+ * @since 4.12
+ */
+public class AnnotationValidatorFactory {
+
+    private static ConcurrentHashMap<ValidateWith, AnnotationValidator> fAnnotationTypeToValidatorMap =
+            new ConcurrentHashMap<ValidateWith, AnnotationValidator>();
+
+    /**
+     * Creates the AnnotationValidator specified by the value in
+     * {@link org.junit.validator.ValidateWith}. Instances are
+     * cached.
+     *
+     * @param validateWithAnnotation
+     * @return An instance of the AnnotationValidator.
+     *
+     * @since 4.12
+     */
+    public AnnotationValidator createAnnotationValidator(ValidateWith validateWithAnnotation) {
+        AnnotationValidator validator = fAnnotationTypeToValidatorMap.get(validateWithAnnotation);
+        if (validator != null) {
+            return validator;
+        }
+
+        Class<? extends AnnotationValidator> clazz = validateWithAnnotation.value();
+        if (clazz == null) {
+            throw new IllegalArgumentException("Can't create validator, value is null in annotation " + validateWithAnnotation.getClass().getName());
+        }
+        try {
+            AnnotationValidator annotationValidator = clazz.newInstance();
+            fAnnotationTypeToValidatorMap.putIfAbsent(validateWithAnnotation, annotationValidator);
+            return fAnnotationTypeToValidatorMap.get(validateWithAnnotation);
+        } catch (Exception e) {
+            throw new RuntimeException("Exception received when creating AnnotationValidator class " + clazz.getName(), e);
+        }
+    }
+
+}