Two new features for ProxyBuilder:

  - generate a proxy class directly (with no instance). This is for Mockito. I'm not 100% convinced on this one yet.
  - generate implemented interfaces.

 Also fix some bugs with covariant return types. We had bugs when two methods had the same name and parameters but different return types.

2 files changed, 294 insertions, 43 deletions

diff --git a/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java b/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
index 6a5e368..639b3dc 100644
--- a/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
+++ b/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
@@ -42,6 +42,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CopyOnWriteArraySet;
 
 /**
  * Creates dynamic proxies of concrete classes.
@@ -131,6 +132,7 @@ public final class ProxyBuilder<T> {
     private File dexCache;
     private Class<?>[] constructorArgTypes = new Class[0];
     private Object[] constructorArgValues = new Object[0];
+    private Set<Class<?>> interfaces = new HashSet<Class<?>>();
 
     private ProxyBuilder(Class<T> clazz) {
         baseClass = clazz;
@@ -164,6 +166,16 @@ public final class ProxyBuilder<T> {
         this.dexCache = dexCache;
         return this;
     }
+    
+    public ProxyBuilder<T> implementing(Class<?>... interfaces) {
+        for (Class<?> i : interfaces) {
+            if (!i.isInterface()) {
+                throw new IllegalArgumentException("Not an interface: " + i.getName());
+            }
+            this.interfaces.add(i);
+        }
+        return this;
+    }
 
     public ProxyBuilder<T> constructorArgValues(Object... constructorArgValues) {
         this.constructorArgValues = constructorArgValues;
@@ -190,12 +202,12 @@ public final class ProxyBuilder<T> {
         check(handler != null, "handler == null");
         check(constructorArgTypes.length == constructorArgValues.length,
                 "constructorArgValues.length != constructorArgTypes.length");
-        Class<? extends T> proxyClass = getProxyClass();
+        Class<? extends T> proxyClass = buildProxyClass();
         Constructor<? extends T> constructor;
         try {
             constructor = proxyClass.getConstructor(constructorArgTypes);
         } catch (NoSuchMethodException e) {
-            throw new IllegalArgumentException("No constructor for " + proxyClass.getName()
+            throw new IllegalArgumentException("No constructor for " + baseClass.getName()
                     + " with parameter types " + Arrays.toString(constructorArgTypes));
         }
         T result;
@@ -215,11 +227,15 @@ public final class ProxyBuilder<T> {
         return result;
     }
 
-    private Class<? extends T> getProxyClass() throws IOException {
+    // TODO: test coverage for this
+    // TODO: documentation for this
+    public Class<? extends T> buildProxyClass() throws IOException {
         // try the cache to see if we've generated this one before
         @SuppressWarnings("unchecked") // we only populate the map with matching types
         Class<? extends T> proxyClass = (Class) generatedProxyClasses.get(baseClass);
-        if (proxyClass != null && proxyClass.getClassLoader().getParent() == parentClassLoader) {
+        if (proxyClass != null
+                && proxyClass.getClassLoader().getParent() == parentClassLoader
+                && interfaces.equals(asSet(proxyClass.getInterfaces()))) {
             return proxyClass; // cache hit!
         }
 
@@ -229,15 +245,17 @@ public final class ProxyBuilder<T> {
         TypeId<? extends T> generatedType = TypeId.get("L" + generatedName + ";");
         TypeId<T> superType = TypeId.get(baseClass);
         generateConstructorsAndFields(dexMaker, generatedType, superType, baseClass);
-        Method[] methodsToProxy = getMethodsToProxy(baseClass);
+        Method[] methodsToProxy = getMethodsToProxyRecursive();
         generateCodeForAllMethods(dexMaker, generatedType, methodsToProxy, superType);
-        dexMaker.declare(generatedType, generatedName + ".generated", PUBLIC, superType);
+        dexMaker.declare(generatedType, generatedName + ".generated", PUBLIC, superType,
+                getInterfacesAsTypeIds());
         ClassLoader classLoader = dexMaker.generateAndLoad(parentClassLoader, dexCache);
         try {
             proxyClass = loadClass(classLoader, generatedName);
         } catch (IllegalAccessError e) {
             // Thrown when the base class is not accessible.
-            throw new UnsupportedOperationException("cannot proxy inaccessible classes", e);
+            throw new UnsupportedOperationException(
+                    "cannot proxy inaccessible class " + baseClass, e);
         } catch (ClassNotFoundException e) {
             // Should not be thrown, we're sure to have generated this class.
             throw new AssertionError(e);
@@ -314,6 +332,8 @@ public final class ProxyBuilder<T> {
         }
     }
 
+    // TODO: test coverage for isProxyClass
+    
     /**
      * Returns true if {@code c} is a proxy class created by this builder.
      */
