HV-618 Making sure overridden methods are detected correctly, also if…

… generic types are involved

engine/pom.xml

@@ -49,6 +49,10 @@
             <groupId>org.jboss.logging</groupId>
             <artifactId>jboss-logging</artifactId>
         </dependency>
+        <dependency>
+            <groupId>com.fasterxml</groupId>
+            <artifactId>classmate</artifactId>
+        </dependency>
 
         <!--
         Optional dependencies

engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ExecutableMetaData.java

@@ -38,7 +38,6 @@
 import org.hibernate.validator.internal.metadata.raw.ConstrainedExecutable;
 import org.hibernate.validator.internal.metadata.raw.ConstrainedParameter;
 import org.hibernate.validator.internal.metadata.raw.ExecutableElement;
-import org.hibernate.validator.internal.util.ReflectionHelper;
 import org.hibernate.validator.internal.util.logging.Log;
 import org.hibernate.validator.internal.util.logging.LoggerFactory;
 
@@ -152,12 +151,17 @@ public Builder(ConstrainedExecutable constrainedExecutable, ConstraintHelper con
 
 		@Override
 		public boolean accepts(ConstrainedElement constrainedElement) {
-			return
-					kind == constrainedElement.getKind() &&
-							ReflectionHelper.haveSameSignature(
-									location.getExecutableElement(),
-									( (ConstrainedExecutable) constrainedElement ).getLocation().getExecutableElement()
-							);
+			if ( kind != constrainedElement.getKind() ) {
+				return false;
+			}
+
+			ExecutableElement executableElement = ( (ConstrainedExecutable) constrainedElement ).getLocation()
+					.getExecutableElement();
+
+			//does one of the executables override the other one?
+			return location.getExecutableElement().overrides( executableElement ) || executableElement.overrides(
+					location.getExecutableElement()
+			);
 		}
 
 		@Override
@@ -172,6 +176,7 @@ public void add(ConstrainedElement constrainedElement) {
 
 		@Override
 		public ExecutableMetaData build() {
+
 			ExecutableElement executableElement = location.getExecutableElement();
 
 			return new ExecutableMetaData(

engine/src/main/java/org/hibernate/validator/internal/metadata/raw/ExecutableElement.java

@@ -27,6 +27,15 @@
 import java.util.List;
 import javax.validation.ParameterNameProvider;
 
+import com.fasterxml.classmate.Filter;
+import com.fasterxml.classmate.MemberResolver;
+import com.fasterxml.classmate.ResolvedType;
+import com.fasterxml.classmate.ResolvedTypeWithMembers;
+import com.fasterxml.classmate.TypeResolver;
+import com.fasterxml.classmate.members.RawMethod;
+import com.fasterxml.classmate.members.ResolvedMethod;
+
+import org.hibernate.validator.internal.util.Contracts;
 import org.hibernate.validator.internal.util.ReflectionHelper;
 
 import static org.hibernate.validator.internal.util.CollectionHelper.newArrayList;
@@ -38,6 +47,11 @@
  */
 public abstract class ExecutableElement {
 
+	/**
+	 * Used for resolving type parameters. Thread-safe.
+	 */
+	private static TypeResolver typeResolver = new TypeResolver();
+
 	public static ExecutableElement forConstructor(Constructor<?> constructor) {
 		return new ConstructorElement( constructor );
 	}
@@ -95,6 +109,70 @@ public String getIdentifier() {
 		return getSimpleName() + Arrays.toString( getParameterTypes() );
 	}
 
+	/**
+	 * Checks, whether the represented method overrides the given method.
+	 *
+	 * @param other The method to test.
+	 *
+	 * @return {@code true} If this methods overrides the passed method,
+	 *         {@code false} otherwise.
+	 */
+	public boolean overrides(ExecutableElement other) {
+
+		Contracts.assertValueNotNull( other, "other" );
+
+		if ( !getMember().getName().equals( other.getMember().getName() ) ) {
+			return false;
+		}
+
+		if ( getParameterTypes().length != other.getParameterTypes().length ) {
+			return false;
+		}
+
+		if ( !other.getMember().getDeclaringClass().isAssignableFrom( getMember().getDeclaringClass() ) ) {
+			return false;
+		}
+
+		//constructors never override another constructor
+		if ( getMember() instanceof Constructor || other.getMember() instanceof Constructor ) {
+			return false;
+		}
+
+		return parametersResolveToSameTypes( (Method) getMember(), (Method) other.getMember() );
+	}
+
+	private boolean parametersResolveToSameTypes(Method subTypeMethod, Method superTypeMethod) {
+		if ( subTypeMethod.getParameterTypes().length == 0 ) {
+			return true;
+		}
+
+		ResolvedType resolvedSubType = typeResolver.resolve( subTypeMethod.getDeclaringClass() );
+
+		MemberResolver memberResolver = new MemberResolver( typeResolver );
+		memberResolver.setMethodFilter( new SimpleMethodFilter( subTypeMethod, superTypeMethod ) );
+		ResolvedTypeWithMembers typeWithMembers = memberResolver.resolve( resolvedSubType, null, null );
+
+		ResolvedMethod[] resolvedMethods = typeWithMembers.getMemberMethods();
+
+		// The ClassMate doc says that overridden methods are flattened to one
+		// resolved method. But that is the case only for methods without any
+		// generic parameters.
+		if ( resolvedMethods.length == 1 ) {
+			return true;
+		}
+
+		// For methods with generic parameters I have to compare the argument
+		// types (which are resolved) of the two filtered member methods.
+		for ( int i = 0; i < resolvedMethods[0].getArgumentCount(); i++ ) {
+
+			if ( !resolvedMethods[0].getArgumentType( i ).equals( resolvedMethods[1].getArgumentType( i ) ) ) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	private static class ConstructorElement extends ExecutableElement {
 
 		private final Constructor<?> constructor;
@@ -223,4 +301,24 @@ public String toString() {
 			return method.toGenericString();
 		}
 	}
+
+	/**
+	 * A filter implementation filtering methods matching given methods.
+	 *
+	 * @author Gunnar Morling
+	 */
+	private static class SimpleMethodFilter implements Filter<RawMethod> {
+		private final Method method1;
+		private final Method method2;
+
+		private SimpleMethodFilter(Method method1, Method method2) {
+			this.method1 = method1;
+			this.method2 = method2;
+		}
+
+		@Override
+		public boolean include(RawMethod element) {
+			return element.getRawMember().equals( method1 ) || element.getRawMember().equals( method2 );
+		}
+	}
 }

engine/src/main/java/org/hibernate/validator/internal/util/ReflectionHelper.java

@@ -590,25 +590,6 @@ private static <T> T run(PrivilegedAction<T> action) {
 		return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run();
 	}
 
-	/**
-	 * Checks, whether the given methods have the same signature, which is the
-	 * case if they have the same name, parameter count and types.
-	 *
-	 * @param method1 A first method.
-	 * @param method2 A second method.
-	 *
-	 * @return True, if the methods have the same signature, false otherwise.
-	 */
-	public static boolean haveSameSignature(ExecutableElement method1, ExecutableElement method2) {
-
-		Contracts.assertValueNotNull( method1, "method1" );
-		Contracts.assertValueNotNull( method2, "method2" );
-
-		return
-				method1.getMember().getName().equals( method2.getMember().getName() ) &&
-						Arrays.equals( method1.getGenericParameterTypes(), method2.getGenericParameterTypes() );
-	}
-
 	/**
 	 * Returns the auto-boxed type of a primitive type.
 	 *