HV-1395 Changing unwrapping rules in AP

Author: marko-bekhta

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

@@ -11,7 +11,6 @@
 import java.util.Set;
 
 import javax.lang.model.element.AnnotationMirror;
-import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.Name;
@@ -22,6 +21,7 @@
 import javax.lang.model.util.Types;
 
 import org.hibernate.validator.ap.internal.util.AnnotationApiHelper;
+import org.hibernate.validator.ap.internal.util.AnnotationApiHelper.UnwrapMode;
 import org.hibernate.validator.ap.internal.util.CollectionHelper;
 import org.hibernate.validator.ap.internal.util.ConstraintHelper;
 import org.hibernate.validator.ap.internal.util.ConstraintHelper.AnnotationProcessorValidationTarget;
@@ -74,11 +74,23 @@ public Set<ConstraintCheckIssue> checkNonAnnotationType(TypeElement element, Ann
 	}
 
 	private Set<ConstraintCheckIssue> checkInternal(Element element, AnnotationMirror annotation, TypeMirror type, String messageKey) {
-		TypeMirror typeToCheck = usesUnwrapping( annotation, type ) ? getUnwrappedType( type ) : type;
-		if ( !isAnnotationAllowedForType( annotation, typeToCheck ) ) {
+		Optional<TypeMirror> typeToCheck = usesUnwrapping( annotation, type ) ?
+				getUnwrappedType( type ) :
+				Optional.of( type );
+		if ( typeToCheck.isPresent() ) {
+			if ( !isAnnotationAllowedForType( annotation, typeToCheck.get() ) ) {
+				return CollectionHelper.asSet(
+						ConstraintCheckIssue.error(
+								element, annotation, messageKey,
+								annotation.getAnnotationType().asElement().getSimpleName()
+						) );
+			}
+		}
+		else {
+			// it means that the type was marked for unwrapping but unwrapped value type was not found
 			return CollectionHelper.asSet(
-					ConstraintCheckIssue.error(
-							element, annotation, messageKey,
+					ConstraintCheckIssue.warning(
+							element, annotation, "NOT_FOUND_UNWRAPPED_TYPE",
 							annotation.getAnnotationType().asElement().getSimpleName()
 					) );
 		}
@@ -91,30 +103,22 @@ private boolean isAnnotationAllowedForType(AnnotationMirror annotation, TypeMirr
 	}
 
 	private boolean usesUnwrapping(AnnotationMirror annotationMirror, TypeMirror typeMirror) {
+		UnwrapMode mode = annotationApiHelper.determineUnwrapMode( annotationMirror );
+		if ( UnwrapMode.SKIP.equals( mode ) ) {
+			return false;
+		}
+
 		//need to check if this annotation is not on one of the types that are automatically unwrapped:
 		if ( constraintHelper.isSupportedForUnwrappingByDefault( getQualifiedName( typeMirror ) ) ) {
 			return true;
 		}
 
 		//otherwise look for Unwrapping type in payload:
-		return annotationApiHelper.getAnnotationArrayValue( annotationMirror, "payload" ).stream()
-				.map( AnnotationValue::getValue )
-				.map( type -> (TypeMirror) type )
-				.map( typeUtils::asElement )
-				.map( elem -> ( (TypeElement) elem ).getQualifiedName() )
-				.filter( name -> name.toString().equals( "javax.validation.valueextraction.Unwrapping.Unwrap" ) )
-				.findAny().isPresent();
+		return UnwrapMode.UNWRAP.equals( mode );
 	}
 
-	private TypeMirror getUnwrappedType(TypeMirror type) {
-		Optional<TypeMirror> optional = constraintHelper.getUnwrappedToByDefault( getQualifiedName( type ) );
-		if ( optional.isPresent() ) {
-			return optional.get();
-		}
-		else {
-			//TODO: need to find a way to check for unwrapping
-			return type;
-		}
+	private Optional<TypeMirror> getUnwrappedType(TypeMirror type) {
+		return constraintHelper.getUnwrappedToByDefault( getQualifiedName( type ) );
 	}
 
 	private Name getQualifiedName(TypeMirror typeMirror) {

annotation-processor/src/main/java/org/hibernate/validator/ap/internal/util/AnnotationApiHelper.java

@@ -16,6 +16,7 @@
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.Name;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeKind;
@@ -345,4 +346,38 @@ public boolean isClass(TypeMirror typeMirror) {
 		return TypeKind.DECLARED.equals( typeMirror.getKind() ) && ( (DeclaredType) typeMirror ).asElement().getKind().isClass();
 	}
 
+	/**
+	 * Checks the annotation's payload for unwrapping option ({@code javax.validation.valueextraction.Unwrapping.Unwrap},
+	 * {@code javax.validation.valueextraction.Unwrapping.Skip}) of constraint.
+	 *
+	 * @param annotationMirror constraint annotation mirror under check
+	 * @return unwrapping option, if one is present in the annotation payload, {@link UnwrapMode#NONE} otherwise
+	 */
+	public UnwrapMode determineUnwrapMode(AnnotationMirror annotationMirror) {
+		return getAnnotationArrayValue( annotationMirror, "payload" ).stream()
+				.map( AnnotationValue::getValue )
+				.map( type -> (TypeMirror) type )
+				.map( typeUtils::asElement )
+				.map( elem -> ( (TypeElement) elem ).getQualifiedName() )
+				.filter( name -> name.toString().startsWith( "javax.validation.valueextraction.Unwrapping." ) )
+				.map( UnwrapMode::of )
+				.findAny().orElse( UnwrapMode.NONE );
+	}
+
+	public enum UnwrapMode {
+		UNWRAP, SKIP, NONE;
+
+		public static UnwrapMode of(Name qualifiedName) {
+			if ( "javax.validation.valueextraction.Unwrapping.Unwrap".equals( qualifiedName.toString() ) ) {
+				return UNWRAP;
+			}
+			else if ( "javax.validation.valueextraction.Unwrapping.Skip".equals( qualifiedName.toString() ) ) {
+				return SKIP;
+			}
+			else {
+				return NONE;
+			}
+		}
+	}
+
 }

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

@@ -3,6 +3,7 @@
 ONLY_GETTERS_MAY_BE_ANNOTATED=Constraint annotations must not be specified at methods, which are no valid JavaBeans getter methods.
 NOT_SUPPORTED_TYPE=The annotation @{0} is disallowed for this data type.
 NOT_SUPPORTED_RETURN_TYPE=The annotation @{0} is disallowed for the return type of this method.
+NOT_FOUND_UNWRAPPED_TYPE=Constraint was marked for unwrapping but no unwrapped type was registered by Annotation Processor.
 ONLY_CONSTRAINT_ANNOTATIONS_MAY_BE_ANNOTATED=Constraint annotations must not be specified at annotation types, which are no constraint annotation types themselves.
 STATIC_METHODS_MAY_NOT_BE_ANNOTATED=Only non-static methods may be annotated with constraint annotations.
 STATIC_FIELDS_MAY_NOT_BE_ANNOTATED=Only non-static fields may be annotated with constraint annotations.

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

@@ -305,7 +305,8 @@ public void usingIncompatibleUnwrappingCombination() {
 		assertFalse( compilationResult );
 		assertThatDiagnosticsMatch(
 				diagnostics,
-				new DiagnosticExpectation( Kind.ERROR, 28 )
+				new DiagnosticExpectation( Kind.ERROR, 29 ),
+				new DiagnosticExpectation( Kind.WARNING, 35 )
 		);
 	}
 

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

@@ -676,9 +676,13 @@ public void unwrappingConstraints() {
 		assertFalse( compilationResult );
 		assertThatDiagnosticsMatch(
 				diagnostics,
-				new DiagnosticExpectation( Kind.ERROR, 32 ),
-				new DiagnosticExpectation( Kind.ERROR, 35 ),
-				new DiagnosticExpectation( Kind.ERROR, 38 )
+				new DiagnosticExpectation( Kind.ERROR, 33 ),
+				new DiagnosticExpectation( Kind.ERROR, 36 ),
+				new DiagnosticExpectation( Kind.ERROR, 39 ),
+				new DiagnosticExpectation( Kind.ERROR, 42 ),
+				new DiagnosticExpectation( Kind.ERROR, 45 ),
+				new DiagnosticExpectation( Kind.ERROR, 48 ),
+				new DiagnosticExpectation( Kind.WARNING, 54 )
 		);
 	}
 }

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

@@ -13,6 +13,7 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 import java.util.Optional;
+import java.util.OptionalInt;
 
 import javax.validation.Constraint;
 import javax.validation.ConstraintValidator;
@@ -26,15 +27,15 @@
 public class InvalidUnwrappingCombination {
 
 	@OptionalConstraint(payload = { Unwrapping.Unwrap.class, Unwrapping.Skip.class })
-	private final Optional<Integer> number;
+	private final OptionalInt number;
 
 	@OptionalConstraint(payload = { Unwrapping.Skip.class })
 	private final Optional<Integer> number2;
 
 	@OptionalConstraint(payload = { Unwrapping.Unwrap.class })
 	private final Optional<Integer> number3;
 
-	public InvalidUnwrappingCombination(Optional<Integer> number, Optional<Integer> number2, Optional<Integer> number3) {
+	public InvalidUnwrappingCombination(OptionalInt number, Optional<Integer> number2, Optional<Integer> number3) {
 		this.number = number;
 		this.number2 = number2;
 		this.number3 = number3;

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

@@ -12,6 +12,7 @@
 
 import javax.validation.constraints.Min;
 import javax.validation.constraints.Past;
+import javax.validation.valueextraction.Unwrapping;
 
 public class UnwrappingConstraints {
 
@@ -37,4 +38,19 @@ public class UnwrappingConstraints {
 
 	@Past
 	public OptionalDouble badOptionalDouble;
+
+	@Min(value = 10, payload = Unwrapping.Skip.class)
+	public OptionalInt skipOptionalInt;
+
+	@Min(value = 10, payload = Unwrapping.Skip.class)
+	public OptionalLong skipOptionalLong;
+
+	@Min(value = 10, payload = Unwrapping.Skip.class)
+	public OptionalDouble skipOptionalDouble;
+
+	/**
+	 * Warning
+	 */
+	@Min(value = 10, payload = Unwrapping.Unwrap.class)
+	public Integer unwrappedInteger;
 }