ReflectiveMethodExecutor skips interface search (plus related polishing)

spring-context/src/main/java/org/springframework/context/event/GenericApplicationListenerAdapter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -86,17 +86,11 @@ public class GenericApplicationListenerAdapter implements GenericApplicationList
 		return (this.delegate instanceof Ordered ? ((Ordered) this.delegate).getOrder() : Ordered.LOWEST_PRECEDENCE);
 	}
 
-	@Nullable
-	static ResolvableType resolveDeclaredEventType(Class<?> listenerType) {
-		ResolvableType resolvableType = ResolvableType.forClass(listenerType).as(ApplicationListener.class);
-		return (resolvableType.hasGenerics() ? resolvableType.getGeneric() : null);
-	}
 
 	@Nullable
 	private static ResolvableType resolveDeclaredEventType(ApplicationListener<ApplicationEvent> listener) {
 		ResolvableType declaredEventType = resolveDeclaredEventType(listener.getClass());
-		if (declaredEventType == null || declaredEventType.isAssignableFrom(
-				ResolvableType.forClass(ApplicationEvent.class))) {
+		if (declaredEventType == null || declaredEventType.isAssignableFrom(ApplicationEvent.class)) {
 			Class<?> targetClass = AopUtils.getTargetClass(listener);
 			if (targetClass != listener.getClass()) {
 				declaredEventType = resolveDeclaredEventType(targetClass);
@@ -105,4 +99,10 @@ public class GenericApplicationListenerAdapter implements GenericApplicationList
 		return declaredEventType;
 	}
 
+	@Nullable
+	static ResolvableType resolveDeclaredEventType(Class<?> listenerType) {
+		ResolvableType resolvableType = ResolvableType.forClass(listenerType).as(ApplicationListener.class);
+		return (resolvableType.hasGenerics() ? resolvableType.getGeneric() : null);
+	}
+
 }

spring-core/src/main/java/org/springframework/util/ClassUtils.java

@@ -227,7 +227,7 @@ public abstract class ClassUtils {
 	 * @param name the name of the Class
 	 * @param classLoader the class loader to use
 	 * (may be {@code null}, which indicates the default class loader)
-	 * @return Class instance for the supplied name
+	 * @return a class instance for the supplied name
 	 * @throws ClassNotFoundException if the class was not found
 	 * @throws LinkageError if the class file could not be loaded
 	 * @see Class#forName(String, boolean, ClassLoader)
@@ -298,7 +298,7 @@ public abstract class ClassUtils {
 	 * @param className the name of the Class
 	 * @param classLoader the class loader to use
 	 * (may be {@code null}, which indicates the default class loader)
-	 * @return Class instance for the supplied name
+	 * @return a class instance for the supplied name
 	 * @throws IllegalArgumentException if the class name was not resolvable
 	 * (that is, the class could not be found or the class file could not be loaded)
 	 * @see #forName(String, ClassLoader)
@@ -868,7 +868,7 @@ public abstract class ClassUtils {
 	public static Class<?> getUserClass(Class<?> clazz) {
 		if (clazz.getName().contains(CGLIB_CLASS_SEPARATOR)) {
 			Class<?> superclass = clazz.getSuperclass();
-			if (superclass != null && Object.class != superclass) {
+			if (superclass != null && superclass != Object.class) {
 				return superclass;
 			}
 		}
@@ -1227,10 +1227,10 @@ public abstract class ClassUtils {
 	 * access (e.g. calls to {@code Class#getDeclaredMethods} etc, this implementation
 	 * will fall back to returning the originally provided method.
 	 * @param method the method to be invoked, which may come from an interface
-	 * @param targetClass the target class for the current invocation.
-	 * May be {@code null} or may not even implement the method.
+	 * @param targetClass the target class for the current invocation
+	 * (may be {@code null} or may not even implement the method)
 	 * @return the specific target method, or the original method if the
-	 * {@code targetClass} doesn't implement it or is {@code null}
+	 * {@code targetClass} does not implement it
 	 */
 	public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {
 		if (targetClass != null && targetClass != method.getDeclaringClass() && isOverridable(method, targetClass)) {

spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java

@@ -182,8 +182,7 @@ public class MethodReference extends SpelNodeImpl {
 			@Nullable TypeDescriptor target, List<TypeDescriptor> argumentTypes) {
 
 		List<MethodResolver> methodResolvers = evaluationContext.getMethodResolvers();
-		if (methodResolvers.size() != 1 ||
-				!(methodResolvers.get(0) instanceof ReflectiveMethodResolver)) {
+		if (methodResolvers.size() != 1 || !(methodResolvers.get(0) instanceof ReflectiveMethodResolver)) {
 			// Not a default ReflectiveMethodResolver - don't know whether caching is valid
 			return null;
 		}
@@ -249,7 +248,7 @@ public class MethodReference extends SpelNodeImpl {
 			Method method = ((ReflectiveMethodExecutor) executorToCheck.get()).getMethod();
 			String descriptor = CodeFlow.toDescriptor(method.getReturnType());
 			if (this.nullSafe && CodeFlow.isPrimitive(descriptor)) {
-				originalPrimitiveExitTypeDescriptor = descriptor;
+				this.originalPrimitiveExitTypeDescriptor = descriptor;
 				this.exitTypeDescriptor = CodeFlow.toBoxedDescriptor(descriptor);
 			}
 			else {
@@ -301,7 +300,7 @@ public class MethodReference extends SpelNodeImpl {
 
 		return true;
 	}
-	
+
 	@Override
 	public void generateCode(MethodVisitor mv, CodeFlow cf) {
 		CachedMethodExecutor executorToCheck = this.cachedExecutor;
@@ -332,7 +331,7 @@ public class MethodReference extends SpelNodeImpl {
 			// Something on the stack when nothing is needed
 			mv.visitInsn(POP);
 		}
-		
+
 		if (CodeFlow.isPrimitive(descriptor)) {
 			CodeFlow.insertBoxIfNecessary(mv, descriptor.charAt(0));
 		}

spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -29,6 +29,8 @@ import org.springframework.lang.Nullable;
 import org.springframework.util.ReflectionUtils;
 
 /**
+ * {@link MethodExecutor} that works via reflection.
+ *
  * @author Andy Clement
  * @author Juergen Hoeller
  * @since 3.0
@@ -48,6 +50,10 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 	private boolean argumentConversionOccurred = false;
 
 
+	/**
+	 * Create a new executor for the given method.
+	 * @param method the method to invoke
+	 */
 	public ReflectiveMethodExecutor(Method method) {
 		this.method = method;
 		if (method.isVarArgs()) {
@@ -60,6 +66,9 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 	}
 
 
+	/**
+	 * Return the original method that this executor has been configured for.
+	 */
 	public Method getMethod() {
 		return this.method;
 	}
@@ -68,21 +77,22 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 	 * Find the first public class in the methods declaring class hierarchy that declares this method.
 	 * Sometimes the reflective method discovery logic finds a suitable method that can easily be
 	 * called via reflection but cannot be called from generated code when compiling the expression
-	 * because of visibility restrictions. For example if a non public class overrides toString(), this
-	 * helper method will walk up the type hierarchy to find the first public type that declares the
-	 * method (if there is one!). For toString() it may walk as far as Object.
+	 * because of visibility restrictions. For example if a non-public class overrides toString(),
+	 * this helper method will walk up the type hierarchy to find the first public type that declares
+	 * the method (if there is one!). For toString() it may walk as far as Object.
 	 */
 	@Nullable
 	public Class<?> getPublicDeclaringClass() {
 		if (!this.computedPublicDeclaringClass) {
-			this.publicDeclaringClass = discoverPublicClass(this.method, this.method.getDeclaringClass());
+			this.publicDeclaringClass =
+					discoverPublicDeclaringClass(this.method, this.method.getDeclaringClass());
 			this.computedPublicDeclaringClass = true;
 		}
 		return this.publicDeclaringClass;
 	}
 
 	@Nullable
-	private Class<?> discoverPublicClass(Method method, Class<?> clazz) {
+	private Class<?> discoverPublicDeclaringClass(Method method, Class<?> clazz) {
 		if (Modifier.isPublic(clazz.getModifiers())) {
 			try {
 				clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());
@@ -92,12 +102,8 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 				// Continue below...
 			}
 		}
-		Class<?>[] ifcs = clazz.getInterfaces();
-		for (Class<?> ifc: ifcs) {
-			discoverPublicClass(method, ifc);
-		}
 		if (clazz.getSuperclass() != null) {
-			return discoverPublicClass(method, clazz.getSuperclass());
+			return discoverPublicDeclaringClass(method, clazz.getSuperclass());
 		}
 		return null;
 	}

spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
 
 package org.springframework.expression.spel;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.expression.Expression;
@@ -36,17 +35,11 @@ public class CachedMethodExecutorTests {
 
 	private final ExpressionParser parser = new SpelExpressionParser();
 
-	private StandardEvaluationContext context;
-
-
-	@Before
-	public void setUp() throws Exception {
-		this.context = new StandardEvaluationContext(new RootObject());
-	}
+	private final StandardEvaluationContext context = new StandardEvaluationContext(new RootObject());
 
 
 	@Test
-	public void testCachedExecutionForParameters() throws Exception {
+	public void testCachedExecutionForParameters() {
 		Expression expression = this.parser.parseExpression("echo(#var)");
 
 		assertMethodExecution(expression, 42, "int: 42");
@@ -56,7 +49,7 @@ public class CachedMethodExecutorTests {
 	}
 
 	@Test
-	public void testCachedExecutionForTarget() throws Exception {
+	public void testCachedExecutionForTarget() {
 		Expression expression = this.parser.parseExpression("#var.echo(42)");
 
 		assertMethodExecution(expression, new RootObject(), "int: 42");
@@ -76,7 +69,6 @@ public class CachedMethodExecutorTests {
 		public String echo(String value) {
 			return "String: " + value;
 		}
-
 	}
 
 	public static class RootObject extends BaseObject {
@@ -84,7 +76,6 @@ public class CachedMethodExecutorTests {
 		public String echo(int value) {
 			return "int: " + value;
 		}
-
 	}
 
 }