changeset 320:9c392e4cd432

Yak shaving: move @State objects verification to appropriate place.
author shade
date Fri, 06 Dec 2013 19:14:03 +0400
parents 99f21384901b
children 5f79c18b487d
files jmh-core/src/main/java/org/openjdk/jmh/processor/internal/GenerateMicroBenchmarkProcessor.java
diffstat 1 files changed, 43 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- a/jmh-core/src/main/java/org/openjdk/jmh/processor/internal/GenerateMicroBenchmarkProcessor.java	Fri Dec 06 19:03:07 2013 +0400
+++ b/jmh-core/src/main/java/org/openjdk/jmh/processor/internal/GenerateMicroBenchmarkProcessor.java	Fri Dec 06 19:14:03 2013 +0400
@@ -176,17 +176,44 @@
             throw new GenerationException("Microbenchmark should have package other than default.", clazz);
         }
 
+        Collection<TypeElement> states = new ArrayList<TypeElement>();
+
         // validate all arguments are @State-s
         for (Element e : methods) {
             ExecutableElement method = (ExecutableElement) e;
             for (VariableElement var : method.getParameters()) {
-                Element argClass = processingEnv.getTypeUtils().asElement(var.asType());
+                TypeElement argClass = (TypeElement) processingEnv.getTypeUtils().asElement(var.asType());
                 if (argClass.getAnnotation(State.class) == null) {
                     throw new GenerationException(
                             "The " + GenerateMicroBenchmark.class.getSimpleName() +
                             " annotation only supports methods with @State-bearing typed parameters.",
                             var);
                 }
+                states.add(argClass);
+            }
+        }
+
+        // validate @State classes
+        for (TypeElement state : states) {
+            if (!state.getModifiers().contains(Modifier.PUBLIC)) {
+                throw new GenerationException("The " + State.class.getSimpleName() +
+                        " annotation only supports public classes.", state);
+            }
+            if (state.getNestingKind().isNested() && !state.getModifiers().contains(Modifier.STATIC)) {
+                throw new GenerationException("The " + State.class.getSimpleName() +
+                        " annotation does not support inner classes, make sure the class is nested (static).",
+                        state);
+            }
+
+            boolean hasDefaultConstructor = false;
+            for (ExecutableElement constructor : ElementFilter.constructorsIn(state.getEnclosedElements())) {
+                hasDefaultConstructor |= (constructor.getParameters().isEmpty() && constructor.getModifiers().contains(Modifier.PUBLIC));
+            }
+
+            if (!hasDefaultConstructor) {
+                throw new GenerationException("The " + State.class.getSimpleName() +
+                        " annotation can only be applied to the classes having the default public constructor.",
+                        state);
             }
         }
 
@@ -203,15 +230,17 @@
             }
         }
 
+        // check modifiers
         for (Element m : methods) {
+            if (!m.getModifiers().contains(Modifier.PUBLIC)) {
+                throw new GenerationException("@" + GenerateMicroBenchmark.class.getSimpleName() +
+                        " method should be public.", m);
+            }
+
             if (m.getModifiers().contains(Modifier.ABSTRACT)) {
                 throw new GenerationException("@" + GenerateMicroBenchmark.class.getSimpleName()
                         + " method can not be abstract.", m);
             }
-            if (m.getModifiers().contains(Modifier.PRIVATE)) {
-                throw new GenerationException("@" + GenerateMicroBenchmark.class.getSimpleName()
-                        + " method can not be private.", m);
-            }
             if (m.getModifiers().contains(Modifier.SYNCHRONIZED)) {
                 if (clazz.getAnnotation(State.class) == null) {
                     throw new GenerationException("@" + GenerateMicroBenchmark.class.getSimpleName()
@@ -220,6 +249,15 @@
                     }
             }
         }
+
+        // check annotations
+        for (Element m : methods) {
+            OperationsPerInvocation opi = AnnUtils.getAnnotationRecursive(m, OperationsPerInvocation.class);
+            if (opi != null && opi.value() < 1) {
+                throw new GenerationException("The " + OperationsPerInvocation.class.getSimpleName() +
+                        " needs to be greater than 0.", m);
+            }
+        }
     }
 
     /**
@@ -371,14 +409,10 @@
             // Write all methods
             for (String groupName : info.methodGroups.keySet()) {
                 for (Element method : info.methodGroups.get(groupName).methods()) {
-                    // Final checks...
-                    verifyAnnotations(method);
-
                     // Look for method signature and figure out state bindings
                     ExecutableElement execMethod = (ExecutableElement) method;
                     for (VariableElement element : execMethod.getParameters()) {
                         TypeElement stateType = (TypeElement) processingEnv.getTypeUtils().asElement(element.asType());
-                        verifyState(stateType);
                         states.bindArg(execMethod, stateType);
                     }
                 }
@@ -418,32 +452,6 @@
         }
     }
 
-    private void verifyState(TypeElement type) {
-        if (!type.getModifiers().contains(Modifier.PUBLIC)) {
-            throw new GenerationException(
-                    "The " + State.class.getSimpleName() + " annotation only supports public classes.",
-                    type);
-        }
-        if (type.getNestingKind().isNested() && !type.getModifiers().contains(Modifier.STATIC)) {
-            throw new GenerationException(
-                    "The " + State.class.getSimpleName()
-                            + " annotation does not support inner classes, make sure the class is nested (static).",
-                    type);
-        }
-
-        boolean hasDefaultConstructor = false;
-        for (ExecutableElement constructor : ElementFilter.constructorsIn(type.getEnclosedElements())) {
-            hasDefaultConstructor |= (constructor.getParameters().isEmpty() && constructor.getModifiers().contains(Modifier.PUBLIC));
-        }
-
-        if (!hasDefaultConstructor) {
-            throw new GenerationException(
-                    "The " + State.class.getSimpleName()
-                            + " annotation can only be applied to the classes having the default public constructor.",
-                    type);
-        }
-    }
-
     private void generateFields(PrintWriter writer) {
         // nothing here
     }
@@ -542,26 +550,6 @@
         return (ann != null) ? ann.value() : 1;
     }
 
-    /**
-     * Verifying that all annotations data is valid
-     *
-     * @param method
-     */
-    private void verifyAnnotations(Element method) {
-        OperationsPerInvocation operationsPerInvocation = method.getAnnotation(OperationsPerInvocation.class);
-        if (operationsPerInvocation != null && operationsPerInvocation.value() < 1) {
-            throw new GenerationException(
-                    "The " + OperationsPerInvocation.class.getSimpleName()
-                            + " needs to be greater than 0.",
-                    method);
-        }
-        if (!method.getModifiers().contains(Modifier.PUBLIC)) {
-            throw new GenerationException(
-                    "@" + GenerateMicroBenchmark.class.getSimpleName() + " method should be public.",
-                    method);
-        }
-    }
-
 
     private static String annotationMapToString(Map<String, String> map) {
         StringBuilder sb = new StringBuilder();