fix bugs in the mappings converter

7 files changed, 121 insertions, 56 deletions

diff --git a/src/cuchaz/enigma/ConvertMain.java b/src/cuchaz/enigma/ConvertMain.java
index c5c92bcb..79caae44 100644
--- a/src/cuchaz/enigma/ConvertMain.java
+++ b/src/cuchaz/enigma/ConvertMain.java
@@ -14,12 +14,16 @@ import cuchaz.enigma.convert.MemberMatches;
 import cuchaz.enigma.gui.ClassMatchingGui;
 import cuchaz.enigma.gui.MemberMatchingGui;
 import cuchaz.enigma.mapping.BehaviorEntry;
+import cuchaz.enigma.mapping.ClassEntry;
+import cuchaz.enigma.mapping.ClassMapping;
 import cuchaz.enigma.mapping.FieldEntry;
+import cuchaz.enigma.mapping.FieldMapping;
 import cuchaz.enigma.mapping.MappingParseException;
 import cuchaz.enigma.mapping.Mappings;
 import cuchaz.enigma.mapping.MappingsChecker;
 import cuchaz.enigma.mapping.MappingsReader;
 import cuchaz.enigma.mapping.MappingsWriter;
+import cuchaz.enigma.mapping.MethodMapping;
 
 
 public class ConvertMain {
@@ -158,7 +162,7 @@ public class ConvertMain {
                 
                 // apply matches
                 Mappings newMappings = MappingsConverter.newMappings(classMatches, mappings, deobfuscators.source, deobfuscators.dest);
-                MappingsConverter.applyMemberMatches(newMappings, fieldMatches, MappingsConverter.getFieldDoer());
+                MappingsConverter.applyMemberMatches(newMappings, classMatches, fieldMatches, MappingsConverter.getFieldDoer());
                 
                 // write out the converted mappings
                 try (FileWriter out = new FileWriter(outMappingsFile)) {
@@ -232,8 +236,25 @@ public class ConvertMain {
                 
                 // apply matches
                 Mappings newMappings = MappingsConverter.newMappings(classMatches, mappings, deobfuscators.source, deobfuscators.dest);
-                MappingsConverter.applyMemberMatches(newMappings, fieldMatches, MappingsConverter.getFieldDoer());
-                MappingsConverter.applyMemberMatches(newMappings, methodMatches, MappingsConverter.getMethodDoer());
+                MappingsConverter.applyMemberMatches(newMappings, classMatches, fieldMatches, MappingsConverter.getFieldDoer());
+                MappingsConverter.applyMemberMatches(newMappings, classMatches, methodMatches, MappingsConverter.getMethodDoer());
+                
+                // check the final mappings
+                MappingsChecker checker = new MappingsChecker(deobfuscators.dest.getJarIndex());
+                checker.dropBrokenMappings(newMappings);
+                
+                for (java.util.Map.Entry<ClassEntry,ClassMapping> mapping : checker.getDroppedClassMappings().entrySet()) {
+                        System.out.println("WARNING: Broken class entry " + mapping.getKey() + " (" + mapping.getValue().getDeobfName() + ")");
+                }
+                for (java.util.Map.Entry<ClassEntry,ClassMapping> mapping : checker.getDroppedInnerClassMappings().entrySet()) {
+                        System.out.println("WARNING: Broken inner class entry " + mapping.getKey() + " (" + mapping.getValue().getDeobfName() + ")");
+                }
+                for (java.util.Map.Entry<FieldEntry,FieldMapping> mapping : checker.getDroppedFieldMappings().entrySet()) {
+                        System.out.println("WARNING: Broken field entry " + mapping.getKey() + " (" + mapping.getValue().getDeobfName() + ")");
+                }
+                for (java.util.Map.Entry<BehaviorEntry,MethodMapping> mapping : checker.getDroppedMethodMappings().entrySet()) {
+                        System.out.println("WARNING: Broken behavior entry " + mapping.getKey() + " (" + mapping.getValue().getDeobfName() + ")");
+                }
                 
                 // write out the converted mappings
                 try (FileWriter out = new FileWriter(outMappingsFile)) {
diff --git a/src/cuchaz/enigma/convert/MappingsConverter.java b/src/cuchaz/enigma/convert/MappingsConverter.java
index 59f3b5ba..ddd3a53c 100644
--- a/src/cuchaz/enigma/convert/MappingsConverter.java
+++ b/src/cuchaz/enigma/convert/MappingsConverter.java
@@ -183,7 +183,7 @@ public class MappingsConverter {
                 return newMappings;
         }
         
-        private static ClassMapping migrateClassMapping(ClassEntry newObfClass, ClassMapping mapping, final ClassMatches matches, boolean useSimpleName) {
+        private static ClassMapping migrateClassMapping(ClassEntry newObfClass, ClassMapping oldClassMapping, final ClassMatches matches, boolean useSimpleName) {
                 
                 ClassNameReplacer replacer = new ClassNameReplacer() {
                         @Override
@@ -196,30 +196,28 @@ public class MappingsConverter {
                         }
                 };
                 
-                ClassMapping newMapping;
-                String deobfName = mapping.getDeobfName();
+                ClassMapping newClassMapping;
+                String deobfName = oldClassMapping.getDeobfName();
                 if (deobfName != null) {
                         if (useSimpleName) {
                                 deobfName = new ClassEntry(deobfName).getSimpleName();
                         }
-                        newMapping = new ClassMapping(newObfClass.getName(), deobfName);
+                        newClassMapping = new ClassMapping(newObfClass.getName(), deobfName);
                 } else {
-                        newMapping = new ClassMapping(newObfClass.getName());
+                        newClassMapping = new ClassMapping(newObfClass.getName());
                 }
                 
                 // copy fields
-                for (FieldMapping fieldMapping : mapping.fields()) {
-                        // TODO: map field obf names too...
-                        newMapping.addFieldMapping(new FieldMapping(fieldMapping, replacer));
+                for (FieldMapping fieldMapping : oldClassMapping.fields()) {
+                        newClassMapping.addFieldMapping(new FieldMapping(fieldMapping, replacer));
                 }
                 
                 // copy methods
-                for (MethodMapping methodMapping : mapping.methods()) {
-                        // TODO: map method obf names too...
-                        newMapping.addMethodMapping(new MethodMapping(methodMapping, replacer));
+                for (MethodMapping methodMapping : oldClassMapping.methods()) {
+                        newClassMapping.addMethodMapping(new MethodMapping(methodMapping, replacer));
                 }
                 
-                return newMapping;
+                return newClassMapping;
         }
 
         public static void convertMappings(Mappings mappings, BiMap<ClassEntry,ClassEntry> changes) {
@@ -262,8 +260,9 @@ public class MappingsConverter {
                 Collection<T> getObfEntries(JarIndex jarIndex);
                 Collection<? extends MemberMapping<T>> getMappings(ClassMapping destClassMapping);
                 Set<T> filterEntries(Collection<T> obfEntries, T obfSourceEntry, ClassMatches classMatches);
-                void setMemberObfName(ClassMapping classMapping, MemberMapping<T> memberMapping, String newObfName);
+                void setUpdateObfMember(ClassMapping classMapping, MemberMapping<T> memberMapping, T newEntry);
                 boolean hasObfMember(ClassMapping classMapping, T obfEntry);
+                void removeMemberByObf(ClassMapping classMapping, T obfEntry);
         }
         
         public static Doer<FieldEntry> getFieldDoer() {
@@ -297,15 +296,20 @@ public class MappingsConverter {
                         }
 
                         @Override
-                        public void setMemberObfName(ClassMapping classMapping, MemberMapping<FieldEntry> memberMapping, String newObfName) {
+                        public void setUpdateObfMember(ClassMapping classMapping, MemberMapping<FieldEntry> memberMapping, FieldEntry newField) {
                                 FieldMapping fieldMapping = (FieldMapping)memberMapping;
-                                classMapping.setFieldObfName(fieldMapping.getObfName(), fieldMapping.getObfType(), newObfName);
+                                classMapping.setFieldObfNameAndType(fieldMapping.getObfName(), fieldMapping.getObfType(), newField.getName(), newField.getType());
                         }
                         
                         @Override
                         public boolean hasObfMember(ClassMapping classMapping, FieldEntry obfField) {
                                 return classMapping.getFieldByObf(obfField.getName(), obfField.getType()) != null;
                         }
+
+                        @Override
+                        public void removeMemberByObf(ClassMapping classMapping, FieldEntry obfField) {
+                                classMapping.removeFieldMapping(classMapping.getFieldByObf(obfField.getName(), obfField.getType()));
+                        }
                 };
         }
         
@@ -344,15 +348,20 @@ public class MappingsConverter {
                         }
 
                         @Override
-                        public void setMemberObfName(ClassMapping classMapping, MemberMapping<BehaviorEntry> memberMapping, String newObfName) {
+                        public void setUpdateObfMember(ClassMapping classMapping, MemberMapping<BehaviorEntry> memberMapping, BehaviorEntry newBehavior) {
                                 MethodMapping methodMapping = (MethodMapping)memberMapping;
-                                classMapping.setMethodObfName(methodMapping.getObfName(), methodMapping.getObfSignature(), newObfName);
+                                classMapping.setMethodObfNameAndSignature(methodMapping.getObfName(), methodMapping.getObfSignature(), newBehavior.getName(), newBehavior.getSignature());
                         }
                         
                         @Override
                         public boolean hasObfMember(ClassMapping classMapping, BehaviorEntry obfBehavior) {
                                 return classMapping.getMethodByObf(obfBehavior.getName(), obfBehavior.getSignature()) != null;
                         }
+
+                        @Override
+                        public void removeMemberByObf(ClassMapping classMapping, BehaviorEntry obfBehavior) {
+                                classMapping.removeMethodMapping(classMapping.getMethodByObf(obfBehavior.getName(), obfBehavior.getSignature()));
+                        }
                 };
         }
         
@@ -477,56 +486,74 @@ public class MappingsConverter {
                 });
         }
 
-        public static <T extends Entry> void applyMemberMatches(Mappings mappings, MemberMatches<T> memberMatches, Doer<T> doer) {
+        public static <T extends Entry> void applyMemberMatches(Mappings mappings, ClassMatches classMatches, MemberMatches<T> memberMatches, Doer<T> doer) {
                 for (ClassMapping classMapping : mappings.classes()) {
-                        applyMemberMatches(classMapping, memberMatches, doer);
+                        applyMemberMatches(classMapping, classMatches, memberMatches, doer);
                 }
         }
         
-        private static <T extends Entry> void applyMemberMatches(ClassMapping classMapping, MemberMatches<T> memberMatches, Doer<T> doer) {
-                ClassEntry classEntry = classMapping.getObfEntry();
+        private static <T extends Entry> void applyMemberMatches(ClassMapping classMapping, ClassMatches classMatches, MemberMatches<T> memberMatches, Doer<T> doer) {
+                
+                // get the classes
+                ClassEntry obfDestClass = classMapping.getObfEntry();
                 
                 // make a map of all the renames we need to make
                 Map<T,T> renames = Maps.newHashMap();
                 for (MemberMapping<T> memberMapping : Lists.newArrayList(doer.getMappings(classMapping))) {
-                        T obfSourceEntry = memberMapping.getObfEntry(classEntry);
-                        T obfDestEntry = memberMatches.matches().get(obfSourceEntry);
-                        if (obfDestEntry != null) {
-                                if (obfDestEntry.getName().equals(obfSourceEntry.getName())) {
-                                        // same name, don't need to change anything
-                                        continue;
-                                }
-                                renames.put(obfSourceEntry, obfDestEntry);
+                        T obfOldDestEntry = memberMapping.getObfEntry(obfDestClass);
+                        T obfSourceEntry = getSourceEntryFromDestMapping(memberMapping, obfDestClass, classMatches);
+                        
+                        // but drop the unmatchable things
+                        if (memberMatches.isUnmatchableSourceEntry(obfSourceEntry)) {
+                                doer.removeMemberByObf(classMapping, obfOldDestEntry);
+                                continue;
+                        }
+                                
+                        T obfNewDestEntry = memberMatches.matches().get(obfSourceEntry);
+                        if (obfNewDestEntry != null && !obfOldDestEntry.getName().equals(obfNewDestEntry.getName())) {
+                                renames.put(obfOldDestEntry, obfNewDestEntry);
                         }
                 }
                 
-                // apply to this class (should never need more than n passes)
-                int numRenames = renames.size();
-                for (int i=0; i<numRenames && !renames.isEmpty(); i++) {
-                        for (MemberMapping<T> memberMapping : Lists.newArrayList(doer.getMappings(classMapping))) {
-                                T obfSourceEntry = memberMapping.getObfEntry(classEntry);
-                                T obfDestEntry = renames.get(obfSourceEntry);
-                                if (obfDestEntry != null) {
-                                        // make sure this rename won't cause a collision
-                                        if (!doer.hasObfMember(classMapping, obfDestEntry)) {
-                                                doer.setMemberObfName(classMapping, memberMapping, obfDestEntry.getName());
-                                                renames.remove(obfSourceEntry);
+                if (!renames.isEmpty()) {
+                
+                        // apply to this class (should never need more than n passes)
+                        int numRenamesAppliedThisRound;
+                        do {
+                                numRenamesAppliedThisRound = 0;
+                                
+                                for (MemberMapping<T> memberMapping : Lists.newArrayList(doer.getMappings(classMapping))) {
+                                        T obfOldDestEntry = memberMapping.getObfEntry(obfDestClass);
+                                        T obfNewDestEntry = renames.get(obfOldDestEntry);
+                                        if (obfNewDestEntry != null) {
+                                                // make sure this rename won't cause a collision
+                                                // otherwise, save it for the next round and try again next time
+                                                if (!doer.hasObfMember(classMapping, obfNewDestEntry)) {
+                                                        doer.setUpdateObfMember(classMapping, memberMapping, obfNewDestEntry);
+                                                        renames.remove(obfOldDestEntry);
+                                                        numRenamesAppliedThisRound++;
+                                                }
                                         }
                                 }
-                        }
-                }
-                if (!renames.isEmpty()) {
-                        System.err.println(String.format("WARNING: Couldn't apply all the renames for class %s. %d/%d renames left.",
-                                classMapping.getObfFullName(), renames.size(), numRenames
-                        ));
-                        for (Map.Entry<T,T> entry : renames.entrySet()) {
-                                System.err.println(String.format("\t%s -> %s", entry.getKey(), entry.getValue()));
+                        } while(numRenamesAppliedThisRound > 0);
+                        
+                        if (!renames.isEmpty()) {
+                                System.err.println(String.format("WARNING: Couldn't apply all the renames for class %s. %d renames left.",
+                                        classMapping.getObfFullName(), renames.size()
+                                ));
+                                for (Map.Entry<T,T> entry : renames.entrySet()) {
+                                        System.err.println(String.format("\t%s -> %s", entry.getKey().getName(), entry.getValue().getName()));
+                                }
                         }
                 }
                 
                 // recurse
                 for (ClassMapping innerClassMapping : classMapping.innerClasses()) {
-                        applyMemberMatches(innerClassMapping, memberMatches, doer);
+                        applyMemberMatches(innerClassMapping, classMatches, memberMatches, doer);
                 }
         }
+        
+        private static <T extends Entry> T getSourceEntryFromDestMapping(MemberMapping<T> destMemberMapping, ClassEntry obfDestClass, ClassMatches classMatches) {
+                return translate(destMemberMapping.getObfEntry(obfDestClass), classMatches.getUniqueMatches().inverse());
+        }
 }
diff --git a/src/cuchaz/enigma/convert/MemberMatches.java b/src/cuchaz/enigma/convert/MemberMatches.java
index 1078ab75..1c162eac 100644
--- a/src/cuchaz/enigma/convert/MemberMatches.java
+++ b/src/cuchaz/enigma/convert/MemberMatches.java
@@ -113,6 +113,10 @@ public class MemberMatches<T extends Entry> {
         public boolean isMatchedDestEntry(T destEntry) {
                 return m_matches.containsValue(destEntry);
         }
+        
+        public boolean isUnmatchableSourceEntry(T sourceEntry) {
+                return m_unmatchableSourceEntries.containsEntry(sourceEntry.getClassEntry(), sourceEntry);
+        }
 
         public void makeMatch(T sourceEntry, T destEntry) {
                 boolean wasRemoved = m_unmatchedSourceEntries.remove(sourceEntry.getClassEntry(), sourceEntry);
diff --git a/src/cuchaz/enigma/mapping/ClassMapping.java b/src/cuchaz/enigma/mapping/ClassMapping.java
index ac70df0d..38cd3d6d 100644
--- a/src/cuchaz/enigma/mapping/ClassMapping.java
+++ b/src/cuchaz/enigma/mapping/ClassMapping.java
@@ -243,12 +243,13 @@ public class ClassMapping implements Serializable, Comparable<ClassMapping> {
                 }
         }
         
-        public void setFieldObfName(String oldObfName, Type obfType, String newObfName) {
+        public void setFieldObfNameAndType(String oldObfName, Type obfType, String newObfName, Type newObfType) {
                 assert(newObfName != null);
                 FieldMapping fieldMapping = m_fieldsByObf.remove(getFieldKey(oldObfName, obfType));
                 assert(fieldMapping != null);
                 fieldMapping.setObfName(newObfName);
-                boolean obfWasAdded = m_fieldsByObf.put(getFieldKey(newObfName, obfType), fieldMapping) == null;
+                fieldMapping.setObfType(newObfType);
+                boolean obfWasAdded = m_fieldsByObf.put(getFieldKey(newObfName, newObfType), fieldMapping) == null;
                 assert(obfWasAdded);
         }
         
@@ -328,12 +329,13 @@ public class ClassMapping implements Serializable, Comparable<ClassMapping> {
                 }
         }
         
-        public void setMethodObfName(String oldObfName, Signature obfSignature, String newObfName) {
+        public void setMethodObfNameAndSignature(String oldObfName, Signature obfSignature, String newObfName, Signature newObfSignature) {
                 assert(newObfName != null);
                 MethodMapping methodMapping = m_methodsByObf.remove(getMethodKey(oldObfName, obfSignature));
                 assert(methodMapping != null);
                 methodMapping.setObfName(newObfName);
-                boolean obfWasAdded = m_methodsByObf.put(getMethodKey(newObfName, obfSignature), methodMapping) == null;
+                methodMapping.setObfSignature(newObfSignature);
+                boolean obfWasAdded = m_methodsByObf.put(getMethodKey(newObfName, newObfSignature), methodMapping) == null;
                 assert(obfWasAdded);
         }
         
diff --git a/src/cuchaz/enigma/mapping/FieldMapping.java b/src/cuchaz/enigma/mapping/FieldMapping.java
index 3aa9e698..463d901c 100644
--- a/src/cuchaz/enigma/mapping/FieldMapping.java
+++ b/src/cuchaz/enigma/mapping/FieldMapping.java
@@ -32,6 +32,7 @@ public class FieldMapping implements Serializable, Comparable<FieldMapping>, Mem
                 m_obfType = new Type(other.m_obfType, obfClassNameReplacer);
         }
 
+        @Override
         public String getObfName() {
                 return m_obfName;
         }
@@ -52,6 +53,10 @@ public class FieldMapping implements Serializable, Comparable<FieldMapping>, Mem
                 return m_obfType;
         }
         
+        public void setObfType(Type val) {
+                m_obfType = val;
+        }
+        
         @Override
         public int compareTo(FieldMapping other) {
                 return (m_obfName + m_obfType).compareTo(other.m_obfName + other.m_obfType);
diff --git a/src/cuchaz/enigma/mapping/MemberMapping.java b/src/cuchaz/enigma/mapping/MemberMapping.java
index d62a7c10..d8e2ee3a 100644
--- a/src/cuchaz/enigma/mapping/MemberMapping.java
+++ b/src/cuchaz/enigma/mapping/MemberMapping.java
@@ -3,4 +3,5 @@ package cuchaz.enigma.mapping;
 
 public interface MemberMapping<T extends Entry> {
         T getObfEntry(ClassEntry classEntry);
+        String getObfName();
 }
diff --git a/src/cuchaz/enigma/mapping/MethodMapping.java b/src/cuchaz/enigma/mapping/MethodMapping.java
index a67e352f..bef232e3 100644
--- a/src/cuchaz/enigma/mapping/MethodMapping.java
+++ b/src/cuchaz/enigma/mapping/MethodMapping.java
@@ -52,6 +52,7 @@ public class MethodMapping implements Serializable, Comparable<MethodMapping>, M
                 }
         }
 
+        @Override
         public String getObfName() {
                 return m_obfName;
         }
@@ -72,6 +73,10 @@ public class MethodMapping implements Serializable, Comparable<MethodMapping>, M
                 return m_obfSignature;
         }
         
+        public void setObfSignature(Signature val) {
+                m_obfSignature = val;
+        }
+        
         public Iterable<ArgumentMapping> arguments() {
                 return m_arguments.values();
         }