7 files changed, 27 insertions, 257 deletions

diff --git a/enigma/build.gradle b/enigma/build.gradle
index 2c5d329f..7cebcd16 100644
--- a/enigma/build.gradle
+++ b/enigma/build.gradle
@@ -53,7 +53,7 @@ file('src/test/java/cuchaz/enigma/inputs').listFiles().each { theFile ->
 
             args '@src/test/resources/proguard-test.conf', '-injars', file('build/test-inputs/' +
                     "${theFile.name}.jar"), '-libraryjars', libraryJarsArg,
-                    '-outjars', file('build/test-obf/' + "${theFile.name}.jar"), '-printmapping', file("build/${theFile.name}-mapping.txt")
+                    '-outjars', file('build/test-obf/' + "${theFile.name}.jar")
         }
 
         test.dependsOn "${theFile.name}TestObf"
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 a84b0fb3..f9f3b88e 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java
@@ -1,19 +1,15 @@
 package cuchaz.enigma.translation.mapping;
 
 import java.util.Collection;
-import java.util.Objects;
-import java.util.Set;
+import java.util.HashSet;
 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;
 
@@ -39,16 +35,18 @@ public class MappingValidator {
 
         private void validateUnique(ValidationContext vc, Entry<?> entry, String name) {
                 ClassEntry containingClass = entry.getContainingClass();
-                InheritanceIndex inheritanceIndex = index.getInheritanceIndex();
+                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());
 
-                //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 (!isUnique(translatedEntry, translatedSiblings, name)) {
+                                Entry<?> parent = translatedEntry.getParent();
                                 if (parent != null) {
                                         vc.raise(Message.NONUNIQUE_NAME_CLASS, name, parent);
                                 } else {
@@ -56,88 +54,26 @@ public class MappingValidator {
                                 }
                         }
                 }
-
-                //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);
-                                        }
-                                }
-                        }
-                }
-
-                //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);
-                                                }
-                                        }
-                                }
-                        }
-                }
         }
 
-        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 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
-                }
+        private Collection<ClassEntry> getRelatedClasses(ClassEntry classEntry) {
+                InheritanceIndex inheritanceIndex = index.getInheritanceIndex();
 
-                //bad == accessLevel < superAccessLevel
-                return !(descendentFlags.getAccessLevel() < ancestorFlags.getAccessLevel());
-        }
+                Collection<ClassEntry> relatedClasses = new HashSet<>();
+                relatedClasses.add(classEntry);
+                relatedClasses.addAll(inheritanceIndex.getChildren(classEntry));
+                relatedClasses.addAll(inheritanceIndex.getAncestors(classEntry));
 
-        private boolean isSynthetic(Entry<?> entry) {
-                AccessFlags accessFlags = findAccessFlags(entry);
-                return accessFlags != null && accessFlags.isSynthetic();
+                return relatedClasses;
         }
 
-        private AccessFlags findAccessFlags(Entry<?> entry) {
-                return (entry instanceof DefEntry) ? ((DefEntry<?>) entry).getAccess() : index.getEntryIndex()
-                                .getEntryAccess(entry);
+        private boolean isUnique(Entry<?> entry, Collection<Entry<?>> siblings, String name) {
+                for (Entry<?> sibling : siblings) {
+                        if (entry.canConflictWith(sibling) && sibling.getName().equals(name)) {
+                                return false;
+                        }
+                }
+                return true;
         }
 
 }
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 aa48a5b2..b280eef2 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java
@@ -8,10 +8,6 @@ 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;
 
