Prevent UI from exporting aggregate mappings to unsupported types whe…

…n descriptors are missing

recaf-core/src/main/java/software/coley/recaf/services/mapping/IntermediateMappings.java

@@ -34,7 +34,7 @@ public class IntermediateMappings implements Mappings {
 	 * @param newName
 	 * 		Post-mapping name.
 	 */
-	public void addClass(String oldName, String newName) {
+	public void addClass(@Nonnull String oldName, @Nonnull String newName) {
 		if (Objects.equals(oldName, newName)) return; // Skip identity mappings
 		classes.put(oldName, new ClassMapping(oldName, newName));
 	}
@@ -49,7 +49,7 @@ public void addClass(String oldName, String newName) {
 	 * @param newName
 	 * 		Post-mapping field name.
 	 */
-	public void addField(String ownerName, String desc, String oldName, String newName) {
+	public void addField(@Nonnull String ownerName, @Nullable String desc, @Nonnull String oldName, @Nonnull String newName) {
 		if (Objects.equals(oldName, newName)) return; // Skip identity mappings
 		fields.computeIfAbsent(ownerName, n -> new ArrayList<>())
 				.add(new FieldMapping(ownerName, oldName, desc, newName));
@@ -65,7 +65,7 @@ public void addField(String ownerName, String desc, String oldName, String newNa
 	 * @param newName
 	 * 		Post-mapping method name.
 	 */
-	public void addMethod(String ownerName, String desc, String oldName, String newName) {
+	public void addMethod(@Nonnull String ownerName, @Nonnull String desc, @Nonnull String oldName, @Nonnull String newName) {
 		if (Objects.equals(oldName, newName)) return; // Skip identity mappings
 		methods.computeIfAbsent(ownerName, n -> new ArrayList<>())
 				.add(new MethodMapping(ownerName, oldName, desc, newName));
@@ -87,9 +87,9 @@ public void addMethod(String ownerName, String desc, String oldName, String newN
 	 * @param newName
 	 * 		Post-mapping method name.
 	 */
-	public void addVariable(String ownerName, String methodName, String methodDesc,
-	                        String desc, String oldName, int index,
-	                        String newName) {
+	public void addVariable(@Nonnull String ownerName, @Nonnull String methodName, @Nonnull String methodDesc,
+	                        @Nullable String desc, @Nullable String oldName, int index,
+	                        @Nonnull String newName) {
 		if (Objects.equals(oldName, newName)) return; // Skip identity mappings
 		String key = varKey(ownerName, methodName, methodDesc);
 		variables.computeIfAbsent(key, n -> new ArrayList<>())
@@ -147,7 +147,7 @@ public Map<String, List<VariableMapping>> getVariables() {
 	 * @return Mapping instance of class. May be {@code null}.
 	 */
 	@Nullable
-	public ClassMapping getClassMapping(String name) {
+	public ClassMapping getClassMapping(@Nonnull String name) {
 		return classes.get(name);
 	}
 
@@ -158,7 +158,7 @@ public ClassMapping getClassMapping(String name) {
 	 * @return List of field mapping instances.
 	 */
 	@Nonnull
-	public List<FieldMapping> getClassFieldMappings(String name) {
+	public List<FieldMapping> getClassFieldMappings(@Nonnull String name) {
 		return fields.getOrDefault(name, Collections.emptyList());
 	}
 
@@ -169,7 +169,7 @@ public List<FieldMapping> getClassFieldMappings(String name) {
 	 * @return List of method mapping instances.
 	 */
 	@Nonnull
-	public List<MethodMapping> getClassMethodMappings(String name) {
+	public List<MethodMapping> getClassMethodMappings(@Nonnull String name) {
 		return methods.getOrDefault(name, Collections.emptyList());
 	}
 
@@ -184,7 +184,7 @@ public List<MethodMapping> getClassMethodMappings(String name) {
 	 * @return List of field mapping instances.
 	 */
 	@Nonnull
-	public List<VariableMapping> getMethodVariableMappings(String ownerName, String methodName, String methodDesc) {
+	public List<VariableMapping> getMethodVariableMappings(@Nonnull String ownerName, @Nonnull String methodName, @Nonnull String methodDesc) {
 		return variables.getOrDefault(varKey(ownerName, methodName, methodDesc), Collections.emptyList());
 	}
 

recaf-core/src/main/java/software/coley/recaf/services/mapping/aggregate/AggregateMappingManager.java

@@ -1,15 +1,17 @@
 package software.coley.recaf.services.mapping.aggregate;
 
 import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 import org.slf4j.Logger;
 import software.coley.collections.Unchecked;
 import software.coley.recaf.analytics.logging.Logging;
-import software.coley.recaf.cdi.AutoRegisterWorkspaceListeners;
-import software.coley.recaf.cdi.WorkspaceScoped;
 import software.coley.recaf.services.Service;
 import software.coley.recaf.services.mapping.Mappings;
 import software.coley.recaf.services.workspace.WorkspaceCloseListener;
+import software.coley.recaf.services.workspace.WorkspaceManager;
+import software.coley.recaf.services.workspace.WorkspaceOpenListener;
 import software.coley.recaf.workspace.model.Workspace;
 
 import java.util.List;
@@ -21,22 +23,25 @@
  * @author Matt Coley
  * @author Marius Renner
  */
-@WorkspaceScoped
-@AutoRegisterWorkspaceListeners
+@ApplicationScoped
 public class AggregateMappingManager implements Service, WorkspaceCloseListener {
 	public static final String SERVICE_ID = "mapping-aggregator";
 	private static final Logger logger = Logging.get(AggregateMappingManager.class);
 	private final List<AggregatedMappingsListener> aggregateListeners = new CopyOnWriteArrayList<>();
-	private final AggregatedMappings aggregatedMappings;
 	private final AggregateMappingManagerConfig config;
+	private AggregatedMappings aggregatedMappings;
 
 	@Inject
 	public AggregateMappingManager(@Nonnull AggregateMappingManagerConfig config,
-	                               @Nonnull Workspace workspace) {
+	                               @Nonnull WorkspaceManager workspaceManager) {
 		this.config = config;
-		aggregatedMappings = new AggregatedMappings(workspace);
+
+		ListenerHost host = new ListenerHost();
+		workspaceManager.addWorkspaceOpenListener(host);
+		workspaceManager.addWorkspaceCloseListener(host);
 	}
 
+
 	@Override
 	public void onWorkspaceClosed(@Nonnull Workspace workspace) {
 		aggregateListeners.clear();
@@ -50,6 +55,8 @@ public void onWorkspaceClosed(@Nonnull Workspace workspace) {
 	 * 		The additional mappings that were added.
 	 */
 	public void updateAggregateMappings(@Nonnull Mappings newMappings) {
+		if (aggregatedMappings == null)
+			return;
 		aggregatedMappings.update(newMappings);
 		Unchecked.checkedForEach(aggregateListeners, listener -> listener.onAggregatedMappingsUpdated(getAggregatedMappings()),
 				(listener, t) -> logger.error("Exception thrown when updating aggregate mappings", t));
@@ -59,6 +66,8 @@ public void updateAggregateMappings(@Nonnull Mappings newMappings) {
 	 * Clears all mapping information.
 	 */
 	private void clearAggregated() {
+		if (aggregatedMappings == null)
+			return;
 		aggregatedMappings.clear();
 		Unchecked.checkedForEach(aggregateListeners, listener -> listener.onAggregatedMappingsUpdated(getAggregatedMappings()),
 				(listener, t) -> logger.error("Exception thrown when updating aggregate mappings", t));
@@ -84,9 +93,9 @@ public boolean removeAggregatedMappingListener(@Nonnull AggregatedMappingsListen
 	}
 
 	/**
-	 * @return Current aggregated mappings in the ASM format.
+	 * @return Current aggregated mappings in the ASM format. Will be {@code null} if no workspace is open.
 	 */
-	@Nonnull
+	@Nullable
 	public AggregatedMappings getAggregatedMappings() {
 		return aggregatedMappings;
 	}
@@ -102,4 +111,16 @@ public String getServiceId() {
 	public AggregateMappingManagerConfig getServiceConfig() {
 		return config;
 	}
+
+	private class ListenerHost implements WorkspaceOpenListener, WorkspaceCloseListener {
+		@Override
+		public void onWorkspaceOpened(@Nonnull Workspace workspace) {
+			aggregatedMappings = new AggregatedMappings(workspace);
+		}
+
+		@Override
+		public void onWorkspaceClosed(@Nonnull Workspace workspace) {
+			aggregatedMappings = null;
+		}
+	}
 }

recaf-core/src/main/java/software/coley/recaf/services/mapping/aggregate/AggregatedMappings.java

@@ -35,6 +35,7 @@
 public class AggregatedMappings extends IntermediateMappings {
 	private final Map<String, String> reverseOrderClassMapping = new HashMap<>();
 	private final WorkspaceBackedRemapper reverseMapper;
+	private boolean missingFieldDescriptors;
 
 	/**
 	 * @param workspace
@@ -199,10 +200,22 @@ public void addClass(@Nonnull String oldName, @Nonnull String newName) {
 		reverseOrderClassMapping.put(newName, oldName);
 	}
 
+	/**
+	 * Some mapping formats do not include the descriptor of fields since they assume all fields are uniquely identified by name.
+	 * This kinda sucks because unless we do a lot of additional lookup work <i>(Which may not even be successful)</i>,
+	 * we're missing out on data. If this is ever the case formats that do require this data cannot be exported to.
+	 *
+	 * @return {@code true} when there are field entries in these mapping which do not have descriptors associated with them.
+	 */
+	public boolean isMissingFieldDescriptors() {
+		return missingFieldDescriptors;
+	}
+
 	/**
 	 * Clears the mapping entries.
 	 */
 	public void clear() {
+		missingFieldDescriptors = false;
 		classes.clear();
 		fields.clear();
 		methods.clear();
@@ -237,7 +250,7 @@ public boolean update(@Nonnull Mappings newMappings) {
 		return bridged;
 	}
 
-	private boolean updateClasses(Map<String, ClassMapping> classes) {
+	private boolean updateClasses(@Nonnull Map<String, ClassMapping> classes) {
 		boolean bridged = false;
 		for (ClassMapping newMapping : classes.values()) {
 			String cName = newMapping.getNewName();
@@ -256,7 +269,7 @@ private boolean updateClasses(Map<String, ClassMapping> classes) {
 		return bridged;
 	}
 
-	private <M extends MemberMapping> boolean updateMembers(Collection<List<M>> newMappings) {
+	private <M extends MemberMapping> boolean updateMembers(@Nonnull Collection<List<M>> newMappings) {
 		// With members, we need to take special care, for example:
 		// 1. a --> b
 		// 2. b.x --> b.y
@@ -283,16 +296,17 @@ private <M extends MemberMapping> boolean updateMembers(Collection<List<M>> newM
 
 				// Add bridged entry
 				if (newMemberMapping.isField()) {
+					missingFieldDescriptors |= desc == null;
 					addField(owner, desc, oldMemberName, newMemberName);
-				} else {
+				} else if (desc != null) {
 					addMethod(owner, desc, oldMemberName, newMemberName);
 				}
 			}
 		}
 		return bridged;
 	}
 
-	private boolean updateVariables(Collection<List<VariableMapping>> newMappings) {
+	private boolean updateVariables(@Nonnull Collection<List<VariableMapping>> newMappings) {
 		// a.foo() var x
 		//   ...
 		// a.foo() --> b.foo()
@@ -315,7 +329,8 @@ private boolean updateVariables(Collection<List<VariableMapping>> newMappings) {
 		return bridged;
 	}
 
-	private String applyReverseMappings(String desc) {
+	@Nullable
+	private String applyReverseMappings(@Nullable String desc) {
 		if (desc == null)
 			return null;
 		else if (desc.charAt(0) == '(')
@@ -324,15 +339,17 @@ else if (desc.charAt(0) == '(')
 			return reverseMapper.mapDesc(desc);
 	}
 
-	private String findPriorMemberName(String oldClassName, MemberMapping memberMapping) {
+	@Nonnull
+	private String findPriorMemberName(@Nonnull String oldClassName, @Nonnull MemberMapping memberMapping) {
 		if (memberMapping.isField()) {
 			return findPriorName(memberMapping, getClassFieldMappings(oldClassName));
 		} else {
 			return findPriorName(memberMapping, getClassMethodMappings(oldClassName));
 		}
 	}
 
-	private String findPriorName(MemberMapping newMethodMapping, List<? extends MemberMapping> members) {
+	@Nonnull
+	private String findPriorName(@Nonnull MemberMapping newMethodMapping, @Nonnull List<? extends MemberMapping> members) {
 		// If the old name not previously mapped, then it's the same as what the new mapping has given.
 		// So the passed new mapping is a safe default.
 		MemberMapping target = newMethodMapping;

recaf-core/src/main/java/software/coley/recaf/services/mapping/format/SrgMappings.java

@@ -191,7 +191,7 @@ public boolean doesSupportVariableTypeDifferentiation() {
 
 		@Nullable
 		@Override
-		public ClassMapping getClassMapping(String name) {
+		public ClassMapping getClassMapping(@Nonnull String name) {
 			ClassMapping classMapping = super.getClassMapping(name);
 			if (classMapping == null && !packageMappings.isEmpty()) {
 				for (Pair<String, String> packageMapping : packageMappings) {

recaf-core/src/test/java/software/coley/recaf/services/mapping/MappingApplierTest.java

@@ -134,6 +134,7 @@ public boolean shouldMapMethod(@Nonnull ClassInfo owner, @Nonnull MethodMember m
 		// We will validate this is only done AFTER 'results.apply()' is run.
 		// For future tests we will skip this since if it works here, it works there.
 		AggregatedMappings aggregatedMappings = aggregateMappingManager.getAggregatedMappings();
+		assertNotNull(aggregatedMappings);
 		assertNull(aggregatedMappings.getMappedClassName(stringSupplierName),
 				"StringSupplier should not yet be tracked in aggregate");
 		results.apply();
@@ -177,6 +178,7 @@ public boolean shouldMapMethod(@Nonnull ClassInfo owner, @Nonnull MethodMember m
 		// Assert aggregate updated too.
 		results.apply();
 		AggregatedMappings aggregatedMappings = aggregateMappingManager.getAggregatedMappings();
+		assertNotNull(aggregatedMappings);
 		assertNotNull(aggregatedMappings.getMappedClassName(dummyEnumName),
 				"DummyEnum should be tracked in aggregate");
 
@@ -213,6 +215,7 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) {
 		// Assert aggregate updated too.
 		results.apply();
 		AggregatedMappings aggregatedMappings = aggregateMappingManager.getAggregatedMappings();
+		assertNotNull(aggregatedMappings);
 		assertNotNull(aggregatedMappings.getMappedClassName(annotationName),
 				"AnnotationImpl should be tracked in aggregate");
 
@@ -268,6 +271,7 @@ public boolean shouldMapMethod(@Nonnull ClassInfo owner, @Nonnull MethodMember m
 		// Assert aggregate updated too.
 		results.apply();
 		AggregatedMappings aggregatedMappings = aggregateMappingManager.getAggregatedMappings();
+		assertNotNull(aggregatedMappings);
 		assertNotNull(aggregatedMappings.getMappedClassName(overlapInterfaceAName),
 				"OverlapInterfaceA should be tracked in aggregate");
 		assertNotNull(aggregatedMappings.getMappedClassName(overlapInterfaceBName),

recaf-core/src/test/java/software/coley/recaf/services/mapping/aggregate/AggregateMappingManagerTest.java

@@ -8,6 +8,7 @@
 import software.coley.recaf.workspace.model.EmptyWorkspace;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 /**
  * Tests for {@link AggregateMappingManager}
@@ -37,24 +38,30 @@ void testRefactorIntFieldWithManager() {
 
 		// 'a' renamed to 'b'
 		mappings1.addClass("a", "b");
+
 		// 'b' renamed to 'c', so 'a' is now 'c'
 		// but also rename the field 'oldName' to 'newName'
 		mappings2.addClass("b", "c");
 		mappings2.addField("b", "I", "oldName", "newName");
+
 		// 'c' renamed to 'd'
 		// but also rename the field from before to 'brandNewName'
 		mappings3.addClass("c", "d");
 		mappings3.addField("c", "I", "newName", "brandNewName");
 
 		// Get aggregate instance from manager
 		AggregatedMappings aggregated = aggregateMappingManager.getAggregatedMappings();
+		assertNotNull(aggregated);
+
 		// Validate after first mapping pass
 		aggregateMappingManager.updateAggregateMappings(mappings1);
 		assertEquals("b", aggregated.getMappedClassName("a"));
+
 		// Validate after second mapping pass
 		aggregateMappingManager.updateAggregateMappings(mappings2);
 		assertEquals("c", aggregated.getMappedClassName("a"));
 		assertEquals("newName", aggregated.getMappedFieldName("a", "oldName", "I"));
+
 		// Validate after third mapping pass
 		aggregateMappingManager.updateAggregateMappings(mappings3);
 		assertEquals("d", aggregated.getMappedClassName("a"));
@@ -70,24 +77,30 @@ void testRefactorGetInstanceWithManager() {
 
 		// 'a' renamed to 'b'
 		mappings1.addClass("a", "b");
+
 		// 'b' renamed to 'c', so 'a' is now 'c'
 		// but also rename the method 'obf' to 'factory'
 		mappings2.addClass("b", "c");
 		mappings2.addMethod("b", "()Lb;", "obf", "factory");
+
 		// 'c' renamed to 'd'
 		// but also rename the method from before to 'getInstance'
 		mappings3.addClass("c", "d");
 		mappings3.addMethod("c", "()Lc;", "factory", "getInstance");
 
 		// Get aggregate instance from manager
 		AggregatedMappings aggregated = aggregateMappingManager.getAggregatedMappings();
+		assertNotNull(aggregated);
+
 		// Validate after first mapping pass
 		aggregateMappingManager.updateAggregateMappings(mappings1);
 		assertEquals("b", aggregated.getMappedClassName("a"));
+
 		// Validate after second mapping pass
 		aggregateMappingManager.updateAggregateMappings(mappings2);
 		assertEquals("c", aggregated.getMappedClassName("a"));
 		assertEquals("factory", aggregated.getMappedMethodName("a", "obf", "()La;"));
+
 		// Validate after third mapping pass
 		aggregateMappingManager.updateAggregateMappings(mappings3);
 		assertEquals("d", aggregated.getMappedClassName("a"));