Unpick v3 fixes (#1347)

gradle/libs.versions.toml

@@ -12,7 +12,7 @@ mapping-io = "0.7.1"
 lorenz-tiny = "4.0.2"
 mercury = "0.4.2"
 loom-native = "0.2.0"
-unpick = "3.0.0-beta.5"
+unpick = "3.0.0-beta.9"
 
 # Plugins
 spotless = "6.25.0"

src/main/java/net/fabricmc/loom/task/service/UnpickRemapperService.java

@@ -29,6 +29,7 @@ import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
 import java.io.Reader;
+import java.lang.reflect.Field;
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.List;
@@ -41,6 +42,7 @@ import org.gradle.api.Project;
 import org.gradle.api.provider.Property;
 import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.Nested;
+import org.objectweb.asm.Type;
 import org.objectweb.asm.commons.Remapper;
 
 import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
@@ -110,17 +112,17 @@ public class UnpickRemapperService extends Service<UnpickRemapperService.Options
 
 		@Override
 		protected String mapClassName(String className) {
-			return remapper.map(className);
+			return remapper.map(className.replace('.', '/')).replace('/', '.');
 		}
 
 		@Override
 		protected String mapFieldName(String className, String fieldName, String fieldDesc) {
-			return remapper.mapFieldName(className, fieldName, fieldDesc);
+			return remapper.mapFieldName(className.replace('.', '/'), fieldName, fieldDesc);
 		}
 
 		@Override
 		protected String mapMethodName(String className, String methodName, String methodDesc) {
-			return remapper.mapMethodName(className, methodName, methodDesc);
+			return remapper.mapMethodName(className.replace('.', '/'), methodName, methodDesc);
 		}
 
 		// Return all classes in the given package, not recursively.
@@ -134,19 +136,35 @@ public class UnpickRemapperService extends Service<UnpickRemapperService.Options
 
 		@Override
 		protected String getFieldDesc(String className, String fieldName) {
-			TrClass trClass = tinyRemapper.getEnvironment().getClass(className);
+			TrClass trClass = tinyRemapper.getEnvironment().getClass(className.replace('.', '/'));
 
-			if (trClass == null) {
-				return null;
-			}
-
-			for (TrField trField : trClass.getFields()) {
-				if (trField.getName().equals(fieldName)) {
-					return trField.getDesc();
+			if (trClass != null) {
+				for (TrField trField : trClass.getFields()) {
+					if (trField.getName().equals(fieldName)) {
+						return trField.getDesc();
+					}
 				}
 			}
 
-			return null;
+			String fieldDesc = getFieldDescFromReflection(className, fieldName);
+
+			if (fieldDesc == null) {
+				throw new IllegalStateException("Could not find field " + fieldName + " in class " + className);
+			}
+
+			return fieldDesc;
+		}
+
+		private static String getFieldDescFromReflection(String className, String fieldName) {
+			try {
+				// Use the bootstrap class loader, which should only resolve classes from the JDK.
+				// Don't run the static initializer.
+				Class<?> clazz = Class.forName(className, false, null);
+				Field field = clazz.getDeclaredField(fieldName);
+				return Type.getDescriptor(field.getType());
+			} catch (ClassNotFoundException | NoSuchFieldException e) {
+				return null;
+			}
 		}
 	}
 }

src/test/groovy/net/fabricmc/loom/test/unit/service/UnpickRemapperServiceTest.groovy

@@ -36,7 +36,10 @@ import net.fabricmc.loom.test.unit.service.mocks.MockTinyRemapper
 import net.fabricmc.loom.test.unit.service.mocks.MockTinyRemapperService
 import net.fabricmc.loom.util.Pair
 import net.fabricmc.loom.util.ZipUtils
+import net.fabricmc.tinyremapper.api.TrClass
+import net.fabricmc.tinyremapper.api.TrField
 
+import static org.mockito.Mockito.mock
 import static org.mockito.Mockito.when
 
 // Based on https://github.com/Earthcomputer/unpick-v3-parser/blob/68b11c50a7c97a75218f70f5ec1291a38b178ad7/src/test/java/net/earthcomputer/unpickv3parser/remapper/TestRemapper.java
@@ -50,17 +53,17 @@ class UnpickRemapperServiceTest extends ServiceTestBase {
 	]
 
 	private static final Map<String, String> CLASSES = [
-		"unmapped.foo.A": "mapped.foo.X",
-		"unmapped.foo.B": "mapped.bar.Y",
-		"unmapped.bar.C": "mapped.bar.Z"
+		"unmapped/foo/A": "mapped/foo/X",
+		"unmapped/foo/B": "mapped/bar/Y",
+		"unmapped/bar/C": "mapped/bar/Z"
 	]
 
 	private static final Map<MemberKey, String> FIELDS = [
-		(new MemberKey("unmapped.foo.B", "baz", "I")): "quux"
+		(new MemberKey("unmapped/foo/B", "baz", "I")): "quux"
 	]
 
 	private static final Map<MemberKey, String> METHODS = [
-		(new MemberKey("unmapped.foo.B", "foo2", "(Lunmapped/foo/A;)V")): "bar2"
+		(new MemberKey("unmapped/foo/B", "foo2", "(Lunmapped/foo/A;)V")): "bar2"
 	]
 
 	@TempDir
@@ -94,6 +97,18 @@ class UnpickRemapperServiceTest extends ServiceTestBase {
 			when(mockTr.remapper.mapFieldName(key.owner, key.name, key.descriptor)).thenReturn(mapped)
 		}
 
+		FIELDS.groupBy { it.key.owner }.each { owner, fields ->
+			def mockClass = mock(TrClass.class)
+			when(mockTr.trEnvironment.getClass(owner)).thenReturn(mockClass)
+			def mockFields = fields.collect { key, mapped ->
+				def mockField = mock(TrField.class)
+				when(mockField.name).thenReturn(key.name)
+				when(mockField.desc).thenReturn(key.descriptor)
+				mockField
+			}
+			when(mockClass.fields).thenReturn(mockFields)
+		}
+
 		METHODS.each { key, mapped ->
 			when(mockTr.remapper.mapMethodName(key.owner, key.name, key.descriptor)).thenReturn(mapped)
 		}

src/test/resources/unpick/remapped.unpick

@@ -7,7 +7,7 @@ target_field mapped.bar.Z null Lmapped/foo/X; g
 target_method mapped.bar.Y bar2 (Lmapped/foo/X;)V
 
 group int
-	mapped.bar.Y.*
+	mapped.bar.Y.quux
 
 group float
 	mapped.bar.Y.quux:int