#1067 compilerJava - findMethods() patch

alex_panchenko Wed 7 Apr 2010

Hi,

There is a small deficiency in the collecting of java methods because Class.getMethods() can return the same method multiple times if method is defined in the interface and in the class.

Small example:

public interface A { int size(); }
public abstract class B { public abstract int size(); }
public abstract class C extends B implements A { }

It causes problems if you want to inherit from java abstract class because compiler reports Cannot override Java overloaded method: ...

The following patch fixes it.

Thank you,

Alex

diff -r 6072746778ce src/compilerJava/fan/JavaReflect.fan
--- a/src/compilerJava/fan/JavaReflect.fan	Tue Apr 06 09:46:03 2010 -0400
+++ b/src/compilerJava/fan/JavaReflect.fan	Thu Apr 08 00:43:05 2010 +0700
@@ -88,25 +88,47 @@
   }
 
   **
+  ** Collect public and protected methods of the specified java class
+  **
+  private static Void collectMethods(Class cls, HashMap acc)
+  {
+    cls.getDeclaredMethods.each |JMethod j|
+    {
+      if (JModifier.isProtected(j.getModifiers) || JModifier.isPublic(j.getModifiers)) {
+        key := MethodKey(j)
+        if (acc[key] == null) acc.put(key, j)
+      }
+    }
+  }
+
+  **
+  ** Collect methods of implemented interfaces and their super-interfaces, etc
+  ** 
+  private static Void collectInterfaceMethods(Class cls, HashMap acc, HashSet processedInterfaces)
+  {
+    cls.getInterfaces.each |Class intf|
+    {
+      if (processedInterfaces.add(intf))
+      {
+        collectMethods(intf, acc)
+        collectInterfaceMethods(intf, acc, processedInterfaces)
+      }
+    }
+  }
+
+  **
   ** Reflect the public and protected methods which Java
   ** reflection makes very difficult.
   **
   static JMethod[] findMethods(Class? cls)
   {
     acc := HashMap() // mutable keys
+    processedInterfaces := HashSet()
 
-    // first add all the public methods
-    cls.getMethods.each |JMethod j| { acc.put(j, j) }
-
-    // do protected methods working back up the hierarchy; don't
-    // worry about interfaces b/c they can declare protected members
     while (cls != null)
     {
-      cls.getDeclaredMethods.each |JMethod j|
-      {
-        if (!JModifier.isProtected(j.getModifiers)) return
-        if (acc[j] == null) acc.put(j, j)
-      }
+      collectMethods(cls, acc)
+      collectInterfaceMethods(cls, acc, processedInterfaces)
       cls = cls.getSuperclass
     }
 
@@ -315,4 +337,34 @@
     // just print a warning and ignore problematic APIs
     echo("WARNING: Cannot map Java type: $e.msg")
   }
+}
+
+internal const class MethodKey {
+  const Str name
+  const Class[] parameterTypes
+
+  new make(JMethod method)
+  {
+    name = method.getName()
+    parameterTypes = method.getParameterTypes()
+  }
+
+  override Int hash()
+  {
+    return name.hash()
+  }
+
+  override Bool equals(Obj? obj)
+  {
+    if (obj is MethodKey)
+    {
+      other := obj as MethodKey
+      return name == other.name && parameterTypes == other.parameterTypes;
+    }
+    else
+    {
+      return false
+    }
+  }
+
 }
\ No newline at end of file
diff -r 6072746778ce src/sys/java/fanx/util/FanUtil.java
--- a/src/sys/java/fanx/util/FanUtil.java	Tue Apr 06 09:46:03 2010 -0400
+++ b/src/sys/java/fanx/util/FanUtil.java	Thu Apr 08 00:43:05 2010 +0700
@@ -82,6 +82,7 @@
     javaImmutables.put("java.lang.Long",       Boolean.TRUE);
     javaImmutables.put("java.lang.Double",     Boolean.TRUE);
     javaImmutables.put("java.math.BigDecimal", Boolean.TRUE);
+    javaImmutables.put("java.lang.Class",      Boolean.TRUE);
   }
 
   /**

brian Sat 10 Apr 2010

Promoted to ticket #1067 and assigned to brian

I think I understand the problem, but I can't write any tests due to issue #965. So we need to fix that first.

The great idea in this patch is to go ahead and make classes like java.lang.Class immutable from Fantom runtime perspective. That will let us actually use proper Fantom maps in that JavaReflect code instead of HashMap which should cleanup the whole source file quite a bit.

brian Tue 11 May 2010

Ticket resolved in 1.0.53

I pushed a fix for this issue.

I fixed it addSlot where we add it to the link list (as opposed to creating a special hash key in findMethods)

changeset

Login or Signup to reply.