HV-446: Supporting method constraints in AP

HV-446: Adding tests

hibernate-validator-annotation-processor/src/main/java/org/hibernate/validator/ap/ConstraintAnnotationVisitor.java

@@ -51,11 +51,11 @@ final class ConstraintAnnotationVisitor extends ElementKindVisitor6<Void, List<A
 	private final boolean verbose;
 
 	public ConstraintAnnotationVisitor(
-			ProcessingEnvironment processingEnvironment, MessagerAdapter messager, boolean verbose) {
+			ProcessingEnvironment processingEnvironment, MessagerAdapter messager, boolean verbose, boolean methodConstraintsSupported) {
 
 		this.messager = messager;
 		this.verbose = verbose;
-
+		
 		AnnotationApiHelper annotationApiHelper = new AnnotationApiHelper(
 				processingEnvironment.getElementUtils(), processingEnvironment.getTypeUtils()
 		);
@@ -65,7 +65,7 @@ public ConstraintAnnotationVisitor(
 		);
 
 		constraintCheckFactory = new ConstraintCheckFactory(
-				processingEnvironment.getTypeUtils(), constraintHelper, annotationApiHelper
+				processingEnvironment.getTypeUtils(), constraintHelper, annotationApiHelper, methodConstraintsSupported
 		);
 	}
 

hibernate-validator-annotation-processor/src/main/java/org/hibernate/validator/ap/ConstraintValidationProcessor.java

@@ -40,22 +40,29 @@
  * href="http://jcp.org/en/jsr/detail?id=303">Bean Validation</a> constraints.
  * The processor supports the following options:
  * <ul>
- * <li><code>diagnosticKind</code>: the severity with which any occurred problems
- * shall be reported. Must be given in form of the string representation of a
- * value from {@link javax.tools.Diagnostic.Kind}, e.g.
+ * <li><code>diagnosticKind</code>: the severity with which any occurred
+ * problems shall be reported. Must be given in form of the string
+ * representation of a value from {@link javax.tools.Diagnostic.Kind}, e.g.
  * "diagnosticKind=WARNING". Default is Kind.ERROR.</li>