@@ -458,14 +478,14 @@ public final class ProxyBuilder<T> {
             /*
              * And to allow calling the original super method, the following is also generated:
              *
-             *     public int super_doSomething(Bar param0, int param1) {
+             *     public String super$doSomething$java_lang_String(Bar param0, int param1) {
              *          int result = super.doSomething(param0, param1);
              *          return result;
              *     }
              */
-            String superName = "super_" + name;
+            // TODO: don't include a super_ method if the target is abstract!
             MethodId<G, ?> callsSuperMethod = generatedType.getMethod(
-                    resultType, superName, argTypes);
+                    resultType, superMethodName(method), argTypes);
             Code superCode = dexMaker.declare(callsSuperMethod, PUBLIC);
             Local<G> superThis = superCode.getThis(generatedType);
             Local<?>[] superArgs = new Local<?>[argClasses.length];
@@ -498,12 +518,24 @@ public final class ProxyBuilder<T> {
         return temp;
     }
 
-    public static Object callSuper(Object proxy, Method method, Object... args)
-            throws SecurityException, IllegalAccessException,
-            InvocationTargetException, NoSuchMethodException {
-        return proxy.getClass()
-                .getMethod("super_" + method.getName(), method.getParameterTypes())
-                .invoke(proxy, args);
+    public static Object callSuper(Object proxy, Method method, Object... args) throws Throwable {
+        try {
+            return proxy.getClass()
+                    .getMethod(superMethodName(method), method.getParameterTypes())
+                    .invoke(proxy, args);
+        } catch (InvocationTargetException e) {
+            throw e.getCause();
+        }
+    }
+
+    /**
+     * The super method must include the return type, otherwise its ambiguous
+     * for methods with covariant return types.
+     */
+    private static String superMethodName(Method method) {
+        String returnType = method.getReturnType().getName();
+        return "super$" + method.getName() + "$"
+                + returnType.replace('.', '_').replace('[', '_').replace(';', '_');
     }
 
     private static void check(boolean condition, String message) {
@@ -548,28 +580,28 @@ public final class ProxyBuilder<T> {
         return (Constructor<T>[]) clazz.getDeclaredConstructors();
     }
 
+    private TypeId<?>[] getInterfacesAsTypeIds() {
+        TypeId<?>[] result = new TypeId<?>[interfaces.size()];
+        int i = 0;
+        for (Class<?> implemented : interfaces) {
+            result[i++] = TypeId.get(implemented);
+        }
+        return result;
+    }
+
     /**
-     * Gets all {@link Method} objects we can proxy in the hierarchy of the supplied class.
+     * Gets all {@link Method} objects we can proxy in the hierarchy of the
+     * supplied class.
      */
-    private static <T> Method[] getMethodsToProxy(Class<T> clazz) {
+    private Method[] getMethodsToProxyRecursive() {
         Set<MethodSetEntry> methodsToProxy = new HashSet<MethodSetEntry>();
-        for (Class<?> current = clazz; current != null; current = current.getSuperclass()) {
-            for (Method method : current.getDeclaredMethods()) {
-                if ((method.getModifiers() & Modifier.FINAL) != 0) {
-                    // Skip final methods, we can't override them.
-                    continue;
-                }
-                if ((method.getModifiers() & STATIC) != 0) {
-                    // Skip static methods, overriding them has no effect.
-                    continue;
-                }
-                if (method.getName().equals("finalize") && method.getParameterTypes().length == 0) {
-                    // Skip finalize method, it's likely important that it execute as normal.
-                    continue;
-                }
-                methodsToProxy.add(new MethodSetEntry(method));
-            }
+        for (Class<?> c = baseClass; c != null; c = c.getSuperclass()) {
+            getMethodsToProxy(methodsToProxy, c);
+        }
+        for (Class<?> c : interfaces) {
+            getMethodsToProxy(methodsToProxy, c);
         }
+
         Method[] results = new Method[methodsToProxy.size()];
         int i = 0;
         for (MethodSetEntry entry : methodsToProxy) {
@@ -578,6 +610,28 @@ public final class ProxyBuilder<T> {
         return results;
     }
 
+    private void getMethodsToProxy(Set<MethodSetEntry> sink, Class<?> c) {
+        for (Method method : c.getDeclaredMethods()) {
+            if ((method.getModifiers() & Modifier.FINAL) != 0) {
+                // Skip final methods, we can't override them.
+                continue;
+            }
+            if ((method.getModifiers() & STATIC) != 0) {
+                // Skip static methods, overriding them has no effect.
+                continue;
+            }
+            if (method.getName().equals("finalize") && method.getParameterTypes().length == 0) {
+                // Skip finalize method, it's likely important that it execute as normal.
+                continue;
+            }
+            sink.add(new MethodSetEntry(method));
+        }
+        
+        for (Class<?> i : c.getInterfaces()) {
+            getMethodsToProxy(sink, i);
+        }
+    }
+
     private static <T> String getMethodNameForProxyOf(Class<T> clazz) {
         return clazz.getSimpleName() + "_Proxy";
     }
@@ -613,6 +667,10 @@ public final class ProxyBuilder<T> {
         }
     }
 
+    private static <T> Set<T> asSet(T... array) {
+        return new CopyOnWriteArraySet<T>(Arrays.asList(array));
+    }
+
     private static MethodId<?, ?> getUnboxMethodForPrimitive(Class<?> methodReturnType) {
         return PRIMITIVE_TO_UNBOX_METHOD.get(methodReturnType);
     }
diff --git a/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java b/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java
index d613053..1b65ea8 100644
--- a/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java
+++ b/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java
@@ -22,7 +22,10 @@ import java.io.IOException;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.UndeclaredThrowableException;
+import java.util.Arrays;
 import java.util.Random;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicInteger;
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
@@ -164,7 +167,7 @@ public class ProxyBuilderTest extends TestCase {
         assertEquals(false, proxy.equals(proxy));
     }
 
-    public static class AllPrimitiveMethods {
+    public static class AllReturnTypes {
         public boolean getBoolean() { return true; }
         public int getInt() { return 1; }
         public byte getByte() { return 2; }
@@ -173,10 +176,12 @@ public class ProxyBuilderTest extends TestCase {
         public float getFloat() { return 5f; }
         public double getDouble() { return 6.0; }
         public char getChar() { return 'c'; }
+        public int[] getIntArray() { return new int[] { 8, 9 }; }
+        public String[] getStringArray() { return new String[] { "d", "e" }; }
     }
 
-    public void testAllPrimitiveReturnTypes() throws Throwable {
-        AllPrimitiveMethods proxy = proxyFor(AllPrimitiveMethods.class).build();
+    public void testAllReturnTypes() throws Throwable {
+        AllReturnTypes proxy = proxyFor(AllReturnTypes.class).build();
         fakeHandler.setFakeResult(false);
         assertEquals(false, proxy.getBoolean());
         fakeHandler.setFakeResult(8);
@@ -193,9 +198,13 @@ public class ProxyBuilderTest extends TestCase {
         assertEquals(13.0, proxy.getDouble());
         fakeHandler.setFakeResult('z');
         assertEquals('z', proxy.getChar());
+        fakeHandler.setFakeResult(new int[] { -1, -2 });
+        assertEquals("[-1, -2]", Arrays.toString(proxy.getIntArray()));
+        fakeHandler.setFakeResult(new String[] { "x", "y" });
+        assertEquals("[x, y]", Arrays.toString(proxy.getStringArray()));
     }
 
-    public static class PassThroughAllPrimitives {
+    public static class PassThroughAllTypes {
         public boolean getBoolean(boolean input) { return input; }
         public int getInt(int input) { return input; }
         public byte getByte(byte input) { return input; }
@@ -215,8 +224,8 @@ public class ProxyBuilderTest extends TestCase {
         }
     }
 
-    public void testPassThroughWorksForAllPrimitives() throws Exception {
-        PassThroughAllPrimitives proxy = proxyFor(PassThroughAllPrimitives.class)
+    public void testPassThroughWorksForAllTypes() throws Exception {
+        PassThroughAllTypes proxy = proxyFor(PassThroughAllTypes.class)
                 .handler(new InvokeSuperHandler())
                 .build();
         assertEquals(false, proxy.getBoolean(false));
@@ -244,12 +253,12 @@ public class ProxyBuilderTest extends TestCase {
         proxy.getNothing();
     }
 
-    public static class ExtendsAllPrimitiveMethods extends AllPrimitiveMethods {
+    public static class ExtendsAllReturnTypes extends AllReturnTypes {
         public int example() { return 0; }
     }
 
     public void testProxyWorksForSuperclassMethodsAlso() throws Throwable {
-        ExtendsAllPrimitiveMethods proxy = proxyFor(ExtendsAllPrimitiveMethods.class).build();
+        ExtendsAllReturnTypes proxy = proxyFor(ExtendsAllReturnTypes.class).build();
         fakeHandler.setFakeResult(99);
         assertEquals(99, proxy.example());
         assertEquals(99, proxy.getInt());
@@ -342,7 +351,7 @@ public class ProxyBuilderTest extends TestCase {
 
     public void testDefaultProxyHasSuperMethodToAccessOriginal() throws Exception {
         Object objectProxy = proxyFor(Object.class).build();
-        assertNotNull(objectProxy.getClass().getMethod("super_hashCode"));
+        assertNotNull(objectProxy.getClass().getMethod("super$hashCode$int"));
     }
 
     public static class PrintsOddAndValue {
@@ -371,6 +380,31 @@ public class ProxyBuilderTest extends TestCase {
         assertEquals("even 2", proxy.method(2));
         assertEquals("odd 3", proxy.method(3));
     }
+    
+    public void testCallSuperThrows() throws Exception {
+        InvocationHandler handler = new InvocationHandler() {
+            public Object invoke(Object o, Method method, Object[] objects) throws Throwable {
+                return ProxyBuilder.callSuper(o, method, objects);
+            }
+        };
+
+        FooThrows fooThrows = proxyFor(FooThrows.class)
+                .handler(handler)
+                .build();
+
+        try {
+            fooThrows.foo();
+            fail();
+        } catch (IllegalStateException expected) {
+            assertEquals("boom!", expected.getMessage());
+        }
+    }
+    
+    public static class FooThrows {
+        public void foo() {
+            throw new IllegalStateException("boom!");
+        }
+    }
 
     public static class DoubleReturn {
         public double getValue() {
@@ -564,7 +598,166 @@ public class ProxyBuilderTest extends TestCase {
 
         assertTrue(a.getClass() != b.getClass());
     }
+    
+    public void testAbstractClassWithUndeclaredInterfaceMethod() throws Throwable {
+        DeclaresInterface declaresInterface = proxyFor(DeclaresInterface.class)
+                .build();
+        assertEquals("fake result", declaresInterface.call());
+        try {
+            ProxyBuilder.callSuper(declaresInterface, Callable.class.getMethod("call"));
+            fail();
+        } catch (AbstractMethodError expected) {
+        }
+    }
+    
+    public static abstract class DeclaresInterface implements Callable<String> {
+    }
+
+    public void testImplementingInterfaces() throws Throwable {
+        SimpleClass simpleClass = proxyFor(SimpleClass.class)
+                .implementing(Callable.class)
+                .implementing(Comparable.class)
+                .build();
+        assertEquals("fake result", simpleClass.simpleMethod());
+
+        Callable<?> asCallable = (Callable<?>) simpleClass;
+        assertEquals("fake result", asCallable.call());
+
+        Comparable<?> asComparable = (Comparable<?>) simpleClass;
+        fakeHandler.fakeResult = 3;
+        assertEquals(3, asComparable.compareTo(null));
+    }
+
+    public void testCallSuperWithInterfaceMethod() throws Throwable {
+        SimpleClass simpleClass = proxyFor(SimpleClass.class)
+                .implementing(Callable.class)
+                .build();
+        try {
+            ProxyBuilder.callSuper(simpleClass, Callable.class.getMethod("call"));
+            fail();
+        } catch (AbstractMethodError expected) {
+        } catch (NoSuchMethodError expected) {
+        }
+    }
 
+    public void testImplementInterfaceCallingThroughConcreteClass() throws Throwable {
+        InvocationHandler invocationHandler = new InvocationHandler() {
+            public Object invoke(Object o, Method method, Object[] objects) throws Throwable {
+                assertEquals("a", ProxyBuilder.callSuper(o, method, objects));
+                return "b";
+            }
+        };
+        ImplementsCallable proxy = proxyFor(ImplementsCallable.class)
+                .implementing(Callable.class)
+                .handler(invocationHandler)
+                .build();
+        assertEquals("b", proxy.call());
+        assertEquals("a", ProxyBuilder.callSuper(
+                proxy, ImplementsCallable.class.getMethod("call")));
+    }
+    
+    /**
+     * This test is a bit unintuitive because it exercises the synthetic methods
+     * that support covariant return types. Calling 'Object call()' on the
+     * interface bridges to 'String call()', and so the super method appears to
+     * also be proxied.
+     */
+    public void testImplementInterfaceCallingThroughInterface() throws Throwable {
+        final AtomicInteger count = new AtomicInteger();
+
+        InvocationHandler invocationHandler = new InvocationHandler() {
+            public Object invoke(Object o, Method method, Object[] objects) throws Throwable {
+                count.incrementAndGet();
+                return ProxyBuilder.callSuper(o, method, objects);
+            }
+        };
+
+        Callable<?> proxy = proxyFor(ImplementsCallable.class)
+                .implementing(Callable.class)
+                .handler(invocationHandler)
+                .build();
+
+        // the invocation handler is called twice!
+        assertEquals("a", proxy.call());
+        assertEquals(2, count.get());
+
+        // the invocation handler is called, even though this is a callSuper() call!
+        assertEquals("a", ProxyBuilder.callSuper(proxy, Callable.class.getMethod("call")));
+        assertEquals(3, count.get());
+    }
+    
+    public static class ImplementsCallable implements Callable<String> {
+        public String call() throws Exception {
+            return "a";
+        }
+    }
+
+    /**
+     * This test shows that our generated proxies follow the bytecode convention
+     * where methods can have the same name but unrelated return types. This is
+     * different from javac's convention where return types must be assignable
+     * in one direction or the other.
+     */
+    public void testInterfacesSameNamesDifferentReturnTypes() throws Throwable {
+        InvocationHandler handler = new InvocationHandler() {
+            public Object invoke(Object o, Method method, Object[] objects) throws Throwable {
+                if (method.getReturnType() == void.class) {
+                    return null;
+                } else if (method.getReturnType() == String.class) {
+                    return "X";
+                } else if (method.getReturnType() == int.class) {
+                    return 3;
+                } else {
+                    throw new AssertionFailedError();
+                }
+            }
+        };
+        
+        Object o = proxyFor(Object.class)
+                .implementing(FooReturnsVoid.class, FooReturnsString.class, FooReturnsInt.class)
+                .handler(handler)
+                .build();
+        
+        FooReturnsVoid a = (FooReturnsVoid) o;
+        a.foo();
+        
+        FooReturnsString b = (FooReturnsString) o;
+        assertEquals("X", b.foo());
+        
+        FooReturnsInt c = (FooReturnsInt) o;
+        assertEquals(3, c.foo());
+    }
+    
+    public void testInterfacesSameNamesSameReturnType() throws Throwable {
+        Object o = proxyFor(Object.class)
+                .implementing(FooReturnsInt.class, FooReturnsInt2.class)
+                .build();
+        
+        fakeHandler.setFakeResult(3);
+
+        FooReturnsInt a = (FooReturnsInt) o;
+        assertEquals(3, a.foo());
+
+        FooReturnsInt2 b = (FooReturnsInt2) o;
+        assertEquals(3, b.foo());
+    }
+    
+    public interface FooReturnsVoid {
+        void foo();
+    }
+
+    public interface FooReturnsString {
+        String foo();
+    }
+
+    public interface FooReturnsInt {
+        int foo();
+    }
+    
+    public interface FooReturnsInt2 {
+        int foo();
+    }
+    
     private ClassLoader newPathClassLoader() throws Exception {
         return (ClassLoader) Class.forName("dalvik.system.PathClassLoader")
                 .getConstructor(String.class, ClassLoader.class)