Merge pull request #621 from pimterry/named-datapoints-#65

Added named datapoint(s) support to theories, fixing #65.

Author: David Saff

Diff for: src/main/java/org/junit/experimental/theories/DataPoint.java

@@ -10,5 +10,5 @@
 @Retention(RetentionPolicy.RUNTIME)
 @Target({FIELD, METHOD})
 public @interface DataPoint {
-
+    String[] value() default {}; 
 }

Diff for: src/main/java/org/junit/experimental/theories/DataPoints.java

@@ -7,8 +7,52 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+/**
+ * <p>
+ * Annotating an array-typed field or method with &#064;DataPoints will cause
+ * the values in the array (or returned array) to be used as potential
+ * parameters for theories in that class, when run with the
+ * {@link org.junit.experimental.theories.Theories Theories} runner.
+ * </p>
+ * <p>
+ * DataPoints will only be considered as potential values for parameters for
+ * which their types are assignable. When multiple sets of DataPoints exist with
+ * overlapping types more control can be obtained by naming the DataPoints using
+ * the value of this annotation, e.g. with
+ * <code>&#064;DataPoints({"dataset1", "dataset2"})</code>, and then specifying
+ * which named set to consider as potential values for each parameter using the
+ * {@link org.junit.experimental.theories.FromDataPoints &#064;FromDataPoints}
+ * annotation.
+ * </p>
+ * <p>
+ * Parameters with no specified source (i.e. without &#064;FromDataPoints or
+ * other {@link org.junit.experimental.theories.ParametersSuppliedBy
+ * &#064;ParameterSuppliedBy} annotations) will use all DataPoints that are
+ * assignable to the parameter type as potential values, including named sets of
+ * DataPoints.
+ * </p>
+ * 
+ * <pre>
+ * &#064;DataPoints
+ * public static String[] dataPoints = new String[] { ... };
+ * 
+ * &#064;DataPoints
+ * public static String[] generatedDataPoints() {
+ *     return new String[] { ... };
+ * }
+ * 
+ * &#064;Theory
+ * public void theoryMethod(String param) {
+ *     ...
+ * }</pre>
+ * 
+ * @see org.junit.experimental.theories.Theories
+ * @see org.junit.experimental.theories.Theory
+ * @see org.junit.experimental.theories.DataPoint
+ * @see org.junit.experimental.theories.FromDataPoints
+ */
 @Retention(RetentionPolicy.RUNTIME)
 @Target({FIELD, METHOD})
 public @interface DataPoints {
-
+    String[] value() default {};
 }

Diff for: src/main/java/org/junit/experimental/theories/FromDataPoints.java

