Allow shadowing fields (#475)

* Fix Enigma not allowing to shadow static fields

* Fix mapping uniqueness verification

* Allow shadowing any field

* Fix a message

Co-authored-by: ByMartrixx <bymartrixx@gmail.com>

8 files changed, 72 insertions, 3 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 5f42373c..f6c0a6b8 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java
@@ -4,10 +4,13 @@ import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 
+import javax.annotation.Nullable;
+
 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.Entry;
 import cuchaz.enigma.utils.validation.Message;
@@ -41,12 +44,20 @@ public class MappingValidator {
                 Collection<ClassEntry> relatedClasses = getRelatedClasses(containingClass);
 
                 boolean error = false;
+                Entry<?> shadowedEntry;
 
                 for (ClassEntry relatedClass : relatedClasses) {
+                        if (isStatic(entry) && relatedClass != containingClass) {
+                                // static entries can only conflict with entries in the same class
+                                continue;
+                        }
+
                         Entry<?> relatedEntry = entry.replaceAncestor(containingClass, relatedClass);
                         Entry<?> translatedEntry = deobfuscator.translate(relatedEntry);
 
-                        List<? extends Entry<?>> translatedSiblings = obfToDeobf.getSiblings(relatedEntry).stream().map(deobfuscator::translate).toList();
+                        List<? extends Entry<?>> translatedSiblings = obfToDeobf.getSiblings(relatedEntry).stream()
+                                        .map(deobfuscator::translate)
+                                        .toList();
 
                         if (!isUnique(translatedEntry, translatedSiblings, name)) {
                                 Entry<?> parent = translatedEntry.getParent();
@@ -58,6 +69,14 @@ public class MappingValidator {
                                 }
 
                                 error = true;
+                        } else if ((shadowedEntry = getShadowedEntry(translatedEntry, translatedSiblings, name)) != null) {
+                                Entry<?> parent = shadowedEntry.getParent();
+
+                                if (parent != null) {
+                                        vc.raise(Message.SHADOWED_NAME_CLASS, name, parent);
+                                } else {
+                                        vc.raise(Message.SHADOWED_NAME, name);
+                                }
                         }
                 }
 
@@ -77,11 +96,35 @@ public class MappingValidator {
 
         private boolean isUnique(Entry<?> entry, List<? extends Entry<?>> siblings, String name) {
                 for (Entry<?> sibling : siblings) {
-                        if (entry.canConflictWith(sibling) && sibling.getName().equals(name)) {
+                        if (canConflict(entry, sibling) && sibling.getName().equals(name)) {
                                 return false;
                         }
                 }
 
                 return true;
         }
+
+        private boolean canConflict(Entry<?> entry, Entry<?> sibling) {
+                return entry.canConflictWith(sibling);
+        }
+
+        @Nullable
+        private Entry<?> getShadowedEntry(Entry<?> entry, List<? extends Entry<?>> siblings, String name) {
+                for (Entry<?> sibling : siblings) {
+                        if (canShadow(entry, sibling) && sibling.getName().equals(name)) {
+                                return sibling;
+                        }
+                }
+
+                return null;
+        }
+
+        private boolean canShadow(Entry<?> entry, Entry<?> sibling) {
+                return entry.canShadow(sibling);
+        }
+
+        private boolean isStatic(Entry<?> entry) {
+                AccessFlags accessFlags = index.getEntryIndex().getEntryAccess(entry);
+                return accessFlags != null && accessFlags.isStatic();
+        }
 }
diff --git a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/ClassEntry.java b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/ClassEntry.java
index be5f0b38..8659b402 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/ClassEntry.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/ClassEntry.java
@@ -122,6 +122,11 @@ public class ClassEntry extends ParentedEntry<ClassEntry> implements Comparable<
         }
 
         @Override
+        public boolean canShadow(Entry<?> entry) {
+                return false;
+        }
+
+        @Override
         public void validateName(ValidationContext vc, String name) {
                 IdentifierValidation.validateClassName(vc, name, this.isInnerClass());
         }
diff --git a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/Entry.java b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/Entry.java
index 9615ca8e..71425e0f 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/Entry.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/Entry.java
@@ -108,6 +108,8 @@ public interface Entry<P extends Entry<?>> extends Translatable {
 
         boolean canConflictWith(Entry<?> entry);
 
+        boolean canShadow(Entry<?> entry);
+
         default ClassEntry getContainingClass() {
                 ClassEntry last = null;
                 Entry<?> current = this;
diff --git a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/FieldEntry.java b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/FieldEntry.java
index c1592a43..b4ad1bac 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/FieldEntry.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/FieldEntry.java
@@ -85,7 +85,12 @@ public class FieldEntry extends ParentedEntry<ClassEntry> implements Comparable<
 
         @Override
         public boolean canConflictWith(Entry<?> entry) {
-                return entry instanceof FieldEntry && ((FieldEntry) entry).parent.equals(parent);
+                return false;
+        }
+
+        @Override
+        public boolean canShadow(Entry<?> entry) {
+                return entry instanceof FieldEntry;
         }
 
         @Override
diff --git a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/LocalVariableEntry.java b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/LocalVariableEntry.java
index d22188b2..9d0bbedd 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/LocalVariableEntry.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/LocalVariableEntry.java
@@ -85,6 +85,11 @@ public class LocalVariableEntry extends ParentedEntry<MethodEntry> implements Co
         }
 
         @Override
+        public boolean canShadow(Entry<?> entry) {
+                return false;
+        }
+
+        @Override
         public String toString() {
                 return this.parent + "(" + this.index + ":" + this.name + ")";
         }
diff --git a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/MethodEntry.java b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/MethodEntry.java
index 6fc3f0a0..5ccb9081 100644
--- a/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/MethodEntry.java
+++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/entry/MethodEntry.java
@@ -98,6 +98,11 @@ public class MethodEntry extends ParentedEntry<ClassEntry> implements Comparable
         }
 
         @Override
+        public boolean canShadow(Entry<?> entry) {
+                return entry instanceof MethodEntry;
+        }
+
+        @Override
         public String toString() {
                 return this.getFullName() + this.descriptor;
         }
diff --git a/enigma/src/main/java/cuchaz/enigma/utils/validation/Message.java b/enigma/src/main/java/cuchaz/enigma/utils/validation/Message.java
index b7e67f2b..9dbbb187 100644
--- a/enigma/src/main/java/cuchaz/enigma/utils/validation/Message.java
+++ b/enigma/src/main/java/cuchaz/enigma/utils/validation/Message.java
@@ -17,6 +17,8 @@ public class Message {
         public static final Message UNKNOWN_RECORD_GETTER = create(Type.ERROR, "unknown_record_getter");
 
         public static final Message STYLE_VIOLATION = create(Type.WARNING, "style_violation");
+        public static final Message SHADOWED_NAME_CLASS = create(Type.WARNING, "shadowed_name_class");
+        public static final Message SHADOWED_NAME = create(Type.WARNING, "shadowed_name");
 
         public final Type type;
         public final String textKey;
diff --git a/enigma/src/main/resources/lang/en_us.json b/enigma/src/main/resources/lang/en_us.json
index 82f2d371..3919b9e2 100644
--- a/enigma/src/main/resources/lang/en_us.json
+++ b/enigma/src/main/resources/lang/en_us.json
@@ -217,6 +217,8 @@
         "validation.message.illegal_doc_comment_end": "Javadoc comment cannot contain the character sequence '*/'.",
         "validation.message.reserved_identifier": "'%s' is a reserved identifier.",
         "validation.message.unknown_record_getter": "Could not find a matching record component getter for %s",
+        "validation.message.shadowed_name_class": "Name '%s' shadows another in '%s'.",
+        "validation.message.shadowed_name": "Name '%s' shadows another entry.",
 
         "crash.title": "%s - Crash Report",
         "crash.summary": "%s has crashed! =(",