allow name clashes to pass where javac would accept them

2 files changed, 111 insertions, 26 deletions

diff --git a/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java b/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java
index f9f3b88e..134c05ce 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java
@@ -1,15 +1,19 @@
 package cuchaz.enigma.translation.mapping;
 
 import java.util.Collection;
-import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import cuchaz.enigma.analysis.index.InheritanceIndex;
 import cuchaz.enigma.analysis.index.JarIndex;
 import cuchaz.enigma.translation.Translator;
 import cuchaz.enigma.translation.mapping.tree.EntryTree;
+import cuchaz.enigma.translation.representation.AccessFlags;
 import cuchaz.enigma.translation.representation.entry.ClassEntry;
+import cuchaz.enigma.translation.representation.entry.DefEntry;
 import cuchaz.enigma.translation.representation.entry.Entry;
+import cuchaz.enigma.translation.representation.entry.FieldEntry;
 import cuchaz.enigma.utils.validation.Message;
 import cuchaz.enigma.utils.validation.ValidationContext;
 
@@ -35,18 +39,16 @@ public class MappingValidator {
 
         private void validateUnique(ValidationContext vc, Entry<?> entry, String name) {
                 ClassEntry containingClass = entry.getContainingClass();
-                Collection<ClassEntry> relatedClasses = getRelatedClasses(containingClass);
-
-                for (ClassEntry relatedClass : relatedClasses) {
-                        Entry<?> relatedEntry = entry.replaceAncestor(containingClass, relatedClass);
-                        Entry<?> translatedEntry = deobfuscator.translate(relatedEntry);
-
-                        Collection<Entry<?>> translatedSiblings = obfToDeobf.getSiblings(relatedEntry).stream()
-                                        .map(deobfuscator::translate)
-                                        .collect(Collectors.toList());
+                InheritanceIndex inheritanceIndex = index.getInheritanceIndex();
 
-                        if (!isUnique(translatedEntry, translatedSiblings, name)) {
-                                Entry<?> parent = translatedEntry.getParent();
+                //Step 1, check it's unique within its own siblings
+                Collection<Entry<?>> directTranslatedSiblings = obfToDeobf.getSiblings(entry).stream()
+                                .map(deobfuscator::translate)
+                                .collect(Collectors.toList());
+                for (Entry<?> sibling : directTranslatedSiblings) {
+                        if (entry.canConflictWith(sibling) && sibling.getName().equals(name) && isSynthetic(entry) == isSynthetic(sibling)) {
+                                // allow clash if one is synthetic and the other is not
+                                Entry<?> parent = entry.getParent();
                                 if (parent != null) {
                                         vc.raise(Message.NONUNIQUE_NAME_CLASS, name, parent);
                                 } else {
@@ -54,26 +56,88 @@ public class MappingValidator {
                                 }
                         }
                 }
-        }
 
-        private Collection<ClassEntry> getRelatedClasses(ClassEntry classEntry) {
-                InheritanceIndex inheritanceIndex = index.getInheritanceIndex();
+                //Step 2, check ancestors, ignoring members invisible to children
+                Set<ClassEntry> ancestors = inheritanceIndex.getAncestors(containingClass);
+                for (ClassEntry ancestor : ancestors) {
+                        Entry<?> reparentedEntry = entry.replaceAncestor(containingClass, ancestor);
+                        Entry<?> translatedEntry = Objects.requireNonNull(deobfuscator.translate(reparentedEntry), "Translation failed");
+                        Collection<Entry<?>> translatedSiblings = obfToDeobf.getSiblings(reparentedEntry).stream()
+                                        .filter(it->!entry.equals(it))//e.g. for root classes, ensure we dont match the name against itself
+                                        .filter(this::isVisibleToChildren)
+                                        .collect(Collectors.toList());
+                        for (Entry<?> parentSibling : translatedSiblings) {
+                                Entry<?> parentSiblingTranslated = Objects.requireNonNull(deobfuscator.translate(parentSibling), "Translation failed");
+                                if (translatedEntry.canConflictWith(parentSiblingTranslated) && parentSiblingTranslated.getName().equals(name) && !isAcceptableOverride(parentSibling, entry)) {
+                                        Entry<?> parent = translatedEntry.getParent();
+                                        if (parent != null) {
+                                                vc.raise(Message.NONUNIQUE_NAME_CLASS, name, parent);
+                                        } else {
+                                                vc.raise(Message.NONUNIQUE_NAME, name);
+                                        }
+                                }
+                        }
+                }
 
-                Collection<ClassEntry> relatedClasses = new HashSet<>();
-                relatedClasses.add(classEntry);
-                relatedClasses.addAll(inheritanceIndex.getChildren(classEntry));
-                relatedClasses.addAll(inheritanceIndex.getAncestors(classEntry));
+                //Step 3, if this entry is visible to children, see if it clashes with any of their names
+                if (isVisibleToChildren(entry)) {
+                        Collection<ClassEntry> children = inheritanceIndex.getDescendants(containingClass);
+                        for (ClassEntry child : children) {
+                                Entry<?> reparentedEntry = entry.replaceAncestor(containingClass, child);
+                                Entry<?> translatedEntry = Objects.requireNonNull(deobfuscator.translate(reparentedEntry), "Translation failed");
+                                Collection<Entry<?>> siblings = obfToDeobf.getSiblings(reparentedEntry).stream()
+                                                .filter(it->!entry.equals(it))//e.g. for root classes, ensure we dont match the name against itself
+                                                .collect(Collectors.toList());
+                                for (Entry<?> childSibling : siblings) {
+                                        Entry<?> childSiblingTranslated = Objects.requireNonNull(deobfuscator.translate(childSibling), "Translation failed");
+                                        if (translatedEntry.canConflictWith(childSiblingTranslated) && childSiblingTranslated.getName().equals(name) && !isAcceptableOverride(entry, childSibling)) {
+                                                Entry<?> parent = translatedEntry.getParent();
+                                                if (parent != null) {
+                                                        vc.raise(Message.NONUNIQUE_NAME_CLASS, name, parent);
+                                                } else {
+                                                        vc.raise(Message.NONUNIQUE_NAME, name);
+                                                }
+                                        }
+                                }
+                        }
+                }
+        }
 
-                return relatedClasses;
+        private boolean isVisibleToChildren(Entry<?> entry) {
+                if (entry instanceof DefEntry) {
+                        return !((DefEntry<?>) entry).getAccess().isPrivate();
+                }
+                AccessFlags accessFlags = index.getEntryIndex().getEntryAccess(entry);
+                if (accessFlags != null) {
+                        return !accessFlags.isPrivate();
+                }
+                return true;//unknown, assume yes
         }
 
-        private boolean isUnique(Entry<?> entry, Collection<Entry<?>> siblings, String name) {
-                for (Entry<?> sibling : siblings) {
-                        if (entry.canConflictWith(sibling) && sibling.getName().equals(name)) {
-                                return false;
-                        }
+        private boolean isAcceptableOverride(Entry<?> ancestor, Entry<?> descendent) {
+                if (ancestor instanceof FieldEntry && descendent instanceof FieldEntry){
+                        return true;//fields don't apply here
+                }
+
+                AccessFlags ancestorFlags = findAccessFlags(ancestor);
+                AccessFlags descendentFlags = findAccessFlags(descendent);
+
+                if (ancestorFlags == null || descendentFlags == null) {
+                        return false;//we can't make any assumptions
                 }
-                return true;
+
+                //bad == accessLevel < superAccessLevel
+                return !(descendentFlags.getAccessLevel() < ancestorFlags.getAccessLevel());
+        }
+
+        private boolean isSynthetic(Entry<?> entry) {
+                AccessFlags accessFlags = findAccessFlags(entry);
+                return accessFlags != null && accessFlags.isSynthetic();
+        }
+
+        private AccessFlags findAccessFlags(Entry<?> entry) {
+                return (entry instanceof DefEntry) ? ((DefEntry<?>) entry).getAccess() : index.getEntryIndex()
+                                .getEntryAccess(entry);
         }
 
 }
diff --git a/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java b/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java
index b280eef2..aa48a5b2 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java
@@ -8,6 +8,10 @@ import java.lang.reflect.Modifier;
 public class AccessFlags {
         public static final AccessFlags PRIVATE = new AccessFlags(Opcodes.ACC_PRIVATE);
         public static final AccessFlags PUBLIC = new AccessFlags(Opcodes.ACC_PUBLIC);
+        public static final int ACCESS_LEVEL_PUBLIC = 4;
+        public static final int ACCESS_LEVEL_PROTECTED = 3;
+        public static final int ACCESS_LEVEL_PACKAGE_LOCAL = 2;
+        public static final int ACCESS_LEVEL_PRIVATE = 1;
 
         private int flags;
 
@@ -89,6 +93,23 @@ public class AccessFlags {
                 return this.flags;
         }
 
+        /**
+         * Adapted from https://github.com/JetBrains/intellij-community/blob/6472c347db91d11bbf02895a767198f9d884b119/java/java-psi-api/src/com/intellij/psi/util/PsiUtil.java#L389
+         * @return visibility access level on a 'weakness scale'
+         */
+        public int getAccessLevel() {
+                if (isPrivate()) {
+                        return ACCESS_LEVEL_PRIVATE;
+                }
+                if (isProtected()) {
+                        return ACCESS_LEVEL_PROTECTED;
+                }
+                if (isPublic()) {
+                        return ACCESS_LEVEL_PUBLIC;
+                }
+                return ACCESS_LEVEL_PACKAGE_LOCAL;
+        }
+
         @Override
         public boolean equals(Object obj) {
                 return obj instanceof AccessFlags && ((AccessFlags) obj).flags == flags;