@@ -0,0 +1,58 @@
+package org.junit.experimental.theories;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+import org.junit.experimental.theories.internal.SpecificDataPointsSupplier;
+
+/**
+ * <p>
+ * Annotating a parameter of a {@link org.junit.experimental.theories.Theory
+ * &#064Theory} method with <code>&#064;FromDataPoints</code> will limit the
+ * datapoints considered as potential values for that parameter to just the
+ * {@link org.junit.experimental.theories.DataPoints DataPoints} with the given
+ * name. DataPoint names can be given as the value parameter of the
+ * &#064DataPoints annotation.
+ * </p>
+ * <p>
+ * DataPoints without names will not be considered as values for any parameters
+ * annotated with &#064FromDataPoints.
+ * </p>
+ * 
+ * <pre>
+ * &#064;DataPoints
+ * public static String[] unnamed = new String[] { ... };
+ * 
+ * &#064;DataPoints("regexes")
+ * public static String[] regexStrings = new String[] { ... };
+ * 
+ * &#064;DataPoints({"forMatching", "alphanumeric"})
+ * public static String[] testStrings = new String[] { ... }; 
+ * 
+ * &#064;Theory
+ * public void stringTheory(String param) {
+ *     // This will be called with every value in 'regexStrings',
+ *     // 'testStrings' and 'unnamed'.
+ * }
+ * 
+ * &#064;Theory
+ * public void regexTheory(&#064;FromDataPoints("regexes") String regex,
+ *                         &#064;FromDataPoints("forMatching") String value) {
+ *     // This will be called with only the values in 'regexStrings' as 
+ *     // regex, only the values in 'testStrings' as value, and none 
+ *     // of the values in 'unnamed'.
+ * }
+ * </pre>
+ * 
+ * @see org.junit.experimental.theories.Theory
+ * @see org.junit.experimental.theories.DataPoint
+ * @see org.junit.experimental.theories.DataPoints
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.PARAMETER)
+@ParametersSuppliedBy(SpecificDataPointsSupplier.class)
+public @interface FromDataPoints {
+    String value();
+}

Diff for: src/main/java/org/junit/experimental/theories/Theories.java

@@ -1,5 +1,6 @@
 package org.junit.experimental.theories;
 
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -76,6 +77,28 @@ protected void validateTestMethods(List<Throwable> errors) {
             } else {
                 each.validatePublicVoidNoArg(false, errors);
             }
+            
+            for (ParameterSignature signature : each.getParameterSignatures()) {
+                ParametersSuppliedBy annotation = signature.findDeepAnnotation(ParametersSuppliedBy.class);
+                if (annotation != null) {
+                    validateParameterSupplier(annotation.value(), errors);
+                }
+            }
+        }
+    }
+
+    private void validateParameterSupplier(Class<? extends ParameterSupplier> supplierClass, List<Throwable> errors) {
+        Constructor<?>[] constructors = supplierClass.getConstructors();
+        
+        if (constructors.length != 1) {
+            errors.add(new Error("ParameterSupplier " + supplierClass.getName() + 
+                                 " must have only one constructor (either empty or taking only a TestClass)"));
+        } else {
+            Class<?>[] paramTypes = constructors[0].getParameterTypes();
+            if (!(paramTypes.length == 0) && !paramTypes[0].equals(TestClass.class)) {
+                errors.add(new Error("ParameterSupplier " + supplierClass.getName() + 
+                                     " constructor must take either nothing or a single TestClass instance"));
+            }
         }
     }
 

Diff for: src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java

@@ -2,15 +2,16 @@
 
 import java.lang.reflect.Array;
 import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 import org.junit.experimental.theories.DataPoint;
 import org.junit.experimental.theories.DataPoints;
 import org.junit.experimental.theories.ParameterSignature;
 import org.junit.experimental.theories.ParameterSupplier;
 import org.junit.experimental.theories.PotentialAssignment;
+import org.junit.runners.model.FrameworkField;
 import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.TestClass;
 
@@ -45,8 +46,8 @@ public Object getValue() throws CouldNotGenerateValueException {
         public String getDescription() throws CouldNotGenerateValueException {
             return fMethod.getName();
         }
-    }
-
+    }   
+    
     private final TestClass fClass;
 
     /**
@@ -60,16 +61,16 @@ public AllMembersSupplier(TestClass type) {
     public List<PotentialAssignment> getValueSources(ParameterSignature sig) {
         List<PotentialAssignment> list = new ArrayList<PotentialAssignment>();
 
-        addFields(sig, list);
+        addSinglePointFields(sig, list);
+        addMultiPointFields(sig, list);
         addSinglePointMethods(sig, list);
         addMultiPointMethods(sig, list);
 
         return list;
     }
 
     private void addMultiPointMethods(ParameterSignature sig, List<PotentialAssignment> list) {
-        for (FrameworkMethod dataPointsMethod : fClass
-                .getAnnotatedMethods(DataPoints.class)) {
+        for (FrameworkMethod dataPointsMethod : getDataPointsMethods(sig)) {
             try {
                 addMultiPointArrayValues(sig, dataPointsMethod.getName(), list, dataPointsMethod.invokeExplosively(null));
             } catch (Throwable e) {
@@ -80,33 +81,35 @@ private void addMultiPointMethods(ParameterSignature sig, List<PotentialAssignme
 
     private void addSinglePointMethods(ParameterSignature sig,
             List<PotentialAssignment> list) {
-        for (FrameworkMethod dataPointMethod : fClass
-                .getAnnotatedMethods(DataPoint.class)) {
+        for (FrameworkMethod dataPointMethod : getSingleDataPointMethods(sig)) {
             if (sig.canAcceptType(dataPointMethod.getType())) {
                 list.add(new MethodParameterValue(dataPointMethod));
             }
         }
     }
-
-    private void addFields(ParameterSignature sig,
+    
+    private void addMultiPointFields(ParameterSignature sig,
             List<PotentialAssignment> list) {
-        for (final Field field : fClass.getJavaClass().getFields()) {
-            if (Modifier.isStatic(field.getModifiers())) {
-                Class<?> type = field.getType();
-                if (sig.canAcceptArrayType(type)
-                        && field.getAnnotation(DataPoints.class) != null) {
-                    try {
-                        addArrayValues(field.getName(), list, getStaticFieldValue(field));
-                    } catch (Throwable e) {
-                        // ignore and move on
-                    }
-                } else if (sig.canAcceptType(type)
-                        && field.getAnnotation(DataPoint.class) != null) {
-                    list.add(PotentialAssignment
-                            .forValue(field.getName(), getStaticFieldValue(field)));
+        for (final Field field : getDataPointsFields(sig)) {
+            Class<?> type = field.getType();
+            if (sig.canAcceptArrayType(type)) {
+                try {
+                    addArrayValues(field.getName(), list, getStaticFieldValue(field));
+                } catch (Throwable e) {
+                    // ignore and move on
                 }
             }
         }
+    }    
+
+    private void addSinglePointFields(ParameterSignature sig,
+            List<PotentialAssignment> list) {
+        for (final Field field : getSingleDataPointFields(sig)) {
+            Class<?> type = field.getType();
+            if (sig.canAcceptType(type)) {
+                list.add(PotentialAssignment.forValue(field.getName(), getStaticFieldValue(field)));
+            }
+        }
     }
 
     private void addArrayValues(String name, List<PotentialAssignment> list, Object array) {
@@ -136,4 +139,35 @@ private Object getStaticFieldValue(final Field field) {
                     "unexpected: getFields returned an inaccessible field");
         }
     }
+
+    protected Collection<FrameworkMethod> getDataPointsMethods(ParameterSignature sig) {
+        return fClass.getAnnotatedMethods(DataPoints.class);        
+    }
+    
+    protected Collection<Field> getSingleDataPointFields(ParameterSignature sig) {
+        List<FrameworkField> fields = fClass.getAnnotatedFields(DataPoint.class);
+        Collection<Field> validFields = new ArrayList<Field>();
+
+        for (FrameworkField frameworkField : fields) {
+            validFields.add(frameworkField.getField());
+        }
+
+        return validFields;
+    }
+    
+    protected Collection<Field> getDataPointsFields(ParameterSignature sig) {
+        List<FrameworkField> fields = fClass.getAnnotatedFields(DataPoints.class);
+        Collection<Field> validFields = new ArrayList<Field>();
+
+        for (FrameworkField frameworkField : fields) {
+            validFields.add(frameworkField.getField());
+        }
+
+        return validFields;
+    }
+    
+    protected Collection<FrameworkMethod> getSingleDataPointMethods(ParameterSignature sig) {
+        return fClass.getAnnotatedMethods(DataPoint.class);
+    }
+
 }

Diff for: src/main/java/org/junit/experimental/theories/internal/Assignments.java

@@ -1,5 +1,6 @@
 package org.junit.experimental.theories.internal;
 
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
@@ -56,8 +57,8 @@ public Assignments assignNext(PotentialAssignment source) {
                 fAssigned);
         assigned.add(source);
 
-        return new Assignments(assigned, fUnassigned.subList(1, fUnassigned
-                .size()), fClass);
+        return new Assignments(assigned, fUnassigned.subList(1,
+                fUnassigned.size()), fClass);
     }
 
     public Object[] getActualValues(int start, int stop, boolean nullsOk)
@@ -74,29 +75,36 @@ public Object[] getActualValues(int start, int stop, boolean nullsOk)
     }
 
     public List<PotentialAssignment> potentialsForNextUnassigned()
-            throws InstantiationException, IllegalAccessException {
+            throws Exception {
         ParameterSignature unassigned = nextUnassigned();
         return getSupplier(unassigned).getValueSources(unassigned);
     }
 
-    public ParameterSupplier getSupplier(ParameterSignature unassigned)
-            throws InstantiationException, IllegalAccessException {
-        ParameterSupplier supplier = getAnnotatedSupplier(unassigned);
-        if (supplier != null) {
-            return supplier;
-        }
+    private ParameterSupplier getSupplier(ParameterSignature unassigned)
+            throws Exception {
+        ParametersSuppliedBy annotation = unassigned
+                .findDeepAnnotation(ParametersSuppliedBy.class);
 
-        return new AllMembersSupplier(fClass);
+        if (annotation != null) {
+            return buildParameterSupplierFromClass(annotation.value());
+        } else {
+            return new AllMembersSupplier(fClass);
+        }
     }
 
-    public ParameterSupplier getAnnotatedSupplier(ParameterSignature unassigned)
-            throws InstantiationException, IllegalAccessException {
-        ParametersSuppliedBy annotation = unassigned
-                .findDeepAnnotation(ParametersSuppliedBy.class);
-        if (annotation == null) {
-            return null;
+    private ParameterSupplier buildParameterSupplierFromClass(
+            Class<? extends ParameterSupplier> cls) throws Exception {
+        Constructor<?>[] supplierConstructors = cls.getConstructors();
+
+        for (Constructor<?> constructor : supplierConstructors) {
+            Class<?>[] parameterTypes = constructor.getParameterTypes();
+            if (parameterTypes.length == 1
+                    && parameterTypes[0].equals(TestClass.class)) {
+                return (ParameterSupplier) constructor.newInstance(fClass);
+            }
         }
-        return annotation.value().newInstance();
+
+        return cls.newInstance();
     }
 
     public Object[] getConstructorArguments(boolean nullsOk)