Ignoring synthetic methods to always return the nearest method with the most specific return type from Reflection2.method

core/src/main/java/org/jclouds/reflect/Reflection2.java

@@ -114,9 +114,13 @@ public class Reflection2 {
 
    /**
     * returns an {@link Invokable} object that reflects a method present in the {@link TypeToken} type.
+    * If there are multiple methods of the same name and parameter list, returns the method in the nearest
+    * ancestor with the most specific return type (see {@link Class#getDeclaredMethod}).
     * 
     * @param ownerType
     *           corresponds to {@link Invokable#getOwnerType()}
+    * @param name
+    *           name of the method to be returned
     * @param parameterTypes
     *           corresponds to {@link Method#getParameterTypes()}
     * 
@@ -224,9 +228,17 @@ public class Reflection2 {
          .newBuilder().build(new CacheLoader<TypeTokenNameAndParameterTypes, Invokable<?, ?>>() {
             public Invokable<?, ?> load(final TypeTokenNameAndParameterTypes key) {
                Set<Invokable<?, ?>> methods = get(methodsForTypeToken, key.type);
+               /*
+                * There may be multiple methods, even on the most immediate ancestor,
+                * of a method with the required name and parameter set. This will occur 
+                * if the method overrides one declared in a parent class with a less specific
+                * return type. These bridge methods inserted by the compiler will be marked
+                * as "synthetic".
+                */
                Optional<Invokable<?, ?>> method = tryFind(methods, new Predicate<Invokable<?, ?>>() {
                   public boolean apply(Invokable<?, ?> input) {
-                     return Objects.equal(input.getName(), key.name)
+                     // Invokable doesn't expose Method#isBridge
+                     return !input.isSynthetic() && Objects.equal(input.getName(), key.name)
                            && Objects.equal(toClasses(input.getParameters()), key.parameterTypes);
                   }
                });
@@ -265,6 +277,8 @@ public class Reflection2 {
 
    /**
     * this gets all declared methods, not just public ones. makes them accessible. Does not include Object methods.
+    * Invokables for a type are ordered so all invokables on a subtype are always listed before invokables on a
+    * supertype (see {@link TypeToken#getTypes()}).
     */
    private static LoadingCache<TypeToken<?>, Set<Invokable<?, ?>>> methodsForTypeToken = CacheBuilder
          .newBuilder().build(new CacheLoader<TypeToken<?>, Set<Invokable<?, ?>>>() {

core/src/test/java/org/jclouds/reflect/Reflection2Test.java

@@ -19,12 +19,17 @@
 package org.jclouds.reflect;
 
 import static com.google.common.base.Functions.toStringFunction;
+import static com.google.common.base.Throwables.propagate;
+import static java.lang.String.format;
 import static org.jclouds.reflect.Reflection2.constructors;
 import static org.jclouds.reflect.Reflection2.method;
 import static org.jclouds.reflect.Reflection2.methods;
 import static org.jclouds.reflect.Reflection2.typeToken;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.fail;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.SortedSet;
@@ -32,6 +37,9 @@ import java.util.SortedSet;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Function;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.reflect.Invokable;
@@ -113,9 +121,97 @@ public class Reflection2Test {
                   .addAll(setMethods).build());
    }
 
+   public void testOverriddenMethodWithNarrowedReturnType() throws NoSuchMethodException {
+       LoadingCache<TypeToken<?>, Set<Invokable<?, ?>>> methodsForTypeTokenBackup =
+           getStaticField(Reflection2.class, "methodsForTypeToken");
+       // expecting two methods: the declared "method" and the bridge version with return type Set
+       final Method[] methods = ChildOverridesAndNarrowsMethod.class.getDeclaredMethods();
+       try {
+           /*
+            * Force Reflection2.methodsForTypeToken to reflect the fact that the declared methods
+            * of a class are not returned in any particular order.
+            */
+           setStaticField(Reflection2.class, "methodsForTypeToken", CacheBuilder.newBuilder().build(
+                   new CacheLoader<TypeToken<?>, Set<Invokable<?, ?>>>() {
+                          @Override
+                           public Set<Invokable<?, ?>> load(TypeToken<?> key) throws Exception {
+                              if (!key.equals(TypeToken.of(ChildOverridesAndNarrowsMethod.class))) {
+                                  fail(format("expected only key %s to be requested, but was %s", 
+                                          TypeToken.of(ChildOverridesAndNarrowsMethod.class), key));
+                              }
+                              return ImmutableSet.<Invokable<?, ?>>of(Invokable.from(methods[0]), Invokable.from(methods[1]));
+                           }
+                   }));
+           // getMethod returns the method with the *narrowest* return type if one exists
+           assertEquals(Reflection2.method(ChildOverridesAndNarrowsMethod.class, "method"), 
+                   Invokable.from(ChildOverridesAndNarrowsMethod.class.getMethod("method")));
+
+           // now the opposite order
+           Reflection2Test.<LoadingCache<?, ?>>
+               getStaticField(Reflection2.class, "methodForParams").invalidateAll();
+           setStaticField(Reflection2.class, "methodsForTypeToken", CacheBuilder.newBuilder().build(
+                   new CacheLoader<TypeToken<?>, Set<Invokable<?, ?>>>() {
+                          @Override
+                           public Set<Invokable<?, ?>> load(TypeToken<?> key) throws Exception {
+                              if (!key.equals(TypeToken.of(ChildOverridesAndNarrowsMethod.class))) {
+                                  fail(format("expected only key %s to be requested, but was %s", 
+                                          TypeToken.of(ChildOverridesAndNarrowsMethod.class), key));
+                              }
+                              return ImmutableSet.<Invokable<?, ?>>of(Invokable.from(methods[1]), Invokable.from(methods[0]));
+                           }
+                   }));
+           // getMethod returns the method with the *narrowest* return type if one exists
+           assertEquals(Reflection2.method(ChildOverridesAndNarrowsMethod.class, "method"), 
+                   Invokable.from(ChildOverridesAndNarrowsMethod.class.getMethod("method")));
+       } finally {
+           setStaticField(Reflection2.class, "methodsForTypeToken", methodsForTypeTokenBackup);
+       }
+   }
+
+   @SuppressWarnings("unchecked")
+   private static <T> T getStaticField(Class<?> declaringClass, String fieldName) {
+       try {
+            Field field = declaringClass.getDeclaredField(fieldName);
+            field.setAccessible(true);
+            // static field
+            return (T) field.get(null);
+        } catch (NoSuchFieldException exception) {
+            throw propagate(exception);
+        } catch (IllegalAccessException exception) {
+            throw propagate(exception);
+        }
+   }
+
+   private static void setStaticField(Class<?> declaringClass, String fieldName, Object value) {
+       try {
+            Field field = declaringClass.getDeclaredField(fieldName);
+            field.setAccessible(true);
+            // static field
+            field.set(null, value);
+        } catch (NoSuchFieldException exception) {
+            throw propagate(exception);
+        } catch (IllegalAccessException exception) {
+            throw propagate(exception);
+        }
+   }
+
    static final Function<Invokable<?, ?>, String> invokableToName = new Function<Invokable<?, ?>, String>() {
       public String apply(Invokable<?, ?> input) {
          return input.getName();
       }
    };
+
+   private static class ParentWithMethod {
+       @SuppressWarnings("unused")
+       public Set<Object> method() {
+           return null;
+       }
+   }
+
+   private static class ChildOverridesAndNarrowsMethod extends ParentWithMethod {
+       @Override
+       public SortedSet<Object> method() {
+           return null;
+       }
+   }
 }