Merge pull request #779 from NativeScript/pete/fix-sbg-abstract-class-methods

Fix SBG

Author: petekanev

android-static-binding-generator/project/staticbindinggenerator/src/main/java/org/nativescript/staticbindinggenerator/Generator.java

@@ -7,13 +7,16 @@
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Queue;
 import java.util.Set;
 import java.util.jar.JarInputStream;
@@ -24,6 +27,8 @@
 import org.apache.bcel.classfile.Method;
 import org.apache.bcel.generic.Type;
 
+import javafx.util.Pair;
+
 public class Generator {
     private static final String JAVA_EXT = ".java";
 
@@ -45,7 +50,6 @@ public Generator(String outputDir, String[] libs, boolean throwOnError) throws I
         this.classes = readClasses(libs, throwOnError);
     }
 
-
     public void writeBindings(String filename) throws IOException, ClassNotFoundException {
         Binding[] bindings = generateBindings(filename);
         Set<File> writtenFiles = new HashSet<File>();
@@ -54,8 +58,15 @@ public void writeBindings(String filename) throws IOException, ClassNotFoundExce
                 try (PrintStream ps = new PrintStream(b.getFile())) {
                     ps.append(b.getContent());
                 }
+                // A file with that name has already been written
             } else {
-                throw new IOException("File already exists. This may lead to undesired behavior.\nPlease change the name of one of the extended classes.\n" + b.getFile());
+                // Compare text contents for equality
+                String content = new String(Files.readAllBytes(Paths.get(b.getFile().toString())));
+                if (content.equals(b.getContent())) {
+                    System.out.println("Warning: File already exists. This could mean the same code has been parsed more than once from two or more different files.");
+                } else {
+                    throw new IOException("File already exists. This may lead to undesired behavior.\nPlease change the name of one of the extended classes.\n" + b.getFile());
+                }
             }
         }
     }
@@ -186,51 +197,32 @@ private String getNormalizedName(String filename) {
         return sb.toString();
     }
 
