HV-1180 added check of interface inheritance in group sequences

hibernate · Dec 12, 2016 · 5e5da72 · 5e5da72
1 parent edb25d6
commit 5e5da72
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 7 deletions.
diff --git a/.../main/java/org/hibernate/validator/ap/checks/annotationparameters/GroupSequenceCheck.java b/.../main/java/org/hibernate/validator/ap/checks/annotationparameters/GroupSequenceCheck.java
@@ -37,7 +37,7 @@
  */
 public class GroupSequenceCheck extends AnnotationParametersAbstractCheck {
 
-	private Types typeUtils;
+	private final Types typeUtils;
 	private final ConstraintHelper constraintHelper;
 
 	public GroupSequenceCheck(AnnotationApiHelper annotationApiHelper, Types typeUtils, ConstraintHelper constraintHelper) {
@@ -121,6 +121,11 @@ else if ( !annotationApiHelper.isInterface( typeMirror ) ) {
 	 */
 	private ConstraintCheckIssue checkForCyclicDefinition(Set<TypeMirror> processedTypes, TypeMirror currentTypeMirror,
 			TypeElement originalElement, AnnotationMirror annotation) {
+		// if passed currentTypeMirror is not a class/interface than - we will not do anything about it, as
+		// such error was already processed by another part of code and if do not ignore them then Types#asElement will fail
+		if ( !TypeKind.DECLARED.equals( currentTypeMirror.getKind() ) ) {
+			return null;
+		}
 		if ( processedTypes.contains( currentTypeMirror ) ) {
 			if ( !redefinesDefaultGroupSequence( originalElement, currentTypeMirror ) ) {
 				return ConstraintCheckIssue.error( originalElement, annotation, "INVALID_GROUP_SEQUENCE_VALUE_CYCLIC_DEFINITION" );
@@ -131,9 +136,8 @@ private ConstraintCheckIssue checkForCyclicDefinition(Set<TypeMirror> processedT
 		}
 		else {
 			processedTypes.add( currentTypeMirror );
-
-			AnnotationMirror groupSequenceMirror = getGroupSequence( currentTypeMirror );
-			List<? extends AnnotationValue> annotationValue = annotationApiHelper.getAnnotationArrayValue( groupSequenceMirror, "value" );
+			// check if there is a @GroupSequence annotation on a currentTypeMirror and check it values if present
+			List<? extends AnnotationValue> annotationValue = annotationApiHelper.getAnnotationArrayValue( getGroupSequence( currentTypeMirror ), "value" );
 			if ( annotationValue != null ) {
 				for ( AnnotationValue value : annotationValue ) {
 					TypeMirror groupTypeMirror = (TypeMirror) value.getValue();
@@ -143,6 +147,13 @@ private ConstraintCheckIssue checkForCyclicDefinition(Set<TypeMirror> processedT
 					}
 				}
 			}
+			// check if currentTypeMirror extends any other interfaces and if so - check them if they are not in the sequence already
+			for ( TypeMirror extendedInterfaceTypeMirror : ( (TypeElement) typeUtils.asElement( currentTypeMirror ) ).getInterfaces() ) {
+				ConstraintCheckIssue issue = checkForCyclicDefinition( processedTypes, extendedInterfaceTypeMirror, originalElement, annotation );
+				if ( issue != null ) {
+					return issue;
+				}
+			}
 		}
 		return null;
 	}

diff --git a/...ation-processor/src/test/java/org/hibernate/validator/ap/GroupSequenceValidationTest.java b/...ation-processor/src/test/java/org/hibernate/validator/ap/GroupSequenceValidationTest.java
@@ -61,7 +61,9 @@ public void testInvalidGroupSequenceParameter() {
 				new DiagnosticExpectation( Kind.ERROR, 113 ),
 				new DiagnosticExpectation( Kind.ERROR, 128 ),
 				new DiagnosticExpectation( Kind.ERROR, 132 ),
-				new DiagnosticExpectation( Kind.ERROR, 136 )
+				new DiagnosticExpectation( Kind.ERROR, 136 ),
+				new DiagnosticExpectation( Kind.ERROR, 151 ),
+				new DiagnosticExpectation( Kind.ERROR, 174 )
 		);
 	}
 

diff --git a/...hibernate/validator/ap/testmodel/annotationparameters/InvalidGroupSequenceParameters.java b/...hibernate/validator/ap/testmodel/annotationparameters/InvalidGroupSequenceParameters.java
@@ -140,8 +140,6 @@ public interface GroupSequence3 {
 
 	/**
 	 * Case 8: Cyclic definition due to group inheritance - incorrect
-	 *
-	 * XXX: currently, it does not throw an error
 	 */
 	public static class Case8 {
 		public interface Group1 extends Group2 {
@@ -153,6 +151,29 @@ public interface Group2 {
 		@GroupSequence(value = { Group1.class, Group2.class })
 		public interface GroupSequence1 {
 		}
+	}
+
+	/**
+	 * Case 9: Cyclic definition due to group inheritance and group sequence inheritance - incorrect
+	 */
+	public static class Case9 {
+		public interface Group1 extends Group2 {
+		}
+
+		public interface Group2 {
+		}
+
+		@GroupSequence(value = Group1.class)
+		public interface GroupSequence1 {
+		}
+
+		@GroupSequence(value = GroupSequence1.class)
+		public interface GroupSequence2 {
+		}
+
+		@GroupSequence(value = { Group2.class, GroupSequence2.class })
+		public interface GroupSequence3 {
+		}
 
 	}
 }