Injected Interfaces Expansion => Generics Support

src/main/java/net/fabricmc/loom/configuration/ifaceinject/InterfaceInjectionProcessor.java

@@ -46,6 +46,9 @@
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.signature.SignatureReader;
+import org.objectweb.asm.signature.SignatureVisitor;
+import org.objectweb.asm.util.CheckSignatureAdapter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -54,11 +57,14 @@
 import net.fabricmc.loom.api.processor.ProcessorContext;
 import net.fabricmc.loom.api.processor.SpecContext;
 import net.fabricmc.loom.util.Constants;
+import net.fabricmc.loom.util.LazyCloseable;
 import net.fabricmc.loom.util.Pair;
 import net.fabricmc.loom.util.ZipUtils;
 import net.fabricmc.loom.util.fmj.FabricModJson;
 import net.fabricmc.mappingio.tree.MappingTree;
 import net.fabricmc.mappingio.tree.MemoryMappingTree;
+import net.fabricmc.tinyremapper.TinyRemapper;
+import net.fabricmc.tinyremapper.api.TrRemapper;
 
 public abstract class InterfaceInjectionProcessor implements MinecraftJarProcessor<InterfaceInjectionProcessor.Spec> {
 	private static final Logger LOGGER = LoggerFactory.getLogger(InterfaceInjectionProcessor.class);
@@ -105,22 +111,36 @@ public void processJar(Path jar, Spec spec, ProcessorContext context) throws IOE
 		final MemoryMappingTree mappings = context.getMappings();
 		final int intermediaryIndex = mappings.getNamespaceId(MappingsNamespace.INTERMEDIARY.toString());
 		final int namedIndex = mappings.getNamespaceId(MappingsNamespace.NAMED.toString());
-		final List<InjectedInterface> remappedInjectedInterfaces = spec.injectedInterfaces().stream()
-				.map(injectedInterface -> remap(injectedInterface, s -> mappings.mapClassName(s, intermediaryIndex, namedIndex)))
-				.toList();
-
-		try {
-			ZipUtils.transform(jar, getTransformers(remappedInjectedInterfaces));
-		} catch (IOException e) {
-			throw new RuntimeException("Failed to apply interface injections to " + jar, e);
+
+		try (LazyCloseable<TinyRemapper> tinyRemapper = context.createRemapper(MappingsNamespace.INTERMEDIARY, MappingsNamespace.NAMED)) {
+			final List<InjectedInterface> remappedInjectedInterfaces = spec.injectedInterfaces().stream()
+					.map(injectedInterface -> remap(
+							injectedInterface,
+							s -> mappings.mapClassName(s, intermediaryIndex, namedIndex),
+							tinyRemapper.get().getEnvironment().getRemapper()
+					))
+					.toList();
+			try {
+				ZipUtils.transform(jar, getTransformers(remappedInjectedInterfaces));
+			} catch (IOException e) {
+				throw new RuntimeException("Failed to apply interface injections to " + jar, e);
+			}
 		}
 	}
 
-	private InjectedInterface remap(InjectedInterface in, Function<String, String> remapper) {
+	private InjectedInterface remap(InjectedInterface in, Function<String, String> remapper, TrRemapper signatureRemapper) {
+		String generics = null;
+
+		if (in.generics() != null) {
+			String fakeSignature = signatureRemapper.mapSignature("Ljava/lang/Object" + in.generics() + ";", false); // Turning the raw generics string into a fake signature
+			generics = fakeSignature.substring("Ljava/lang/Object".length(), fakeSignature.length() - 1); // Retrieving the remapped raw generics string from the remapped fake signature
+		}
+
 		return new InjectedInterface(
 				in.modId(),
 				remapper.apply(in.className()),
-				remapper.apply(in.ifaceName())
+				remapper.apply(in.ifaceName()),
+				generics
 		);
 	}
 
@@ -196,7 +216,7 @@ private static String appendComment(String comment, List<InjectedInterface> inje
 		return comment;
 	}
 
-	private record InjectedInterface(String modId, String className, String ifaceName) {
+	private record InjectedInterface(String modId, String className, String ifaceName, @Nullable String generics) {
 		public static List<InjectedInterface> fromMod(FabricModJson fabricModJson) {
 			final String modId = fabricModJson.getId();
 			final JsonElement jsonElement = fabricModJson.getCustom(Constants.CustomModJsonKeys.INJECTED_INTERFACE);
@@ -210,10 +230,25 @@ public static List<InjectedInterface> fromMod(FabricModJson fabricModJson) {
 			final List<InjectedInterface> result = new ArrayList<>();
 
 			for (String className : addedIfaces.keySet()) {
-				final JsonArray ifaceNames = addedIfaces.getAsJsonArray(className);
+				final JsonArray ifacesInfo = addedIfaces.getAsJsonArray(className);
+
+				for (JsonElement ifaceElement : ifacesInfo) {
+					String ifaceInfo = ifaceElement.getAsString();
 
-				for (JsonElement ifaceName : ifaceNames) {
-					result.add(new InjectedInterface(modId, className, ifaceName.getAsString()));
+					String name = ifaceInfo;
+					String generics = null;
+
+					if (ifaceInfo.contains("<") && ifaceInfo.contains(">")) {
+						name = ifaceInfo.substring(0, ifaceInfo.indexOf("<"));
+						generics = ifaceInfo.substring(ifaceInfo.indexOf("<"));
+
+						// First Generics Check, if there are generics, are them correctly written?
+						SignatureReader reader = new SignatureReader("Ljava/lang/Object" + generics + ";");
+						CheckSignatureAdapter checker = new CheckSignatureAdapter(CheckSignatureAdapter.CLASS_SIGNATURE, null);
+						reader.accept(checker);
+					}
+
+					result.add(new InjectedInterface(modId, className, name, generics));
 				}
 			}
 
@@ -226,6 +261,16 @@ public static List<InjectedInterface> fromMods(List<FabricModJson> fabricModJson
 					.flatMap(List::stream)
 					.toList();
 		}
+
+		public static boolean containsGenerics(List<InjectedInterface> injectedInterfaces) {
+			for (InjectedInterface injectedInterface : injectedInterfaces) {
+				if (injectedInterface.generics() != null) {
+					return true;
+				}
+			}
+
+			return false;
+		}
 	}
 
 	private static class InjectingClassVisitor extends ClassVisitor {
@@ -241,6 +286,7 @@ private static class InjectingClassVisitor extends ClassVisitor {
 
 		@Override
 		public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
+			String[] baseInterfaces = interfaces.clone();
 			Set<String> modifiedInterfaces = new LinkedHashSet<>(interfaces.length + injectedInterfaces.size());
 			Collections.addAll(modifiedInterfaces, interfaces);
 
@@ -249,11 +295,35 @@ public void visit(int version, int access, String name, String signature, String
 			}
 
 			// See JVMS: https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-ClassSignature
+			if (InjectedInterface.containsGenerics(injectedInterfaces) && signature == null) {
+				// Classes that are not using generics don't need signatures, so their signatures are null
+				// If the class is not using generics but that an injected interface targeting the class is using them, we are creating the class signature
+				StringBuilder baseSignatureBuilder = new StringBuilder("L" + superName + ";");
+
+				for (String baseInterface : baseInterfaces) {
+					baseSignatureBuilder.append("L").append(baseInterface).append(";");
+				}
+
+				signature = baseSignatureBuilder.toString();
+			}
+
 			if (signature != null) {
+				SignatureReader reader = new SignatureReader(signature);
+
+				// Second Generics Check, if there are passed generics, are all of them present in the target class?
+				GenericsChecker checker = new GenericsChecker(Constants.ASM_VERSION, injectedInterfaces);
+				reader.accept(checker);
+
 				var resultingSignature = new StringBuilder(signature);
 
 				for (InjectedInterface injectedInterface : injectedInterfaces) {
-					String superinterfaceSignature = "L" + injectedInterface.ifaceName() + ";";
+					String superinterfaceSignature;
+
+					if (injectedInterface.generics() != null) {
+						superinterfaceSignature = "L" + injectedInterface.ifaceName() + injectedInterface.generics() + ";";
+					} else {
+						superinterfaceSignature = "L" + injectedInterface.ifaceName() + ";";
+					}
 
 					if (resultingSignature.indexOf(superinterfaceSignature) == -1) {
 						resultingSignature.append(superinterfaceSignature);
@@ -314,4 +384,72 @@ public void visitEnd() {
 			super.visitEnd();
 		}
 	}
+
+	private static class GenericsChecker extends SignatureVisitor {
+		private final List<String> typeParameters;
+
+		private final List<InjectedInterface> injectedInterfaces;
+
+		GenericsChecker(int asmVersion, List<InjectedInterface> injectedInterfaces) {
+			super(asmVersion);
+			this.typeParameters = new ArrayList<>();
+			this.injectedInterfaces = injectedInterfaces;
+		}
+
+		@Override
+		public void visitFormalTypeParameter(String name) {
+			this.typeParameters.add(name);
+			super.visitFormalTypeParameter(name);
+		}
+
+		@Override
+		public void visitEnd() {
+			for (InjectedInterface injectedInterface : this.injectedInterfaces) {
+				if (injectedInterface.generics() != null) {
+					SignatureReader reader = new SignatureReader("Ljava/lang/Object" + injectedInterface.generics() + ";");
+					GenericsConfirm confirm = new GenericsConfirm(
+							Constants.ASM_VERSION,
+							injectedInterface.className(),
+							injectedInterface.ifaceName(),
+							this.typeParameters
+					);
+					reader.accept(confirm);
+				}
+			}
+
+			super.visitEnd();
+		}
+
+		public static class GenericsConfirm extends SignatureVisitor {
+			private final String className;
+
+			private final String interfaceName;
+
+			private final List<String> acceptedTypeVariables;
+
+			GenericsConfirm(int asmVersion, String className, String interfaceName, List<String> acceptedTypeVariables) {
+				super(asmVersion);
+				this.className = className;
+				this.interfaceName = interfaceName;
+				this.acceptedTypeVariables = acceptedTypeVariables;
+			}
+
+			@Override
+			public void visitTypeVariable(String name) {
+				if (!this.acceptedTypeVariables.contains(name)) {
+					throw new IllegalStateException(
+							"Interface "
+							+ this.interfaceName
+							+ " attempted to use a type variable named "
+							+ name
+							+ " which is not present in the "
+							+ this.className
+							+ " class"
+					);
+				}
+
+				super.visitTypeVariable(name);
+			}
+		}
+	}
 }

src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy

@@ -32,17 +32,31 @@ import com.google.gson.JsonObject
 import spock.lang.Specification
 import spock.lang.TempDir
 
+import net.fabricmc.loom.api.mappings.layered.MappingsNamespace
 import net.fabricmc.loom.api.processor.ProcessorContext
 import net.fabricmc.loom.api.processor.SpecContext
 import net.fabricmc.loom.configuration.ifaceinject.InterfaceInjectionProcessor
+import net.fabricmc.loom.test.unit.processor.classes.AdvancedGenericInterface
+import net.fabricmc.loom.test.unit.processor.classes.AdvancedGenericTargetClass
+import net.fabricmc.loom.test.unit.processor.classes.DoubleGenericTargetClass
+import net.fabricmc.loom.test.unit.processor.classes.FirstGenericInterface
+import net.fabricmc.loom.test.unit.processor.classes.GenericInterface
+import net.fabricmc.loom.test.unit.processor.classes.GenericTargetClass
+import net.fabricmc.loom.test.unit.processor.classes.PassingGenericTargetClass
+import net.fabricmc.loom.test.unit.processor.classes.SecondGenericInterface
+import net.fabricmc.loom.test.unit.processor.classes.SelfGenericInterface
+import net.fabricmc.loom.test.unit.processor.classes.SelfGenericTargetClass
 import net.fabricmc.loom.test.unit.processor.classes.SimpleInterface
 import net.fabricmc.loom.test.unit.processor.classes.SimpleTargetClass
 import net.fabricmc.loom.util.Constants
+import net.fabricmc.loom.util.LazyCloseable
 import net.fabricmc.loom.util.Pair
+import net.fabricmc.loom.util.TinyRemapperHelper
 import net.fabricmc.loom.util.ZipUtils
 import net.fabricmc.loom.util.fmj.FabricModJson
 import net.fabricmc.mappingio.MappingReader
 import net.fabricmc.mappingio.tree.MemoryMappingTree
+import net.fabricmc.tinyremapper.TinyRemapper
 
 class InterfaceInjectionProcessorTest extends Specification {
 	@TempDir
@@ -64,6 +78,8 @@ class InterfaceInjectionProcessorTest extends Specification {
 		def jar = tempDir.resolve("test.jar")
 		packageJar(jar)
 
+		processorContext.createRemapper(MappingsNamespace.INTERMEDIARY, MappingsNamespace.NAMED) >> createRemapper(jar, processorContext.getMappings())
+
 		when:
 		def processor = new TestInterfaceInjectionProcessor()
 		def spec = processor.buildSpec(specContext)
@@ -83,10 +99,44 @@ class InterfaceInjectionProcessorTest extends Specification {
 		}
 
 		// Inner class with a simple interface
-		"class_1\$class_2" | "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" |  SimpleTargetClass.Inner.class | { Class<?> loadedClass ->
+		"class_1\$class_2" | "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" | SimpleTargetClass.Inner.class | { Class<?> loadedClass ->
 			loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface"
 			loadedClass.constructors.first().newInstance().injectedMethod() == 123
 		}
+
+		// Class using interface with generics
+		"class_3" | "net/fabricmc/loom/test/unit/processor/classes/GenericInterface<Ljava/lang/String;>" | GenericTargetClass.class | { Class<?> loadedClass ->
+			loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/GenericInterface"
+			loadedClass.constructors.first().newInstance().genericInjectedMethod() == null
+		}
+
+		// Class using generics and passing them to interface
+		"class_4" | "net/fabricmc/loom/test/unit/processor/classes/GenericInterface<TT;>" | PassingGenericTargetClass.class | { Class<?> loadedClass ->
+			loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/GenericInterface"
+			loadedClass.constructors.first().newInstance().genericInjectedMethod() == null
+		}
+
+		// Class having one injected interface with two generics, including one provided by the class
+		"class_5" | "net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface<Ljava/util/function/Predicate<TT;>;Ljava/lang/Integer;>" | AdvancedGenericTargetClass.class | { Class<?> loadedClass ->
+			loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface"
+			loadedClass.constructors.first().newInstance().advancedGenericInjectedMethod().getClass() == AdvancedGenericTargetClass.Pair.class
+		}
+
+		// Class having two injected interfaces with one generic for each of them, including one provided by the class
+		"class_7" | "net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface<Ljava/util/function/Predicate<TT;>;>" | DoubleGenericTargetClass.class | { Class<?> loadedClass ->
+			loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface"
+			loadedClass.constructors.first().newInstance().firstGenericInjectedMethod() == null
+		}
+		"class_7" | "net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface<Ljava/lang/Integer;>" | DoubleGenericTargetClass.class | { Class<?> loadedClass ->
+			loadedClass.interfaces.last().name == "net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface"
+			loadedClass.constructors.last().newInstance().secondGenericInjectedMethod() == null
+		}
+
+		// Self Generic Types + Signature Remapping Check
+		"class_8" | "net/fabricmc/loom/test/unit/processor/classes/SelfGenericInterface<Lclass_7;>" | SelfGenericTargetClass.class | { Class<?> loadedClass ->
+			loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/proessor/classes/SelfGenericInterface"
+			loadedClass.constructors.first().newInstance().selfGenericInjectedMethod() == null
+		}
 	}
 
 	def "nothing to inject"() {
@@ -130,6 +180,16 @@ class InterfaceInjectionProcessorTest extends Specification {
 		return mappings
 	}
 
+	static LazyCloseable<TinyRemapper> createRemapper(Path jar, MemoryMappingTree mappings) {
+		return new LazyCloseable<>({
+			TinyRemapper.Builder builder = TinyRemapper.newRemapper()
+			builder.withMappings(TinyRemapperHelper.create(mappings, MappingsNamespace.INTERMEDIARY.toString(), MappingsNamespace.NAMED.toString(), false))
+			TinyRemapper tinyRemapper = builder.build()
+			tinyRemapper.readClassPath(jar)
+			return tinyRemapper
+		}, { tinyRemapper -> tinyRemapper.finish() })
+	}
+
 	// Load a class from a jar file and execute a closure with it
 	static void withTargetClass(Path jar, Class<?> clazz, Consumer<Class<?>> closure) {
 		// Groovy is needed as the test classes are compiled with it
@@ -159,11 +219,29 @@ class InterfaceInjectionProcessorTest extends Specification {
 	private static final List<Class<?>> CLASSES_TO_PACKAGE = [
 		SimpleTargetClass.class,
 		SimpleTargetClass.Inner.class,
-		SimpleInterface.class
+		SimpleInterface.class,
+		GenericTargetClass.class,
+		PassingGenericTargetClass.class,
+		GenericInterface.class,
+		AdvancedGenericTargetClass.class,
+		AdvancedGenericTargetClass.Pair.class,
+		AdvancedGenericInterface.class,
+		DoubleGenericTargetClass.class,
+		FirstGenericInterface.class,
+		SecondGenericInterface.class,
+		SelfGenericTargetClass.class,
+		SelfGenericInterface.class
 	]
+
 	private static final String MAPPINGS = """
 tiny\t2\t0\tintermediary\tnamed
 c\tclass_1\tnet/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass
 c\tclass_1\$class_2\tnet/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass\$Inner
+c\tclass_3\tnet/fabricmc/loom/test/unit/processor/classes/GenericTargetClass
+c\tclass_4\tnet/fabricmc/loom/test/unit/processor/classes/PassingGenericTargetClass
+c\tclass_5\tnet/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass
+c\tclass_5\$class_6\tnet/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass\$Pair
+c\tclass_7\tnet/fabricmc/loom/test/unit/processor/classes/DoubleGenericTargetClass
+c\tclass_8\tnet/fabricmc/loom/test/unit/processor/classes/SelfGenericTargetClass
 """.trim()
-}
+}