- * <li>TODO GM: validationMode: whether spec compliance shall be checked
- * strictly or loosely (e.g. by allowing validators for parametrized types)</li>
- * </ul>
- *
+ * <li><code>verbose</code>: whether a verbose output shall be created or not.
+ * Must be given as String parsable by {@link Boolean#parseBoolean}. Default is
+ * <code>false</code>.</li>
+ * <li><code>methodConstraintsSupported</code>: Whether constraints at other
+ * methods than JavaBeans getter methods may be annotated with constraints. Must
+ * be given as String parsable by {@link Boolean#parseBoolean}. Can be set to
+ * <code>false</code> in order to allow only getter based property constraints
+ * but not method level constraints as supported by Hibernate Validator. Default
+ * is <code>true</code>.</li>
+ * 
  * @author Hardy Ferentschik
  * @author Gunnar Morling
  */
 @SupportedAnnotationTypes("*")
 @SupportedSourceVersion(SourceVersion.RELEASE_6)
 @SupportedOptions({
 		ConstraintValidationProcessor.DIAGNOSTIC_KIND_PROCESSOR_OPTION_NAME,
-		ConstraintValidationProcessor.VERBOSE_PROCESSOR_OPTION_NAME
+		ConstraintValidationProcessor.VERBOSE_PROCESSOR_OPTION_NAME,
+		ConstraintValidationProcessor.METHOD_CONSTRAINTS_SUPPORTED_PROCESSOR_OPTION_NAME
 })
 public class ConstraintValidationProcessor extends AbstractProcessor {
 
@@ -69,6 +76,12 @@ public class ConstraintValidationProcessor extends AbstractProcessor {
 	 * The name of the processor option for activating verbose message reporting.
 	 */
 	public final static String VERBOSE_PROCESSOR_OPTION_NAME = "verbose";
+
+	/**
+	 * The name of the processor option for allowing constraints at methods
+	 * other than getter methods.
+	 */
+	public final static String METHOD_CONSTRAINTS_SUPPORTED_PROCESSOR_OPTION_NAME = "methodConstraintsSupported";
 
 	/**
 	 * The diagnostic kind to be used if no or an invalid kind is given as processor option.
@@ -89,12 +102,20 @@ public class ConstraintValidationProcessor extends AbstractProcessor {
 	 */
 	private boolean verbose;
 
+	/**
+	 * Whether method constraints are allowed at any method (true) or only
+	 * getter methods (false).
+	 */
+	private boolean methodConstraintsSupported;
+
 	@Override
 	public synchronized void init(ProcessingEnvironment processingEnv) {
 
 		super.init( processingEnv );
 
 		this.verbose = isVerbose();
+		this.methodConstraintsSupported = methodConstraintsSupported();
+
 		messager = new MessagerAdapter( processingEnv.getMessager(), getDiagnosticKind() );
 	}
 
@@ -108,7 +129,7 @@ public boolean process(
 		);
 
 		ElementVisitor<Void, List<AnnotationMirror>> visitor = new ConstraintAnnotationVisitor(
-				processingEnv, messager, verbose
+				processingEnv, messager, verbose, methodConstraintsSupported
 		);
 
 		for ( TypeElement oneAnnotation : annotations ) {
@@ -178,4 +199,13 @@ private boolean isVerbose() {
 		return theValue;
 	}
 
+	/**
+	 * Retrieves the value for the "methodConstraintsSupported" property from the options.
+	 *
+	 * @return The value for the "methodConstraintsSupported" property.
+	 */
+	private boolean methodConstraintsSupported() {
+
+		return Boolean.parseBoolean( processingEnv.getOptions().get( METHOD_CONSTRAINTS_SUPPORTED_PROCESSOR_OPTION_NAME ) );
+	}
 }

hibernate-validator-annotation-processor/src/main/java/org/hibernate/validator/ap/checks/ConstraintCheckFactory.java

@@ -58,7 +58,7 @@ public class ConstraintCheckFactory {
 
 	private final static SingleValuedChecks NULL_CHECKS = new SingleValuedChecks();
 
-	public ConstraintCheckFactory(Types typeUtils, ConstraintHelper constraintHelper, AnnotationApiHelper annotationApiHelper) {
+	public ConstraintCheckFactory(Types typeUtils, ConstraintHelper constraintHelper, AnnotationApiHelper annotationApiHelper, boolean methodConstraintsSupported) {
 
 		this.constraintHelper = constraintHelper;
 
@@ -80,16 +80,16 @@ public ConstraintCheckFactory(Types typeUtils, ConstraintHelper constraintHelper
 		methodChecks = CollectionHelper.newHashMap();
 		methodChecks.put(
 				AnnotationType.CONSTRAINT_ANNOTATION,
-				new SingleValuedChecks( new GetterCheck(), new StaticCheck(), new TypeCheck( constraintHelper ) )
+				new SingleValuedChecks( new GetterCheck(methodConstraintsSupported), new StaticCheck(), new TypeCheck( constraintHelper ) )
 		);
 		methodChecks.put(
 				AnnotationType.MULTI_VALUED_CONSTRAINT_ANNOTATION, new MultiValuedChecks(
-						constraintHelper, new GetterCheck(), new StaticCheck(), new TypeCheck( constraintHelper )
+						constraintHelper, new GetterCheck(methodConstraintsSupported), new StaticCheck(), new TypeCheck( constraintHelper )
 				)
 		);
 		methodChecks.put(
 				AnnotationType.GRAPH_VALIDATION_ANNOTATION,
-				new SingleValuedChecks( new GetterCheck(), new StaticCheck(), new PrimitiveCheck() )
+				new SingleValuedChecks( new GetterCheck(methodConstraintsSupported), new StaticCheck(), new PrimitiveCheck() )
 		);
 		methodChecks.put( AnnotationType.NO_CONSTRAINT_ANNOTATION, NULL_CHECKS );
 

hibernate-validator-annotation-processor/src/main/java/org/hibernate/validator/ap/checks/GetterCheck.java

@@ -30,16 +30,30 @@
  * @author Gunnar Morling
  */
 public class GetterCheck extends AbstractConstraintCheck {
+
+	private final boolean methodConstraintsSupported;
+
+	public GetterCheck(boolean methodConstraintsSupported) {
+		this.methodConstraintsSupported  = methodConstraintsSupported;
+	}
+
 	public Set<ConstraintCheckError> checkMethod(ExecutableElement element,
 												 AnnotationMirror annotation) {
 
-		if ( !isGetterMethod( element ) ) {
+		if ( !methodConstraintsSupported && !isGetterMethod( element ) ) {
 			return CollectionHelper.asSet(
 					new ConstraintCheckError(
 							element, annotation, "ONLY_GETTERS_MAY_BE_ANNOTATED"
 					)
 			);
 		}
+		else if (!hasReturnValue(element)) {
+			return CollectionHelper.asSet(
+					new ConstraintCheckError(
+							element, annotation, "ONLY_NON_VOID_METHODS_MAY_BE_ANNOTATED"
+					)
+			);
+		}
 
 		return Collections.emptySet();
 	}

hibernate-validator-annotation-processor/src/main/resources/org/hibernate/validator/ap/ValidationProcessorMessages.properties

@@ -25,3 +25,4 @@ GROUP_SEQUENCE_PROVIDER_ANNOTATION_NOT_ALLOWED_ON_CLASS_WITH_GROUP_SEQUENCE_ANNO
 GROUP_SEQUENCE_PROVIDER_ANNOTATION_VALUE_MUST_BE_AN_IMPLEMENTATION_CLASS=The annotation @GroupSequenceProvider must define an implementation or a concrete implementation of DefaultGroupSequenceProvider interface.
 GROUP_SEQUENCE_PROVIDER_ANNOTATION_VALUE_DEFINED_PROVIDER_CLASS_WITH_WRONG_TYPE=The annotation @GroupSequenceProvider defines a default group sequence provider for type {0} instead of {1} (super)type.
 GROUP_SEQUENCE_PROVIDER_ANNOTATION_VALUE_CLASS_MUST_HAVE_DEFAULT_CONSTRUCTOR=The annotation @GroupSequenceProvider defines a default group sequence provider class without a public default constructor.
+ONLY_NON_VOID_METHODS_MAY_BE_ANNOTATED=Void methods may not be annotated with constraint annotations.

hibernate-validator-annotation-processor/src/test/java/org/hibernate/validator/ap/ConstraintValidationProcessorTest.java

@@ -16,15 +16,12 @@
 */
 package org.hibernate.validator.ap;
 
-import static org.hibernate.validator.ap.testutil.CompilerTestHelper.assertThatDiagnosticsMatch;
-import static org.testng.Assert.assertFalse;
-import static org.testng.Assert.assertTrue;
-
 import java.io.File;
-
 import javax.tools.Diagnostic;
 import javax.tools.Diagnostic.Kind;
 
+import org.testng.annotations.Test;
+
 import org.hibernate.validator.ap.testmodel.FieldLevelValidationUsingBuiltInConstraints;
 import org.hibernate.validator.ap.testmodel.MethodLevelValidationUsingBuiltInConstraints;
 import org.hibernate.validator.ap.testmodel.ModelWithJodaTypes;
@@ -71,7 +68,9 @@
 import org.hibernate.validator.ap.testmodel.nouniquevalidatorresolution.SizeValidatorForSet;
 import org.hibernate.validator.ap.util.DiagnosticExpectation;
 
-import org.testng.annotations.Test;
+import static org.hibernate.validator.ap.testutil.CompilerTestHelper.assertThatDiagnosticsMatch;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 
 /**
  * Miscellaneous tests for {@link ConstraintValidationProcessor}.
@@ -103,7 +102,7 @@ public void testThatProcessorOptionsAreEvaluated() {
 		// compile with -AdiagnosticKind=Kind.WARNING and -Averbose=true
 		boolean compilationResult =
 				compilerHelper.compile(
-						new ConstraintValidationProcessor(), diagnostics, Kind.WARNING, true, sourceFile
+						new ConstraintValidationProcessor(), diagnostics, Kind.WARNING, true, false, sourceFile
 				);
 
 		// compilation succeeds as there are problems, but Kind.WARNING won't stop compilation
@@ -175,7 +174,37 @@ public void methodLevelValidationUsingBuiltInConstraints() {
 				new DiagnosticExpectation( Kind.ERROR, 31 ),
 				new DiagnosticExpectation( Kind.ERROR, 38 ),
 				new DiagnosticExpectation( Kind.ERROR, 46 ),
-				new DiagnosticExpectation( Kind.ERROR, 53 )
+				new DiagnosticExpectation( Kind.ERROR, 53 ),
+				new DiagnosticExpectation( Kind.ERROR, 61 ),
+				new DiagnosticExpectation( Kind.ERROR, 69 ),
+				new DiagnosticExpectation( Kind.ERROR, 77 ),
+				new DiagnosticExpectation( Kind.ERROR, 84 )
+		);
+	}
+
+	/**
+	 * Constraints are allowed at non-getters per processor option, but all other
+	 * checks (no static methods allowed etc.) still apply.
+	 */
+	@Test
+	public void methodLevelConstraintsAllowedAtNonGetterMethods() {
+
+		File sourceFile = compilerHelper.getSourceFile( MethodLevelValidationUsingBuiltInConstraints.class );
+
+		//compile with -AmethodConstraintsSupported
+		boolean compilationResult =
+				compilerHelper.compile( new ConstraintValidationProcessor(), diagnostics, false, true, sourceFile );
+
+		assertFalse( compilationResult );
+		assertThatDiagnosticsMatch(
+				diagnostics,
+				new DiagnosticExpectation( Kind.ERROR, 31 ),
+				new DiagnosticExpectation( Kind.ERROR, 38 ),
+				new DiagnosticExpectation( Kind.ERROR, 46 ),
+				new DiagnosticExpectation( Kind.ERROR, 53 ),
+				new DiagnosticExpectation( Kind.ERROR, 69 ),
+				new DiagnosticExpectation( Kind.ERROR, 77 ),
+				new DiagnosticExpectation( Kind.ERROR, 84 )
 		);
 	}
 

hibernate-validator-annotation-processor/src/test/java/org/hibernate/validator/ap/testmodel/MethodLevelValidationUsingBuiltInConstraints.java

@@ -54,4 +54,36 @@ public void getAnotherString() {
 	public static String getStringStatically() {
 		return null;
 	}
+
+	/**
+	 * No getter, but allowed with -AmethodConstraintsSupported.
+	 */
+	@Size(min = 10)
+	public String doSomething() {
+		return null;
+	}
+
+	/**
+	 * Also with -AmethodConstraintsSupported not allowed, as return type doesn't match.
+	 */
+	@Size(min = 10)
+	public Date doSomethingReturningDate() {
+		return null;
+	}
+
+	/**
+	 * Also with -AmethodConstraintsSupported not allowed. No return type.
+	 */
+	@Size(min = 10)
+	public void voidDoSomething() {
+	}
+
+	/**
+	 * Also with -AmethodConstraintsSupported not allowed. Static method.
+	 */
+	@Size(min = 10)
+	public static String staticDoSomething() {
+		return null;
+	}
+
 }

hibernate-validator-annotation-processor/src/test/java/org/hibernate/validator/ap/testutil/CompilerTestHelper.java

@@ -16,15 +16,11 @@
 */
 package org.hibernate.validator.ap.testutil;
 
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertTrue;
-
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-
 import javax.annotation.processing.Processor;
 import javax.tools.Diagnostic;
 import javax.tools.Diagnostic.Kind;
@@ -36,6 +32,9 @@
 
 import org.hibernate.validator.ap.util.DiagnosticExpectation;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
 /**
  * Infrastructure for unit tests based on the Java Compiler API.
  *
@@ -79,30 +78,30 @@ public File getSourceFile(Class<?> clazz) {
 	}
 
 	/**
-	 * @see CompilerTestHelper#compile(Processor, DiagnosticCollector, Kind, Boolean, File...)
+	 * @see CompilerTestHelper#compile(Processor, DiagnosticCollector, Kind, Boolean, Boolean, File...)
 	 */
 	public boolean compile(
 			Processor annotationProcessor, DiagnosticCollector<JavaFileObject> diagnostics, File... sourceFiles) {
 
-		return compile( annotationProcessor, diagnostics, null, null, sourceFiles );
+		return compile( annotationProcessor, diagnostics, null, null, null, sourceFiles );
 	}
 
 	/**
-	 * @see CompilerTestHelper#compile(Processor, DiagnosticCollector, Kind, Boolean, File...)
+	 * @see CompilerTestHelper#compile(Processor, DiagnosticCollector, Kind, Boolean, Boolean, File...)
 	 */
 	public boolean compile(
 			Processor annotationProcessor, DiagnosticCollector<JavaFileObject> diagnostics, Kind diagnosticKind, File... sourceFiles) {
 
-		return compile( annotationProcessor, diagnostics, diagnosticKind, null, sourceFiles );
+		return compile( annotationProcessor, diagnostics, diagnosticKind, null, null, sourceFiles );
 	}
 
 	/**
-	 * @see CompilerTestHelper#compile(Processor, DiagnosticCollector, Kind, Boolean, File...)
+	 * @see CompilerTestHelper#compile(Processor, DiagnosticCollector, Kind, Boolean, Boolean, File...)
 	 */
 	public boolean compile(
-			Processor annotationProcessor, DiagnosticCollector<JavaFileObject> diagnostics, boolean verbose, File... sourceFiles) {
+			Processor annotationProcessor, DiagnosticCollector<JavaFileObject> diagnostics, boolean verbose, boolean allowMethodConstraints, File... sourceFiles) {
 
-		return compile( annotationProcessor, diagnostics, null, verbose, sourceFiles );
+		return compile( annotationProcessor, diagnostics, null, verbose, allowMethodConstraints, sourceFiles );
 	}
 
 	/**
@@ -119,7 +118,7 @@ public boolean compile(
 	 *         any errors), false otherwise.
 	 */
 	public boolean compile(
-			Processor annotationProcessor, DiagnosticCollector<JavaFileObject> diagnostics, Kind diagnosticKind, Boolean verbose, File... sourceFiles) {
+			Processor annotationProcessor, DiagnosticCollector<JavaFileObject> diagnostics, Kind diagnosticKind, Boolean verbose, Boolean allowMethodConstraints, File... sourceFiles) {
 
 		StandardJavaFileManager fileManager =
 				compiler.getStandardFileManager( null, null, null );
@@ -138,6 +137,10 @@ public boolean compile(
 			options.add( "-Averbose=" + verbose.toString() );
 		}
 
+		if ( allowMethodConstraints != null ) {
+			options.add( "-AmethodConstraintsSupported=" + allowMethodConstraints.toString() );
+		}
+
 		CompilationTask task = compiler.getTask( null, fileManager, diagnostics, options, null, compilationUnits );
 		task.setProcessors( Arrays.asList( annotationProcessor ) );
 
@@ -168,7 +171,10 @@ public static void assertThatDiagnosticsMatch(DiagnosticCollector<JavaFileObject
 		else {
 
 			if ( diagnosticsList.size() != expectations.length ) {
-				System.out.println( diagnosticsList );
+
+				for ( Diagnostic<? extends JavaFileObject> oneDiagnostic : diagnosticsList ) {
+					System.out.println( oneDiagnostic );
+				}
 			}
 
 			assertEquals( diagnosticsList.size(), expectations.length, "Wrong number of diagnostics." );