Merge pull request #14 from QuiltMC/disaster

automatically fix conflicts with dynamically proposed argument names

src/main/java/org/quiltmc/enigma_plugin/Arguments.java

@@ -32,6 +32,7 @@ public class Arguments {
 	public static final String DISABLE_CODECS = "disable_codecs";
 	public static final String DISABLE_MAP_NON_HASHED = "disable_map_non_hashed";
 	public static final String DISABLE_DELEGATE_PARAMS = "disable_delegate_params";
+	public static final String DISABLE_CONFLICT_FIXER = "disable_conflict_fixer";
 	public static final String CUSTOM_CODECS = "custom_codecs";
 	public static final String SIMPLE_TYPE_FIELD_NAMES_PATH = "simple_type_field_names_path";
 

src/main/java/org/quiltmc/enigma_plugin/index/simple_type_single/SimpleTypeSingleIndex.java

@@ -54,6 +54,7 @@
  */
 public class SimpleTypeSingleIndex extends Index {
 	private final Map<LocalVariableEntry, String> parameters = new HashMap<>();
+	private final Map<LocalVariableEntry, List<String>> parameterFallbacks = new HashMap<>();
 	private final Map<FieldEntry, String> fields = new HashMap<>();
 	private final Map<ClassNode, Map<String, FieldBuildingEntry>> fieldCache = new HashMap<>();
 	private SimpleTypeFieldNamesRegistry registry;
@@ -104,6 +105,10 @@ public void dropCache() {
 		return this.parameters.get(paramEntry);
 	}
 
+	public @Nullable List<String> getParamFallbacks(LocalVariableEntry paramEntry) {
+		return this.parameterFallbacks.get(paramEntry);
+	}
+
 	public Set<FieldEntry> getFields() {
 		return this.fields.keySet();
 	}
@@ -179,6 +184,7 @@ public void visitClassNode(ClassProvider provider, ClassNode node) {
 					int index = param.index() + (isStatic ? 0 : 1);
 					var paramEntry = new LocalVariableEntry(methodEntry, index);
 					this.parameters.put(paramEntry, name);
+					this.parameterFallbacks.put(paramEntry, param.entry.fallback().stream().map(Name::local).toList());
 				}
 			});
 		}

src/main/java/org/quiltmc/enigma_plugin/proposal/ConflictFixProposer.java