@@ -93,23 +89,6 @@ 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;
diff --git a/enigma/src/test/java/cuchaz/enigma/TestAllowableClashes.java b/enigma/src/test/java/cuchaz/enigma/TestAllowableClashes.java
deleted file mode 100644
index ce82e380..00000000
--- a/enigma/src/test/java/cuchaz/enigma/TestAllowableClashes.java
+++ /dev/null
@@ -1,51 +0,0 @@
-package cuchaz.enigma;
-
-import java.io.IOException;
-import java.nio.file.Paths;
-
-import org.hamcrest.MatcherAssert;
-import org.junit.Assert;
-import org.junit.Test;
-
-import cuchaz.enigma.classprovider.ClasspathClassProvider;
-import cuchaz.enigma.translation.mapping.EntryMapping;
-import cuchaz.enigma.translation.mapping.EntryRemapper;
-import cuchaz.enigma.translation.mapping.serde.MappingFormat;
-import cuchaz.enigma.translation.mapping.serde.MappingParseException;
-import cuchaz.enigma.translation.mapping.tree.EntryTree;
-import cuchaz.enigma.translation.mapping.tree.EntryTreeNode;
-import cuchaz.enigma.translation.representation.entry.MethodEntry;
-import cuchaz.enigma.utils.validation.ValidationContext;
-
-/**
- * Test that we can accept some name clashes that are allowed by javac
- */
-public class TestAllowableClashes {
-
-        @Test
-        public void test() throws IOException, MappingParseException {
-                //Load produced mappings
-                Enigma enigma = Enigma.create();
-                EnigmaProject project = enigma.openJar(Paths.get("build/test-obf/visibility.jar"), new ClasspathClassProvider(), ProgressListener.none());
-                EntryTree<EntryMapping> obfToDeobf = MappingFormat.PROGUARD.read(Paths.get("build/visibility-mapping.txt"), ProgressListener.none(), null);
-
-                //Load them into enigma, none should conflict
-                EntryRemapper mapper = project.getMapper();
-                for (int round=0; round<2; round++) {
-                        for (EntryTreeNode<EntryMapping> node : obfToDeobf) {
-                                Assert.assertNotEquals(null, node.getValue());
-                                if (node.getEntry() instanceof MethodEntry && (node.getEntry()
-                                                .getName()
-                                                .equals("<init>") || node.getEntry().getName().equals("<clinit>"))) {
-                                        //skip proguard's constructor entries
-                                        continue;
-                                }
-                                System.out.println(node.getEntry().toString() + " -> " + node.getValue().getTargetName());
-                                ValidationContext vc = new ValidationContext();
-                                mapper.mapFromObf(vc, node.getEntry(), node.getValue());
-                                MatcherAssert.assertThat(vc, ValidationContextMatcher.INSTANCE);
-                        }
-                }
-        }
-
-}
diff --git a/enigma/src/test/java/cuchaz/enigma/ValidationContextMatcher.java b/enigma/src/test/java/cuchaz/enigma/ValidationContextMatcher.java
deleted file mode 100644
index 5404077f..00000000
--- a/enigma/src/test/java/cuchaz/enigma/ValidationContextMatcher.java
+++ /dev/null
@@ -1,35 +0,0 @@
-package cuchaz.enigma;
-
-import cuchaz.enigma.utils.validation.ParameterizedMessage;
-import cuchaz.enigma.utils.validation.ValidationContext;
-import org.hamcrest.CustomMatcher;
-import org.hamcrest.Description;
-
-class ValidationContextMatcher extends CustomMatcher<ValidationContext> {
-        public static final ValidationContextMatcher INSTANCE = new ValidationContextMatcher();
-
-        private ValidationContextMatcher() {
-                super("ValidationContext can proceed");
-        }
-
-        @Override
-        public boolean matches(Object item) {
-                return item instanceof ValidationContext && ((ValidationContext) item).canProceed();
-        }
-
-        @Override
-        public void describeMismatch(Object item, Description description) {
-                if (!(item instanceof ValidationContext)) {
-                        description.appendText("expected ValidationContext, was").appendValue(item);
-                        return;
-                }
-                ValidationContext vc = (ValidationContext) item;
-                for (ParameterizedMessage message : vc.getMessages()) {
-                        description.appendText(message.getText());
-                        String longMessage = message.getLongText();
-                        if (longMessage != null && !longMessage.trim().isEmpty()){
-                                description.appendText(longMessage);
-                        }
-                }
-        }
-}
diff --git a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassA.java b/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassA.java
deleted file mode 100644
index 458f7c0d..00000000
--- a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassA.java
+++ /dev/null
@@ -1,28 +0,0 @@
-package cuchaz.enigma.inputs.visibility;
-
-class ClassA {
-
-        protected Object protectedParentPrivateChild;
-        public Object publicParentPrivateChild;
-
-        public static Object LOGGER = null;
-
-        protected static Object LOGGER2 = null;
-
-        public Object publicPublic;
-
-        public static void equalAccessStatic() {}
-
-        protected static void protectedPublicStatic(){}
-
-        private static void privateStaticParentPublicStaticChild(){}
-
-        private void privateParentPublicStaticChild() {}
-
-        static void packagePrivateParentProtectedChild(){}
-
-        private static void packagePrivateChild(){}
-
-        public ClassA returningSubclass(){return null;}
-
-}
\ No newline at end of file
diff --git a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassB.java b/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassB.java
deleted file mode 100644
index 2a1b68a7..00000000
--- a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassB.java
+++ /dev/null
@@ -1,31 +0,0 @@
-package cuchaz.enigma.inputs.visibility;
-
-public class ClassB extends ClassA {
-        private Object protectedParentPrivateChild;
-
-        private Object publicParentPrivateChild;
-
-        public Object publicPublic;
-
-        public static Object LOGGER;
-
-        public static Object LOGGER2 = null;
-
-        public static void equalAccessStatic() {
-        }
-
-        public static void protectedPublicStatic() {
-        }
-
-        public static void privateStaticParentPublicStaticChild() {
-        }
-
-        public static void privateParentPublicStaticChild() {
-        }
-
-        protected static void packagePrivateParentProtectedChild(){}
-
-        static void packagePrivateChild(){}
-
-        public ClassB returningSubclass(){return null;}
-}