-    private Map<String, List<Method>> getPublicApi(JavaClass clazz) throws ClassNotFoundException {
-        Map<String, List<Method>> api = new HashMap<String, List<Method>>();
+    private Map<String, MethodGroup> getPublicApi(JavaClass clazz) throws ClassNotFoundException {
+        Map<String, MethodGroup> api = new HashMap<>();
         JavaClass currentClass = clazz;
         String clazzName = clazz.getClassName();
         while (true) {
             String currentClassname = currentClass.getClassName();
 
-            boolean shouldCollectMethods = !(!clazzName.equals(currentClassname) && currentClass.isAbstract());
-
-            if (shouldCollectMethods || currentClass.isInterface()) {
-                // Don't include abstract parent class's methods to avoid compilation issues
-                // where a child class has 2 methods, of the same type, with just a
-                // return type/parameter type that differs by being of a superclass of the class being extended.
-                // see Test testCanCompileBindingClassExtendingAnExtendedClassWithMethodsWithTheSameSignature
-                List<Method> methods = new ArrayList<Method>();
-                for (Method m : currentClass.getMethods()) {
-                    methods.add(m);
-                }
-
-//                System.out.println("SBG: getPublicApi:collectInterfaceMethods classname: " + currentClassname);
+            List<Method> methods = new ArrayList<Method>();
+            for (Method m : currentClass.getMethods()) {
+                methods.add(m);
+            }
 
-                collectInterfaceMethods(clazz, methods);
-                for (Method m : methods) {
-                    if (!m.isSynthetic() && (m.isPublic() || m.isProtected()) && !m.isStatic()) {
-                        String name = m.getName();
+            collectInterfaceMethods(clazz, methods);
+            for (Method m : methods) {
+                if (!m.isSynthetic() && (m.isPublic() || m.isProtected()) && !m.isStatic()) {
+                    String name = m.getName();
 
-                        List<Method> methodGroup;
-                        if (api.containsKey(name)) {
-                            methodGroup = api.get(name);
-                        } else {
-                            methodGroup = new ArrayList<Method>();
-                            api.put(name, methodGroup);
-                        }
-                        boolean found = false;
-                        String methodSig = m.getSignature();
-                        for (Method m1 : methodGroup) {
-                            found = methodSig.equals(m1.getSignature());
-                            if (found) {
-                                break;
-                            }
-                        }
-                        if (!found) {
-                            methodGroup.add(m);
-                        }
+                    MethodGroup methodGroup;
+                    if (api.containsKey(name)) {
+                        methodGroup = api.get(name);
+                    } else {
+                        methodGroup = new MethodGroup(currentClassname);
+                        api.put(name, methodGroup);
                     }
+
+                    methodGroup.add(currentClassname, m);
                 }
             }
 
@@ -331,7 +323,7 @@ private void writeBinding(Writer w, DataRow dataRow, JavaClass clazz, String pac
         String[] implInterfaces = dataRow.getInterfaces();
         collectImplementedInterfaces(implInterfaces, clazz);
 
-        Map<String, List<Method>> api = getPublicApi(clazz);
+        Map<String, MethodGroup> api = getPublicApi(clazz);
 
         w.writeln("package " + packageName + ";");
         w.writeln();
@@ -438,7 +430,7 @@ private void writeBinding(Writer w, DataRow dataRow, JavaClass clazz, String pac
             collectInterfaceMethods(clazz, interfaceMethods);
             for (String methodName : dataRow.getMethods()) {
                 if (api.containsKey(methodName)) {
-                    List<Method> methodGroup = api.get(methodName);
+                    List<Method> methodGroup = api.get(methodName).getList();
                     for (Method m : methodGroup) {
                         boolean isInterfaceMethod = false;
                         if (interfaceMethods.contains(m)) {
@@ -719,4 +711,40 @@ private JavaClass getClass(String className) throws ClassNotFoundException {
 
         return clazz;
     }
+
+    private class MethodGroup {
+        private List<Method> methods;
+        private String latestInheritorClassName;
+
+        public MethodGroup(String forClass) {
+            this.methods = new ArrayList<>();
+            this.latestInheritorClassName = forClass;
+        }
+
+        public void add(String methodClassname, Method m) {
+            boolean found = false;
+
+            String methodSig = m.getSignature();
+            for (Method m1 : this.methods) {
+                found = methodSig.equals(m1.getSignature());
+                if (found) {
+                    break;
+                }
+            }
+
+            if (!found) {
+                // Only add method to API if method hasn't already been added by an inheritor of
+                // the current class
+                if (!methodClassname.equals(this.latestInheritorClassName)) {
+                    return;
+                }
+
+                this.methods.add(m);
+            }
+        }
+
+        public List<Method> getList() {
+            return this.methods;
+        }
+    }
 }

android-static-binding-generator/project/staticbindinggenerator/src/test/java/com/example/ListView.java

@@ -1,9 +1,5 @@
 package com.example;
 
-/**
- * Created by pkanev on 4/27/2017.
- */
-
 public class ListView extends View {
     public ListView createView() {
         return new ListView();

android-static-binding-generator/project/staticbindinggenerator/src/test/java/com/example/MyAbstractClass.java

@@ -0,0 +1,11 @@
+package com.example;
+
+public abstract class MyAbstractClass {
+    String className;
+
+    public MyAbstractClass(String name) {
+        this.className = name;
+    }
+
+    public abstract void abstractMethod();
+}

android-static-binding-generator/project/staticbindinggenerator/src/test/java/com/example/MyAbstractClass2.java

@@ -0,0 +1,7 @@
+package com.example;
+
+public abstract class MyAbstractClass2 extends MyAbstractClass {
+    public MyAbstractClass2() {
+        super("MyNotSoAbstractClass");
+    }
+}

android-static-binding-generator/project/staticbindinggenerator/src/test/java/com/example/MyClass.java

@@ -1,7 +1,4 @@
 package com.example;
 
-/**
- * Created by slavchev on 7/6/2016.
- */
 public class MyClass {
 }

android-static-binding-generator/project/staticbindinggenerator/src/test/java/com/example/View.java

@@ -1,9 +1,5 @@
 package com.example;
 
-/**
- * Created by pkanev on 4/27/2017.
- */
-
 public abstract class View {
     public View createView() {
         return null;

android-static-binding-generator/project/staticbindinggenerator/src/test/java/org/nativescript/staticbindinggenerator/test/GeneratorTest.java

@@ -1,6 +1,7 @@
 package org.nativescript.staticbindinggenerator.test;
 
 import com.example.ListView;
+import com.example.MyAbstractClass2;
 import com.example.MyInterface;
 
 import org.apache.commons.io.IOUtils;
@@ -129,4 +130,29 @@ public void testCanCompileBindingClassExtendingAnExtendedClassWithMethodsWithThe
         Assert.assertNotNull(ComplexClass);
         Assert.assertEquals(4, ComplexClass.getDeclaredMethods().length); // 1 + constructor + (equals + hashcode)
     }
+
+    @Test
+    public void testCanCompileBindingClassExtendingAnAbstractClassThatExtendsAbstractClass() throws Exception {
+        URL u = MyAbstractClass2.class.getResource('/' + MyAbstractClass2.class.getName().replace('.', '/') + ".class");
+        File f = new File(u.toURI()).getParentFile().getParentFile().getParentFile();
+
+        String dataRowString = "com.example.MyAbstractClass2*_fapp_l9_c29__*abstractMethod*com.example.com.example.MyExtendedClass**";
+        DataRow dataRow = new DataRow(dataRowString);
+
+        System.out.println(dataRowString);
+
+        String outputDir = null;
+        String[] libs = {runtimePath, f.getAbsolutePath()};
+        Generator generator = new Generator(outputDir, libs);
+        Binding binding = generator.generateBinding(dataRow);
+
+        StringBuffer sourceCode = new StringBuffer();
+        sourceCode.append(binding.getContent());
+
+        Iterable<String> options = new ArrayList<String>(Arrays.asList("-cp", dependenciesDir));
+        Class<?> ComplexClass = InMemoryJavaCompiler.compile(binding.getClassname(), sourceCode.toString(), options);
+
+        // class compiles, meaning abstract method of super-super class is extended properly
+        Assert.assertNotNull(ComplexClass);
+    }
 }

android-static-binding-generator/project/staticbindinggenerator/src/test/resources/org/nativescript/staticbindinggenerator/test/datarow-extend-abstract-class-extending-abstract-class.txt

@@ -0,0 +1 @@
+com.example.MyAbstractClass2*_fapp_l9_c29__*abstractMethod*com.example.com.example.MyExtendedClass**

android-static-binding-generator/project/staticbindinggenerator/src/test/resources/org/nativescript/staticbindinggenerator/test/datarow-extend-class-with-same-method-signature.txt

@@ -1 +1 @@
-"com.example.ListView*_fapp_l9_c29__*createView*com.example.MyListView**"
+com.example.ListView*_fapp_l9_c29__*createView*com.example.MyListView**