rewrite entry resolution logic in TranslationIndex, hopefully fixing bugs

9 files changed, 82 insertions, 89 deletions

diff --git a/build.gradle b/build.gradle
index 8eae880d..54b574b3 100644
--- a/build.gradle
+++ b/build.gradle
@@ -19,7 +19,7 @@ apply plugin: 'com.github.johnrengelman.shadow'
 apply plugin: 'maven'
 
 group = 'cuchaz'
-version = '0.12.1'
+version = '0.12.2'
 
 def ENV = System.getenv()
 if (ENV.BUILD_NUMBER) {
diff --git a/src/main/java/cuchaz/enigma/analysis/TranslationIndex.java b/src/main/java/cuchaz/enigma/analysis/TranslationIndex.java
index b2ddc5fa..db116623 100644
--- a/src/main/java/cuchaz/enigma/analysis/TranslationIndex.java
+++ b/src/main/java/cuchaz/enigma/analysis/TranslationIndex.java
@@ -11,6 +11,8 @@
 
 package cuchaz.enigma.analysis;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -19,15 +21,13 @@ import cuchaz.enigma.bytecode.AccessFlags;
 import cuchaz.enigma.mapping.*;
 import cuchaz.enigma.mapping.entry.*;
 
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 public class TranslationIndex {
 
         private final ReferencedEntryPool entryPool;
         private Map<ClassEntry, ClassEntry> superclasses;
+        private Map<Entry, DefEntry> defEntries = new HashMap<>();
         private Multimap<ClassEntry, FieldDefEntry> fieldEntries;
         private Multimap<ClassEntry, MethodDefEntry> methodEntries;
         private Multimap<ClassEntry, ClassEntry> interfaces;
@@ -38,6 +38,14 @@ public class TranslationIndex {
                 this.fieldEntries = HashMultimap.create();
                 this.methodEntries = HashMultimap.create();
                 this.interfaces = HashMultimap.create();
+
+                for (FieldDefEntry entry : fieldEntries.values()) {
+                        defEntries.put(entry, entry);
+                }
+
+                for (MethodDefEntry entry : methodEntries.values()) {
+                        defEntries.put(entry, entry);
+                }
         }
 
         public TranslationIndex(TranslationIndex other, Translator translator) {
@@ -74,6 +82,14 @@ public class TranslationIndex {
                                         translator.getTranslatedMethodDef(mapEntry.getValue())
                         );
                 }
+
+                for (FieldDefEntry entry : fieldEntries.values()) {
+                        defEntries.put(entry, entry);
+                }
+
+                for (MethodDefEntry entry : methodEntries.values()) {
+                        defEntries.put(entry, entry);
+                }
         }
 
         protected ClassDefEntry indexClass(int access, String name, String signature, String superName, String[] interfaces) {
@@ -101,16 +117,27 @@ public class TranslationIndex {
 
         protected void indexField(FieldDefEntry fieldEntry) {
                 this.fieldEntries.put(fieldEntry.getOwnerClassEntry(), fieldEntry);
+                this.defEntries.put(fieldEntry, fieldEntry);
         }
 
         protected void indexMethod(MethodDefEntry methodEntry) {
                 this.methodEntries.put(methodEntry.getOwnerClassEntry(), methodEntry);
+                this.defEntries.put(methodEntry, methodEntry);
         }
 
         public void renameClasses(Map<String, String> renames) {
                 EntryRenamer.renameClassesInMap(renames, this.superclasses);
                 EntryRenamer.renameClassesInMultimap(renames, this.fieldEntries);
                 EntryRenamer.renameClassesInMultimap(renames, this.methodEntries);
+
+                this.defEntries.clear();
+                for (FieldDefEntry entry : fieldEntries.values()) {
+                        defEntries.put(entry, entry);
+                }
+
+                for (MethodDefEntry entry : methodEntries.values()) {
+                        defEntries.put(entry, entry);
+                }
         }
 
         public ClassEntry getSuperclass(ClassEntry classEntry) {
@@ -171,6 +198,7 @@ public class TranslationIndex {
                 if (entry == null) {
                         return false;
                 }
+
                 if (entry instanceof FieldEntry) {
                         return fieldExists((FieldEntry) entry);
                 } else if (entry instanceof MethodEntry) {
@@ -190,95 +218,42 @@ public class TranslationIndex {
         }
 
         public ClassEntry resolveEntryOwner(Entry entry) {
-                return resolveEntryOwner(entry, false);
-        }
-
-        public ClassEntry resolveEntryOwner(Entry entry, boolean checkSuperclassBeforeChild) {
                 if (entry instanceof ClassEntry) {
                         return (ClassEntry) entry;
                 }
 
-                ClassEntry superclassEntry = resolveSuperclass(entry, checkSuperclassBeforeChild);
-                if (superclassEntry != null) {
-                        return superclassEntry;
-                }
-
-                ClassEntry interfaceEntry = resolveInterface(entry);
-                if (interfaceEntry != null) {
-                        return interfaceEntry;
+                if (entryExists(entry)) {
+                        return entry.getOwnerClassEntry();
                 }
 
-                return null;
-        }
-
-        public ClassEntry resolveSuperclass(Entry entry, boolean checkSuperclassBeforeChild) {
-
-                // Default case
-                if (!checkSuperclassBeforeChild)
-                        return resolveSuperclass(entry);
-
-                // Save the original entry
-                Entry originalEntry = entry;
-
-                // Get all possible superclasses and reverse the list
-                List<ClassEntry> superclasses = Lists.reverse(getAncestry(originalEntry.getOwnerClassEntry()));
-
-                boolean existInEntry = false;
-
-                for (ClassEntry classEntry : superclasses) {
-                        entry = entry.updateOwnership(classEntry);
-                        existInEntry = entryExists(entry);
-
-                        // Check for possible entry in interfaces of superclasses
-                        ClassEntry interfaceEntry = resolveInterface(entry);
-                        if (interfaceEntry != null)
-                                return interfaceEntry;
-                        if (existInEntry)
-                                break;
+                DefEntry def = defEntries.get(entry);
+                if (def != null && (def.getAccess().isPrivate())) {
+                        return null;
                 }
 
-                // Doesn't exists in superclasses? check the child or return null
-                if (!existInEntry)
-                        return !entryExists(originalEntry) ? null : originalEntry.getOwnerClassEntry();
-
-                return entry.getOwnerClassEntry();
-        }
-
-        public ClassEntry resolveSuperclass(Entry entry) {
-                // this entry could refer to a method on a class where the method is not actually implemented
-                // travel up the inheritance tree to find the closest implementation
-
-                while (!entryExists(entry)) {
-                        // is there a parent class?
-                        ClassEntry superclassEntry = getSuperclass(entry.getOwnerClassEntry());
-                        if (superclassEntry == null) {
-                                // this is probably a method from a class in a library
-                                // we can't trace the implementation up any higher unless we index the library
-                                return null;
+                // if we're protected/public/non-static, chances are we're somewhere down
+                LinkedList<ClassEntry> classEntries = new LinkedList<>();
+                classEntries.add(entry.getOwnerClassEntry());
+                while (!classEntries.isEmpty()) {
+                        ClassEntry c = classEntries.remove();
+                        Entry cEntry = entry.updateOwnership(c);
+
+                        if (entryExists(cEntry)) {
+                                def = defEntries.get(cEntry);
+                                if (def == null || (!def.getAccess().isPrivate())) {
+                                        return cEntry.getOwnerClassEntry();
+                                }
                         }
 
-                        // move up to the parent class
-                        entry = entry.updateOwnership(superclassEntry);
-                }
-                return entry.getOwnerClassEntry();
-        }
-
-        public ClassEntry resolveInterface(Entry entry) {
-                // the interfaces for any class is a forest
-                // so let's look at all the trees
-
-                for (ClassEntry interfaceEntry : this.interfaces.get(entry.getOwnerClassEntry())) {
-                        Collection<ClassEntry> subInterface = this.interfaces.get(interfaceEntry);
-                        if (subInterface != null && !subInterface.isEmpty()) {
-                                ClassEntry result = resolveInterface(entry.updateOwnership(interfaceEntry));
-                                if (result != null)
-                                        return result;
+                        ClassEntry superC = getSuperclass(c);
+                        if (superC != null) {
+                                classEntries.add(superC);
                         }
-                        ClassEntry resolvedClassEntry = resolveSuperclass(entry.updateOwnership(interfaceEntry));
-                        if (resolvedClassEntry != null) {
-                                return resolvedClassEntry;
+                        if (entry instanceof MethodEntry) {
+                                classEntries.addAll(getInterfaces(c));
                         }
                 }
+
                 return null;
         }
 
diff --git a/src/main/java/cuchaz/enigma/mapping/DirectionalTranslator.java b/src/main/java/cuchaz/enigma/mapping/DirectionalTranslator.java
index 10fb9215..388e7ac3 100644
--- a/src/main/java/cuchaz/enigma/mapping/DirectionalTranslator.java
+++ b/src/main/java/cuchaz/enigma/mapping/DirectionalTranslator.java
@@ -131,7 +131,7 @@ public class DirectionalTranslator implements Translator {
 
         private String translateFieldName(FieldEntry entry) {
                 // resolve the class entry
-                ClassEntry resolvedClassEntry = this.index.resolveEntryOwner(entry, true);
+                ClassEntry resolvedClassEntry = this.index.resolveEntryOwner(entry);
                 if (resolvedClassEntry != null) {
                         // look for the class
                         ClassMapping classMapping = findClassMapping(resolvedClassEntry);
@@ -175,7 +175,7 @@ public class DirectionalTranslator implements Translator {
 
         private String translateMethodName(MethodEntry entry) {
                 // resolve the class entry
-                ClassEntry resolvedOwner = this.index.resolveEntryOwner(entry, true);
+                ClassEntry resolvedOwner = this.index.resolveEntryOwner(entry);
                 if (resolvedOwner != null) {
                         // look for class
                         ClassMapping classMapping = findClassMapping(resolvedOwner);
diff --git a/src/main/java/cuchaz/enigma/mapping/MappingsRenamer.java b/src/main/java/cuchaz/enigma/mapping/MappingsRenamer.java
index 72a12c26..8ef4f12b 100644
--- a/src/main/java/cuchaz/enigma/mapping/MappingsRenamer.java
+++ b/src/main/java/cuchaz/enigma/mapping/MappingsRenamer.java
@@ -92,9 +92,9 @@ public class MappingsRenamer {
                 deobfName = NameValidator.validateFieldName(deobfName);
                 FieldEntry targetEntry = entryPool.getField(obf.getOwnerClassEntry(), deobfName, obf.getDesc());
                 ClassEntry definedClass = null;
-                if (mappings.containsDeobfField(obf.getOwnerClassEntry(), deobfName) || index.containsEntryWithSameName(targetEntry))
+                if (mappings.containsDeobfField(obf.getOwnerClassEntry(), deobfName) || index.containsEntryWithSameName(targetEntry)) {
                         definedClass = obf.getOwnerClassEntry();
-                else {
+                } else {
                         for (ClassEntry ancestorEntry : this.index.getTranslationIndex().getAncestry(obf.getOwnerClassEntry())) {
                                 if (mappings.containsDeobfField(ancestorEntry, deobfName) || index.containsEntryWithSameName(targetEntry.updateOwnership(ancestorEntry))) {
                                         definedClass = ancestorEntry;
diff --git a/src/main/java/cuchaz/enigma/mapping/entry/ClassDefEntry.java b/src/main/java/cuchaz/enigma/mapping/entry/ClassDefEntry.java
index ac1fe2ab..df72e7e9 100644
--- a/src/main/java/cuchaz/enigma/mapping/entry/ClassDefEntry.java
+++ b/src/main/java/cuchaz/enigma/mapping/entry/ClassDefEntry.java
@@ -15,7 +15,7 @@ import com.google.common.base.Preconditions;
 import cuchaz.enigma.bytecode.AccessFlags;
 import cuchaz.enigma.mapping.Signature;
 
-public class ClassDefEntry extends ClassEntry {
+public class ClassDefEntry extends ClassEntry implements DefEntry {
         private final AccessFlags access;
         private final Signature signature;
 
@@ -31,6 +31,7 @@ public class ClassDefEntry extends ClassEntry {
                 return signature;
         }
 
+        @Override
         public AccessFlags getAccess() {
                 return access;
         }
diff --git a/src/main/java/cuchaz/enigma/mapping/entry/DefEntry.java b/src/main/java/cuchaz/enigma/mapping/entry/DefEntry.java
new file mode 100644
index 00000000..43ad0274
--- /dev/null
+++ b/src/main/java/cuchaz/enigma/mapping/entry/DefEntry.java
@@ -0,0 +1,7 @@
+package cuchaz.enigma.mapping.entry;
+
+import cuchaz.enigma.bytecode.AccessFlags;
+
+public interface DefEntry extends Entry {
+    AccessFlags getAccess();
+}
diff --git a/src/main/java/cuchaz/enigma/mapping/entry/FieldDefEntry.java b/src/main/java/cuchaz/enigma/mapping/entry/FieldDefEntry.java
index d18115bf..223410f5 100644
--- a/src/main/java/cuchaz/enigma/mapping/entry/FieldDefEntry.java
+++ b/src/main/java/cuchaz/enigma/mapping/entry/FieldDefEntry.java
@@ -16,7 +16,7 @@ import cuchaz.enigma.bytecode.AccessFlags;
 import cuchaz.enigma.mapping.Signature;
 import cuchaz.enigma.mapping.TypeDescriptor;
 
-public class FieldDefEntry extends FieldEntry {
+public class FieldDefEntry extends FieldEntry implements DefEntry {
         private final AccessFlags access;
         private final Signature signature;
 
@@ -28,6 +28,7 @@ public class FieldDefEntry extends FieldEntry {
                 this.signature = signature;
         }
 
+        @Override
         public AccessFlags getAccess() {
                 return access;
         }
diff --git a/src/main/java/cuchaz/enigma/mapping/entry/MethodDefEntry.java b/src/main/java/cuchaz/enigma/mapping/entry/MethodDefEntry.java
index ec3af694..960b08d1 100644
--- a/src/main/java/cuchaz/enigma/mapping/entry/MethodDefEntry.java
+++ b/src/main/java/cuchaz/enigma/mapping/entry/MethodDefEntry.java
@@ -16,7 +16,7 @@ import cuchaz.enigma.bytecode.AccessFlags;
 import cuchaz.enigma.mapping.MethodDescriptor;
 import cuchaz.enigma.mapping.Signature;
 
-public class MethodDefEntry extends MethodEntry {
+public class MethodDefEntry extends MethodEntry implements DefEntry {
 
         private final AccessFlags access;
         private final Signature signature;
@@ -29,6 +29,7 @@ public class MethodDefEntry extends MethodEntry {
                 this.signature = signature;
         }
 
+        @Override
         public AccessFlags getAccess() {
                 return access;
         }
diff --git a/src/main/java/cuchaz/enigma/utils/Utils.java b/src/main/java/cuchaz/enigma/utils/Utils.java
index 8e502d47..bd09c64f 100644
--- a/src/main/java/cuchaz/enigma/utils/Utils.java
+++ b/src/main/java/cuchaz/enigma/utils/Utils.java
@@ -27,7 +27,15 @@ import java.util.List;
 public class Utils {
 
         public static int combineHashesOrdered(Object... objs) {
-                return combineHashesOrdered(Arrays.asList(objs));
+                final int prime = 67;
+                int result = 1;
+                for (Object obj : objs) {
+                        result *= prime;
+                        if (obj != null) {
+                                result += obj.hashCode();
+                        }
+                }
+                return result;
         }
 
         public static int combineHashesOrdered(List<Object> objs) {