2.0 API and impl changes for name proposal (#168)

* Add a getParameters method in MethodEntry * Partial MergedEntryMappingTree implementation * Fix checkstyle errors * migrate EntryRemapper to MergedEntryMappingTree * Add a dialog to test mapping trees * It works?? * Fix broken test * Fixes for Mark as deobf/obf * Fix close button in print mapping tree dialog * Apply suggestions from code review Co-authored-by: ix0rai <[email protected]> --------- Co-authored-by: ix0rai <[email protected]>
QuiltMC · Dec 21, 2023 · c7e56d4 · c7e56d4
1 parent 8c2bd14
commit c7e56d4
Show file tree

Hide file tree

Showing 17 changed files with 485 additions and 107 deletions.
diff --git a/enigma-swing/build.gradle b/enigma-swing/build.gradle
@@ -48,7 +48,7 @@ def registerTestTask(String name) {
 
 		def jar = project(":enigma").file("build/test-obf/${name}.jar")
 		def mappings = file("mappings/${name}")
-		args('-jar', jar, '-mappings', mappings)
+		args('-jar', jar, '-mappings', mappings, '--development')
 		doFirst {
 			mappings.mkdirs()
 		}

diff --git a/enigma-swing/src/main/java/org/quiltmc/enigma/gui/Gui.java b/enigma-swing/src/main/java/org/quiltmc/enigma/gui/Gui.java
@@ -3,6 +3,7 @@
 import org.quiltmc.enigma.api.Enigma;
 import org.quiltmc.enigma.api.EnigmaProfile;
 import org.quiltmc.enigma.api.analysis.EntryReference;
+import org.quiltmc.enigma.api.source.TokenType;
 import org.quiltmc.enigma.api.translation.mapping.EntryMapping;
 import org.quiltmc.enigma.api.translation.mapping.EntryRemapper;
 import org.quiltmc.enigma.gui.config.Config;
@@ -457,20 +458,24 @@ public void toggleMapping(EditorPanel editor) {
 	public void toggleMappingFromEntry(Entry<?> obfEntry) {
 		EntryMapping mapping = this.controller.getProject().getRemapper().getMapping(obfEntry);
 
+		EntryChange<?> change = EntryChange.modify(obfEntry);
 		if (mapping.targetName() != null) {
-			EntryRemapper remapper = this.controller.getProject().getRemapper();
-			EntryChange<? extends Entry<?>> change;
-			EntryMapping proposedMapping = remapper.getProposedMappings().get(obfEntry);
-			if (proposedMapping != null) {
-				change = EntryChange.modify(obfEntry).withDeobfName(proposedMapping.targetName()).withTokenType(proposedMapping.tokenType()).withSourcePluginId(proposedMapping.sourcePluginId());
+			if (mapping.tokenType().isProposed()) {
+				change = change.withTokenType(TokenType.DEOBFUSCATED).clearSourcePluginId();
 			} else {
-				change = EntryChange.modify(obfEntry).clearDeobfName();
+				EntryRemapper remapper = this.controller.getProject().getRemapper();
+				EntryMapping proposedMapping = remapper.getProposedMappings().get(obfEntry);
+				if (proposedMapping != null) {
+					change = change.withDeobfName(proposedMapping.targetName()).withTokenType(proposedMapping.tokenType()).withSourcePluginId(proposedMapping.sourcePluginId());
+				} else {
+					change = change.clearDeobfName();
+				}
 			}
-
-			this.controller.applyChange(new ValidationContext(this.getNotificationManager()), change);
 		} else {
-			this.controller.applyChange(new ValidationContext(this.getNotificationManager()), EntryChange.modify(obfEntry).withDeobfName(obfEntry.getName()));
+			change = change.withDeobfName(obfEntry.getName());
 		}
+
+		this.controller.applyChange(new ValidationContext(this.getNotificationManager()), change);
 	}
 
 	public void showDiscardDiag(IntFunction<Void> callback, String... options) {

diff --git a/enigma-swing/src/main/java/org/quiltmc/enigma/gui/GuiController.java b/enigma-swing/src/main/java/org/quiltmc/enigma/gui/GuiController.java
@@ -535,7 +535,7 @@ private void applyChange0(ValidationContext vc, EntryChange<?> change, boolean u
 				this.gui.getActiveEditor().onRename(target);
 			}
 
-			if (!Objects.equals(prev.targetName(), mapping.targetName())) {
+			if (!Objects.equals(prev.targetName(), mapping.targetName()) || !Objects.equals(prev.tokenType(), mapping.tokenType())) {
 				this.chp.invalidateMapped();
 			}
 

diff --git a/enigma-swing/src/main/java/org/quiltmc/enigma/gui/Main.java b/enigma-swing/src/main/java/org/quiltmc/enigma/gui/Main.java
@@ -53,7 +53,7 @@ public static void main(String[] args) throws IOException {
 		parser.acceptsAll(List.of("edit-javadocs", "d"), "Enable editing Javadocs");
 		parser.acceptsAll(List.of("no-edit-javadocs", "D"), "Disable editing Javadocs");
 
-		parser.accepts("single-class-tree", "Unify the deobfuscated and obfuscated class panels");
+		parser.accepts("development", "Enable extra options and information for development");
 
 		parser.accepts("help", "Displays help information");
 
@@ -65,6 +65,10 @@ public static void main(String[] args) throws IOException {
 				return;
 			}
 
+			if (options.has("development")) {
+				System.setProperty("enigma.development", "true");
+			}
+
 			Set<EditableType> editables = EnumSet.allOf(EditableType.class);
 
 			for (OptionSpec<?> spec : options.specs()) {

diff --git a/enigma-swing/src/main/java/org/quiltmc/enigma/gui/element/MenuBar.java b/enigma-swing/src/main/java/org/quiltmc/enigma/gui/element/MenuBar.java
@@ -1,5 +1,6 @@
 package org.quiltmc.enigma.gui.element;
 
+import org.quiltmc.enigma.api.translation.mapping.serde.MappingFormat;
 import org.quiltmc.enigma.gui.ConnectionState;
 import org.quiltmc.enigma.gui.Gui;
 import org.quiltmc.enigma.gui.NotificationManager;
@@ -20,22 +21,31 @@
 import org.quiltmc.enigma.gui.util.GuiUtil;
 import org.quiltmc.enigma.gui.util.LanguageUtil;
 import org.quiltmc.enigma.gui.util.ScaleUtil;
-import org.quiltmc.enigma.api.translation.mapping.serde.MappingFormat;
+import org.quiltmc.enigma.util.EntryTreePrinter;
 import org.quiltmc.enigma.util.I18n;
 import org.quiltmc.enigma.util.Pair;
 import org.quiltmc.enigma.util.validation.Message;
 import org.quiltmc.enigma.util.validation.ParameterizedMessage;
 
 import javax.annotation.Nullable;
 import javax.swing.ButtonGroup;
+import javax.swing.JButton;
 import javax.swing.JFileChooser;
+import javax.swing.JFrame;
 import javax.swing.JMenu;
 import javax.swing.JMenuBar;
 import javax.swing.JMenuItem;
 import javax.swing.JOptionPane;
 import javax.swing.JRadioButtonMenuItem;
+import javax.swing.JScrollPane;
+import javax.swing.JTextArea;
+import javax.swing.WindowConstants;
+import java.awt.BorderLayout;
+import java.awt.Font;
 import java.io.File;
 import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
@@ -91,6 +101,10 @@ public class MenuBar {
 	private final JMenuItem aboutItem = new JMenuItem();
 	private final JMenuItem githubItem = new JMenuItem();
 
+	// Enabled with system property "enigma.development" or "--development" flag
+	private final JMenu devMenu = new JMenu();
+	private final JMenuItem printMappingTreeItem = new JMenuItem();
+
 	private final Gui gui;
 
 	public MenuBar(Gui gui) {
@@ -160,6 +174,11 @@ public MenuBar(Gui gui) {
 		this.helpMenu.add(this.githubItem);
 		ui.add(this.helpMenu);
 
+		this.devMenu.add(this.printMappingTreeItem);
+		if (System.getProperty("enigma.development", "false").equalsIgnoreCase("true")) {
+			ui.add(this.devMenu);
+		}
+
 		this.setKeyBinds();
 
 		this.jarOpenItem.addActionListener(e -> this.onOpenJarClicked());
@@ -188,6 +207,7 @@ public MenuBar(Gui gui) {
 		this.startServerItem.addActionListener(e -> this.onStartServerClicked());
 		this.aboutItem.addActionListener(e -> AboutDialog.show(this.gui.getFrame()));
 		this.githubItem.addActionListener(e -> this.onGithubClicked());
+		this.printMappingTreeItem.addActionListener(e -> this.onPrintMappingTreeClicked());
 	}
 
 	public void setKeyBinds() {
@@ -222,6 +242,7 @@ public void updateUiState() {
 		this.exportSourceItem.setEnabled(jarOpen);
 		this.exportJarItem.setEnabled(jarOpen);
 		this.statsItem.setEnabled(jarOpen);
+		this.printMappingTreeItem.setEnabled(jarOpen);
 	}
 
 	public void retranslateUi() {
@@ -269,6 +290,9 @@ public void retranslateUi() {
 		this.helpMenu.setText(I18n.translate("menu.help"));
 		this.aboutItem.setText(I18n.translate("menu.help.about"));
 		this.githubItem.setText(I18n.translate("menu.help.github"));
+
+		this.devMenu.setText("Dev");
+		this.printMappingTreeItem.setText("Print mapping tree");
 	}
 
 	private void onOpenJarClicked() {
@@ -455,6 +479,29 @@ private void onGithubClicked() {
 		GuiUtil.openUrl("https://github.com/QuiltMC/Enigma");
 	}
 
+	private void onPrintMappingTreeClicked() {
+		var mappings = this.gui.getController().getProject().getRemapper().getMappings();
+
+		StringWriter text = new StringWriter();
+		EntryTreePrinter.print(new PrintWriter(text), mappings);
+
+		var frame = new JFrame("Mapping Tree");
+		var pane = frame.getContentPane();
+		pane.setLayout(new BorderLayout());
+
+		var textArea = new JTextArea(text.toString());
+		textArea.setFont(ScaleUtil.getFont(Font.MONOSPACED, Font.PLAIN, 12));
+		pane.add(new JScrollPane(textArea), BorderLayout.CENTER);
+		var button = new JButton("Close");
+		button.addActionListener(e -> frame.dispose());
+		pane.add(button, BorderLayout.SOUTH);
+
+		frame.setSize(ScaleUtil.getDimension(1200, 400));
+		frame.setLocationRelativeTo(this.gui.getFrame());
+		frame.setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);
+		frame.setVisible(true);
+	}
+
 	private void onOpenMappingsClicked() {
 		this.gui.enigmaMappingsFileChooser.setCurrentDirectory(new File(Config.main().stats.lastSelectedDir.value()));
 		if (this.gui.enigmaMappingsFileChooser.showOpenDialog(this.gui.getFrame()) == JFileChooser.APPROVE_OPTION) {

diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java b/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java
@@ -164,7 +164,7 @@ public EntryRemapper getRemapper() {
 	}
 
 	public void dropMappings(ProgressListener progress) {
-		DeltaTrackingTree<EntryMapping> mappings = this.remapper.getDeobfMappings();
+		DeltaTrackingTree<EntryMapping> mappings = this.remapper.getMappings();
 
 		Collection<Entry<?>> dropped = this.dropMappings(mappings, progress);
 		for (Entry<?> entry : dropped) {

diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/analysis/index/jar/EntryIndex.java b/enigma/src/main/java/org/quiltmc/enigma/api/analysis/index/jar/EntryIndex.java
@@ -77,7 +77,13 @@ public boolean hasEntry(Entry<?> entry) {
 		} else if (entry instanceof FieldEntry fieldEntry) {
 			return this.hasField(fieldEntry);
 		} else if (entry instanceof LocalVariableEntry localVariableEntry) {
-			return this.hasMethod(localVariableEntry.getParent());
+			MethodEntry parent = localVariableEntry.getParent();
+			if (this.hasMethod(parent)) {
+				// TODO: Check using max_locals from the Code attribute (JVMS§4.7.3)
+				AccessFlags parentAccess = this.getMethodAccess(parent);
+				int startIndex = parentAccess != null && parentAccess.isStatic() ? 0 : 1;
+				return localVariableEntry.getIndex() >= startIndex;
+			}
 		}
 
 		return false;

diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/EntryRemapper.java b/enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/EntryRemapper.java
@@ -10,8 +10,8 @@
 import org.quiltmc.enigma.api.translation.Translator;
 import org.quiltmc.enigma.api.translation.mapping.tree.DeltaTrackingTree;
 import org.quiltmc.enigma.api.translation.mapping.tree.EntryTree;
-import org.quiltmc.enigma.api.translation.mapping.tree.EntryTreeUtil;
 import org.quiltmc.enigma.api.translation.mapping.tree.HashEntryTree;
+import org.quiltmc.enigma.api.translation.mapping.tree.MergedEntryMappingTree;
 import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry;
 import org.quiltmc.enigma.api.translation.representation.entry.Entry;
 import org.quiltmc.enigma.api.translation.representation.entry.FieldEntry;
@@ -27,9 +27,9 @@
 import javax.annotation.Nullable;
 
 public class EntryRemapper {
-	private final DeltaTrackingTree<EntryMapping> deobfNames;
-	private final DeltaTrackingTree<EntryMapping> proposedNames;
-	private final DeltaTrackingTree<EntryMapping> mergedNames;
+	private final EntryTree<EntryMapping> deobfMappings;
+	private final EntryTree<EntryMapping> proposedMappings;
+	private final DeltaTrackingTree<EntryMapping> mappings;
 
 	private final EntryResolver obfResolver;
 	private final Translator deobfuscator;
@@ -40,13 +40,13 @@ public class EntryRemapper {
 	private final List<NameProposalService> proposalServices;
 
 	private EntryRemapper(JarIndex jarIndex, MappingsIndex mappingsIndex, EntryTree<EntryMapping> proposedMappings, EntryTree<EntryMapping> deobfMappings, List<NameProposalService> proposalServices) {
-		this.deobfNames = new DeltaTrackingTree<>(deobfMappings);
-		this.proposedNames = new DeltaTrackingTree<>(proposedMappings);
-		this.mergedNames = new DeltaTrackingTree<>(EntryTreeUtil.merge(proposedMappings, deobfMappings));
+		this.deobfMappings = deobfMappings;
+		this.proposedMappings = proposedMappings;
+		this.mappings = new DeltaTrackingTree<>(new MergedEntryMappingTree(deobfMappings, proposedMappings));
 
 		this.obfResolver = jarIndex.getEntryResolver();
 
-		this.deobfuscator = new MappingTranslator(this.mergedNames, this.obfResolver);
+		this.deobfuscator = new MappingTranslator(this.mappings, this.obfResolver);
 		this.jarIndex = jarIndex;
 		this.mappingsIndex = mappingsIndex;
 
@@ -93,9 +93,9 @@ private void doPutMapping(ValidationContext vc, Entry<?> obfuscatedEntry, @Nonnu
 
 		for (Entry<?> resolvedEntry : resolvedEntries) {
 			if (deobfMapping.equals(EntryMapping.DEFAULT)) {
-				this.insertName(resolvedEntry, null);
+				this.mappings.insert(resolvedEntry, null);
 			} else {
-				this.insertName(resolvedEntry, deobfMapping);
+				this.mappings.insert(resolvedEntry, deobfMapping);
 			}
 		}
 
@@ -137,17 +137,6 @@ private void mapRecordComponentGetter(ValidationContext vc, ClassEntry classEntr
 		this.doPutMapping(vc, methodEntry, new EntryMapping(fieldMapping.targetName()), false);
 	}
 
-	private void insertName(Entry<?> entry, @Nullable EntryMapping mapping) {
-		this.mergedNames.insert(entry, mapping);
-		if (mapping != null) {
-			if (mapping.tokenType().isProposed()) {
-				this.proposedNames.insert(entry, mapping);
-			} else {
-				this.deobfNames.insert(entry, mapping);
-			}
-		}
-	}
-
 	/**
 	 * Runs {@link NameProposalService#getDynamicProposedNames(EntryRemapper, Entry, EntryMapping, EntryMapping)} over the names stored in this remapper,
 	 * inserting all mappings generated.
@@ -156,14 +145,14 @@ public void insertDynamicallyProposedMappings(@Nullable Entry<?> obfEntry, @Null
 		for (var service : this.proposalServices) {
 			var proposedNames = service.getDynamicProposedNames(this, obfEntry, oldMapping, newMapping);
 			if (proposedNames != null) {
-				proposedNames.forEach(this::insertName);
+				proposedNames.forEach(this.proposedMappings::insert);
 			}
 		}
 	}
 
 	@Nonnull
 	public EntryMapping getMapping(Entry<?> entry) {
-		EntryMapping entryMapping = this.mergedNames.get(entry);
+		EntryMapping entryMapping = this.mappings.get(entry);
 		return entryMapping == null ? EntryMapping.DEFAULT : entryMapping;
 	}
 
@@ -180,43 +169,43 @@ public Translator getDeobfuscator() {
 	}
 
 	public Stream<Entry<?>> getObfEntries() {
-		return this.mergedNames.getAllEntries();
+		return this.mappings.getAllEntries();
 	}
 
 	public Collection<Entry<?>> getObfChildren(Entry<?> obfuscatedEntry) {
-		return this.mergedNames.getChildren(obfuscatedEntry);
+		return this.mappings.getChildren(obfuscatedEntry);
 	}
 
 	/**
 	 * Gets all mappings, including both manually inserted and proposed names.
 	 * @return the merged mapping tree
 	 */
 	public DeltaTrackingTree<EntryMapping> getMappings() {
-		return this.mergedNames;
+		return this.mappings;
 	}
 
 	/**
 	 * Gets all manually inserted mappings.
 	 * @return the deobfuscated mapping tree
 	 */
-	public DeltaTrackingTree<EntryMapping> getDeobfMappings() {
-		return this.deobfNames;
+	public EntryTree<EntryMapping> getDeobfMappings() {
+		return this.deobfMappings;
 	}
 
 	/**
 	 * Gets all proposed mappings.
 	 * @return the proposed mapping tree
 	 */
-	public DeltaTrackingTree<EntryMapping> getProposedMappings() {
-		return this.proposedNames;
+	public EntryTree<EntryMapping> getProposedMappings() {
+		return this.proposedMappings;
 	}
 
 	public MappingDelta<EntryMapping> takeMappingDelta() {
-		return this.mergedNames.takeDelta();
+		return this.mappings.takeDelta();
 	}
 
 	public boolean isDirty() {
-		return this.mergedNames.isDirty();
+		return this.mappings.isDirty();
 	}
 
 	public EntryResolver getObfResolver() {

diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/IndexEntryResolver.java b/enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/IndexEntryResolver.java
@@ -64,6 +64,12 @@ public <E extends Entry<?>> Collection<E> resolveEntry(E entry, ResolutionStrate
 		return Collections.singleton(entry);
 	}
 
+	/**
+	 * Get a direct child of any class that is an ancestor of the given entry.
+	 *
+	 * @param entry the descendant of a class
+	 * @return the direct child of a class, which is an ancestor of the given entry or the entry itself
+	 */
 	@Nullable
 	private Entry<ClassEntry> getClassChild(Entry<?> entry) {
 		if (entry instanceof ClassEntry) {
@@ -87,6 +93,7 @@ private Entry<ClassEntry> getClassChild(Entry<?> entry) {
 	private Set<Entry<ClassEntry>> resolveChildEntry(Entry<ClassEntry> entry, ResolutionStrategy strategy) {
 		ClassEntry ownerClass = entry.getParent();
 
+		// Resolve specialized methods using their bridges
 		if (entry instanceof MethodEntry methodEntry) {
 			MethodEntry bridgeMethod = this.bridgeMethodIndex.getBridgeFromSpecialized(methodEntry);
 			if (bridgeMethod != null && ownerClass.equals(bridgeMethod.getParent())) {

diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/tree/DeltaTrackingTree.java b/enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/tree/DeltaTrackingTree.java
@@ -107,4 +107,9 @@ private void resetDelta() {
 	public boolean isDirty() {
 		return !this.changes.isEmpty();
 	}
+
+	@Override
+	public String toString() {
+		return "DeltaTrackingTree[" + this.delegate + "]";
+	}
 }