@@ -0,0 +1,101 @@
+/*
+ * Copyright 2024 QuiltMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.quiltmc.enigma_plugin.proposal;
+
+import org.quiltmc.enigma.api.analysis.index.jar.EntryIndex;
+import org.quiltmc.enigma.api.analysis.index.jar.JarIndex;
+import org.quiltmc.enigma.api.translation.mapping.EntryMapping;
+import org.quiltmc.enigma.api.translation.mapping.EntryRemapper;
+import org.quiltmc.enigma.api.translation.representation.entry.Entry;
+import org.quiltmc.enigma.api.translation.representation.entry.LocalVariableEntry;
+import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;
+import org.quiltmc.enigma_plugin.index.JarIndexer;
+import org.quiltmc.enigma_plugin.index.simple_type_single.SimpleTypeSingleIndex;
+
+import java.util.Map;
+import java.util.Optional;
+
+public class ConflictFixProposer extends NameProposer {
+	public static final String ID = "conflict_fix";
+	private final SimpleTypeSingleIndex index;
+
+	public ConflictFixProposer(JarIndexer jarIndex) {
+		super(ID);
+		this.index = jarIndex.getIndex(SimpleTypeSingleIndex.class);
+	}
+
+	@Override
+	public void proposeDynamicNames(EntryRemapper remapper, Entry<?> obfEntry, EntryMapping oldMapping, EntryMapping newMapping, Map<Entry<?>, EntryMapping> mappings) {
+		for (Map.Entry<Entry<?>, EntryMapping> entry : mappings.entrySet()) {
+			this.fixConflicts(mappings, remapper, entry.getKey(), entry.getValue());
+		}
+	}
+
+	private void fixConflicts(Map<Entry<?>, EntryMapping> mappings, EntryRemapper remapper, Entry<?> entry, EntryMapping mapping) {
+		if (entry instanceof LocalVariableEntry param && mapping != null) {
+			this.fixParamConflicts(mappings, remapper, param, mapping);
+		}
+	}
+
+	private void fixParamConflicts(Map<Entry<?>, EntryMapping> mappings, EntryRemapper remapper, LocalVariableEntry entry, EntryMapping mapping) {
+		String name = mapping.targetName();
+		Optional<LocalVariableEntry> conflict = this.getConflictingParam(remapper, entry, name);
+
+		while (conflict.isPresent()) {
+			LocalVariableEntry conflictEntry = conflict.get();
+			var fallbacks = this.index.getParamFallbacks(conflictEntry);
+
+			boolean fixed = false;
+
+			if (fallbacks != null) {
+				for (String fallbackName : fallbacks) {
+					Optional<LocalVariableEntry> newConflict = this.getConflictingParam(remapper, conflictEntry, fallbackName);
+					if (newConflict.isEmpty()) {
+						this.insertDynamicProposal(mappings, conflictEntry, fallbackName);
+						conflict = this.getConflictingParam(remapper, conflictEntry, name);
+						fixed = true;
+						break;
+					}
+				}
+			}
+
+			if (!fixed) {
+				this.insertDynamicProposal(mappings, conflictEntry, (String) null);
+			}
+		}
+	}
+
+	private Optional<LocalVariableEntry> getConflictingParam(EntryRemapper remapper, LocalVariableEntry entry, String name) {
+		MethodEntry method = entry.getParent();
+		if (method != null) {
+			var args = method.getParameterIterator(remapper.getJarIndex().getIndex(EntryIndex.class), remapper.getDeobfuscator());
+
+			while (args.hasNext()) {
+				LocalVariableEntry arg = args.next();
+				if (arg.getIndex() != entry.getIndex() && arg.getName().equals(name)) {
+					return Optional.of(arg);
+				}
+			}
+		}
+
+		return Optional.empty();
+	}
+
+	@Override
+	public void insertProposedNames(JarIndex index, Map<Entry<?>, EntryMapping> mappings) {
+	}
+}

src/main/java/org/quiltmc/enigma_plugin/proposal/MojangNameProposer.java

@@ -50,7 +50,7 @@ public void insertProposedNames(JarIndex index, Map<Entry<?>, EntryMapping> mapp
 
 		for (MethodEntry method : entryIndex.getMethods()) {
 			String name = method.getName();
-			if (!name.startsWith("m_")) {
+			if (!name.startsWith("m_") && !method.isConstructor()) {
 				this.insertProposal(mappings, method, name);
 			}
 		}

src/main/java/org/quiltmc/enigma_plugin/proposal/NameProposerService.java

@@ -52,6 +52,9 @@ public NameProposerService(JarIndexer indexer, EnigmaServiceContext<NameProposal
 		this.addIfEnabled(context, indexer, Arguments.DISABLE_CONSTRUCTOR_PARAMS, ConstructorParamsNameProposer::new);
 		this.addIfEnabled(context, indexer, Arguments.DISABLE_GETTER_SETTER, GetterSetterNameProposer::new);
 		this.addIfEnabled(context, indexer, Arguments.DISABLE_DELEGATE_PARAMS, DelegateParametersNameProposer::new);
+
+		// conflict fixer must be last in order to get context from other dynamic proposers
+		this.addIfEnabled(context, indexer, Arguments.DISABLE_CONFLICT_FIXER, ConflictFixProposer::new);
 	}
 
 	private void addIfEnabled(EnigmaServiceContext<NameProposalService> context, String name, Supplier<NameProposer> factory) {
@@ -65,8 +68,12 @@ private void addIfEnabled(EnigmaServiceContext<NameProposalService> context, Jar
 	}
 
 	private void addIfNotDisabled(EnigmaServiceContext<NameProposalService> context, String name, Supplier<NameProposer> factory) {
+		this.addIfNotDisabled(context, null, name, indexer -> factory.get());
+	}
+
+	private void addIfNotDisabled(EnigmaServiceContext<NameProposalService> context, JarIndexer indexer, String name, Function<JarIndexer, NameProposer> factory) {
 		if (!Arguments.getBoolean(context, name, true)) {
-			this.nameProposers.add(factory.get());
+			this.nameProposers.add(factory.apply(indexer));
 		}
 	}
 

src/main/java/org/quiltmc/enigma_plugin/util/AsmUtil.java

@@ -185,30 +185,34 @@ public static AbstractInsnNode searchInsnInStack(InsnList insns, AbstractInsnNod
 			}
 		}
 
-		if (!shallow && lastStackInsn != null && lastStackInsn.getOpcode() == NEW && lastStackInsn.getNext() != null && lastStackInsn.getNext().getOpcode() == DUP) {
-			// Find the last frame containing the DUP instruction
-			var dup = lastStackInsn.getNext();
-			int searchFrameIndex = insns.indexOf(dup) + 1;
+		if (!shallow && lastStackInsn != null && lastStackInsn.getOpcode() == DUP && lastStackInsn.getPrevious() != null && lastStackInsn.getPrevious().getOpcode() == NEW) {
+			// Find the last frame containing two DUP instructions in the stack
+			// This used to search a single DUP following a NEW, but ASM commit 172221565c4347060d79285f183cdbca72344616
+			// changed the behavior for DUPS to replace the previous value
+			// Stack before: ..., NEW ..., DUP; after: ..., DUP, DUP
+			int searchFrameIndex = insns.indexOf(lastStackInsn) + 1;
 			var searchFrame = frames[searchFrameIndex];
 
 			while (searchFrame != null && searchFrameIndex <= frameIndex) {
-				boolean contains = false;
+				int count = 0;
 				for (int j = 0; j < searchFrame.getStackSize(); j++) {
-					if (frame.getStack(j).insns.contains(dup)) {
-						contains = true;
-						break;
+					if (frame.getStack(j).insns.contains(lastStackInsn)) {
+						count++;
+						if (count == 2) {
+							break;
+						}
 					}
 				}
 
-				if (contains) {
+				if (count == 2) {
 					searchFrameIndex++;
 					if (searchFrameIndex < frames.length) {
 						searchFrame = frames[searchFrameIndex];
 					} else {
 						searchFrame = null;
 					}
 				} else {
-					var insn = insns.get(searchFrameIndex - 1); // This was the last instruction with a frame with a dup
+					var insn = insns.get(searchFrameIndex - 1); // This was the last instruction with a frame with two dups
 					if (insn != frameInsn) {
 						return searchInsnInStack(insns, insn, frames, insnPredicate, false);
 					}

src/test/java/org/quiltmc/enigma_plugin/NameProposalTest.java

@@ -289,6 +289,26 @@ public void testSimpleTypeSingleNames() {
 		assertProposal("valueD", localVar(parent, 0));
 	}
 
+	@Test
+	public void testSimpleTypeNameConflictFix() {
+		// tests the conflict fixer via introducing a conflict manually
+
+		var owner = new ClassEntry("com/a/c/a");
+		var constructor = method(owner, "<init>", "(ILjava/lang/CharSequence;)V");
+
+		// param 2 is initially 'id'
+		assertProposal("id", localVar(constructor, 2));
+
+		// fires dynamic proposal for the constructor parameter, creating a conflict
+		// the conflict should then be automatically fixed by moving to the 'identifier' name
+		// note we bypass putMapping so that we can create a conflict
+		remapper.getMappings().insert(field(owner, "a", "I"), new EntryMapping("id"));
+		remapper.insertDynamicallyProposedMappings(field(owner, "a", "I"), EntryMapping.OBFUSCATED, new EntryMapping("id"));
+
+		assertDynamicProposal("id", localVar(constructor, 1));
+		assertDynamicProposal("identifier", localVar(constructor, 2));
+	}
+
 	@Test
 	public void testDelegateParameterNames() {
 		var classEntry = new ClassEntry("com/a/b");

src/testInputs/java/com/example/GetterSetterTest.java

@@ -6,6 +6,11 @@ public class GetterSetterTest {
 	public int x;
 	public String y;
 
+	public GetterSetterTest(int x, String y) {
+		this.x = x;
+		this.y = y;
+	}
+
 	public int getX() {
 		return this.x;
 	}

src/testInputs/java/com/example/z_conflicts/ConflictTest.java

@@ -0,0 +1,11 @@
+package com.example.z_conflicts;
+
+public class ConflictTest {
+	public final int a;
+	public final CharSequence b;
+
+	public ConflictTest(int a, CharSequence b) {
+		this.a = a;
+		this.b = b;
+	}
+}

src/testInputs/resources/simple_type_field_names.json5

@@ -3,6 +3,14 @@
 
     "com/a/b/a": "config",
 
+    "java/lang/CharSequence": {
+        local_name: "id",
+        exclusive: true,
+        fallback: [
+            "identifier"
+        ]
+    },
+
     // Position
     "com/a/b/b": "pos", // Pos
     "com/a/b/c": { // Position