Allow mixining interfaces

Reapplies FabricMC#13 Fixes FabricMC#34 Includes (a form of) SpongePowered#413 and SpongePowered#415
Chocohead · Jun 16, 2021 · 2731d9d · 2731d9d
1 parent cf6c9a2
commit 2731d9d
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 32 deletions.
diff --git a/src/ap/java/org/spongepowered/tools/obfuscation/AnnotatedMixinElementHandlerInjector.java b/src/ap/java/org/spongepowered/tools/obfuscation/AnnotatedMixinElementHandlerInjector.java
@@ -152,10 +152,6 @@ public void notifyRemapped() {
     }
 
     public void registerInjector(AnnotatedElementInjector elem) {
-        if (this.mixin.isInterface()) {
-            this.ap.printMessage(Kind.ERROR, "Injector in interface is unsupported", elem.getElement());
-        }
-
         for (String reference : elem.getAnnotation().<String>getList("method")) {
             ITargetSelector targetSelector = TargetSelector.parse(reference);
 
@@ -260,10 +256,6 @@ private boolean registerInjector(AnnotatedElementInjector elem, String reference
      * and process the references
      */
     public void registerInjectionPoint(AnnotatedElementInjectionPoint elem, String format) {
-        if (this.mixin.isInterface()) {
-            this.ap.printMessage(Kind.ERROR, "Injector in interface is unsupported", elem.getElement());
-        }
-
         if (!elem.shouldRemap()) {
             return;
         }

diff --git a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGenerator.java b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGenerator.java
@@ -26,11 +26,11 @@
 
 import java.util.ArrayList;
 
+import org.apache.logging.log4j.LogManager;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.AnnotationNode;
 import org.objectweb.asm.tree.MethodNode;
-import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException;
-import org.spongepowered.asm.mixin.refmap.IMixinContext;
+import org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException;
 import org.spongepowered.asm.util.asm.ASM;
 
 /**
@@ -54,10 +54,12 @@ public AccessorGenerator(AccessorInfo info, boolean isStatic) {
     }
 
     protected void checkModifiers() {
-        if (this.info.isStatic() && !this.targetIsStatic) {
-            IMixinContext context = this.info.getContext();
-            throw new InvalidInjectionException(context, String.format("%s is invalid. Accessor method is%s static but the target is not.",
-                    this.info, this.info.isStatic() ? "" : " not"));
+        if (this.info.isStatic() != this.targetIsStatic) {
+            if (!this.targetIsStatic) {
+                throw new InvalidAccessorException(this.info, String.format("%s is invalid. Accessor method is static but the target is not.", this.info));
+            } else {
+                LogManager.getLogger("mixin").info("{} could be static as its target is", this.info);
+            }
         }
     }
 
@@ -70,7 +72,7 @@ protected void checkModifiers() {
      */
     protected final MethodNode createMethod(int maxLocals, int maxStack) {
         MethodNode method = this.info.getMethod();
-        MethodNode accessor = new MethodNode(ASM.API_VERSION, (method.access & ~Opcodes.ACC_ABSTRACT) | Opcodes.ACC_SYNTHETIC, method.name,
+        MethodNode accessor = new MethodNode(ASM.API_VERSION, (method.access & ~Opcodes.ACC_ABSTRACT) | Opcodes.ACC_SYNTHETIC | (this.targetIsStatic ? Opcodes.ACC_STATIC : 0), method.name,
                 method.desc, null, null);
         accessor.visibleAnnotations = new ArrayList<AnnotationNode>();
         accessor.visibleAnnotations.add(this.info.getAnnotation());

diff --git a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorFieldSetter.java b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorFieldSetter.java
@@ -31,6 +31,7 @@
 import org.objectweb.asm.tree.MethodNode;
 import org.objectweb.asm.tree.VarInsnNode;
 import org.spongepowered.asm.mixin.MixinEnvironment.Option;
+import org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException;
 import org.spongepowered.asm.mixin.Mutable;
 import org.spongepowered.asm.mixin.transformer.ClassInfo.Method;
 import org.spongepowered.asm.mixin.transformer.MixinTargetContext;
@@ -52,6 +53,12 @@ public AccessorGeneratorFieldSetter(AccessorInfo info) {
 
     @Override
     public void validate() {
+        if (Bytecode.hasFlag(this.info.getClassNode(), Opcodes.ACC_INTERFACE)) {
+            //This will result in a ClassFormatError when the class is verified
+            throw new InvalidAccessorException(info, String.format("%s tried to change interface field %s::%s",
+                    this.info, this.info.getClassNode().name, this.targetField.name));
+        }
+
         super.validate();
 
         Method method = this.info.getClassInfo().findMethod(this.info.getMethod());

diff --git a/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorMethodProxy.java b/src/main/java/org/spongepowered/asm/mixin/gen/AccessorGeneratorMethodProxy.java
@@ -75,9 +75,10 @@ public MethodNode generate() {
             method.instructions.add(new VarInsnNode(Opcodes.ALOAD, 0));
         }
         Bytecode.loadArgs(this.argTypes, method.instructions, this.targetIsStatic ? 0 : 1);
+        boolean isInterface = Bytecode.hasFlag(this.info.getClassNode(), Opcodes.ACC_INTERFACE);
         boolean isPrivate = Bytecode.hasFlag(this.targetMethod, Opcodes.ACC_PRIVATE);
-        int opcode = this.targetIsStatic ? Opcodes.INVOKESTATIC : (isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL);
-        method.instructions.add(new MethodInsnNode(opcode, this.info.getClassNode().name, this.targetMethod.name, this.targetMethod.desc, false));
+        int opcode = this.targetIsStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : (isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL);
+        method.instructions.add(new MethodInsnNode(opcode, this.info.getClassNode().name, this.targetMethod.name, this.targetMethod.desc, isInterface));
         method.instructions.add(new InsnNode(this.returnType.getOpcode(Opcodes.IRETURN)));
         return method;
     }

diff --git a/src/main/java/org/spongepowered/asm/mixin/injection/code/Injector.java b/src/main/java/org/spongepowered/asm/mixin/injection/code/Injector.java
@@ -405,9 +405,10 @@ protected AbstractInsnNode invokeHandler(InsnList insns) {
      * @return injected insn node
      */
     protected AbstractInsnNode invokeHandler(InsnList insns, MethodNode handler) {
+        boolean isInterface = Bytecode.hasFlag(classNode, Opcodes.ACC_INTERFACE);
         boolean isPrivate = (handler.access & Opcodes.ACC_PRIVATE) != 0;
-        int invokeOpcode = this.isStatic ? Opcodes.INVOKESTATIC : isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL;
-        MethodInsnNode insn = new MethodInsnNode(invokeOpcode, this.classNode.name, handler.name, handler.desc, false);
+        int invokeOpcode = this.isStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL;
+        MethodInsnNode insn = new MethodInsnNode(invokeOpcode, this.classNode.name, handler.name, handler.desc, isInterface);
         insns.add(insn);
         this.info.addCallbackInvocation(handler);
         return insn;

diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinApplicatorInterface.java
@@ -24,10 +24,13 @@
  */
 package org.spongepowered.asm.mixin.transformer;
 
+import java.lang.reflect.Modifier;
 import java.util.Map.Entry;
 
 import org.objectweb.asm.tree.FieldNode;
 import org.objectweb.asm.tree.MethodNode;
+import org.spongepowered.asm.mixin.MixinEnvironment;
+import org.spongepowered.asm.mixin.MixinEnvironment.CompatibilityLevel.LanguageFeature;
 import org.spongepowered.asm.mixin.injection.struct.InjectionInfo;
 import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException;
 import org.spongepowered.asm.mixin.transformer.ClassInfo.Field;
@@ -96,28 +99,34 @@ protected void applyInitialisers(MixinTargetContext mixin) {
      */
     @Override
     protected void prepareInjections(MixinTargetContext mixin) {
-        // disabled for interface mixins
         for (MethodNode method : this.targetClass.methods) {
             try {
                 InjectionInfo injectInfo = InjectionInfo.parse(mixin, method);
                 if (injectInfo != null) {
-                    throw new InvalidInterfaceMixinException(mixin, injectInfo + " is not supported on interface mixin method " + method.name);
+                    //Make sure we're running on a Java version which supports interfaces having method bodies
+                    if (!MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.METHODS_IN_INTERFACES)) {
+                        throw new InvalidInterfaceMixinException(mixin, injectInfo + " is not supported on interface mixin method " + method.name);
+                    }
                 }
             } catch (InvalidInjectionException ex) {
                 String description = ex.getInjectionInfo() != null ? ex.getInjectionInfo().toString() : "Injection";
                 throw new InvalidInterfaceMixinException(mixin, description + " is not supported in interface mixin");
             }
         }
+
+        super.prepareInjections(mixin);
     }
 
-    /* (non-Javadoc)
-     * @see org.spongepowered.asm.mixin.transformer.MixinApplicator
-     *      #applyInjections(
-     *      org.spongepowered.asm.mixin.transformer.MixinTargetContext)
-     */
     @Override
-    protected void applyInjections(MixinTargetContext mixin) {
-        // Do nothing
-    }
+    protected void checkMethodVisibility(MixinTargetContext mixin, MethodNode mixinMethod) {
+        //Allow injecting into static interface methods where it isn't possible to control the access of the injection method
+        if (Modifier.isStatic(mixinMethod.access) && !MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.PRIVATE_METHODS_IN_INTERFACES)) {
+            InjectionInfo injectInfo = InjectionInfo.parse(mixin, mixinMethod);
+            if (injectInfo != null) {
+                return;
+            }
+        }
 
+        super.checkMethodVisibility(mixin, mixinMethod);
+    }
 }
diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPostProcessor.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPostProcessor.java
@@ -213,7 +213,7 @@ private static void createProxy(MethodNode methodNode, ClassInfo targetClass, Me
         Type[] args = Type.getArgumentTypes(methodNode.desc);
         Type returnType = Type.getReturnType(methodNode.desc);
         Bytecode.loadArgs(args, methodNode.instructions, 0);
-        methodNode.instructions.add(new MethodInsnNode(Opcodes.INVOKESTATIC, targetClass.getName(), method.getName(), methodNode.desc, false));
+        methodNode.instructions.add(new MethodInsnNode(Opcodes.INVOKESTATIC, targetClass.getName(), method.getName(), methodNode.desc, targetClass.isInterface()));
         methodNode.instructions.add(new InsnNode(returnType.getOpcode(Opcodes.IRETURN)));
         methodNode.maxStack = Bytecode.getFirstNonArgLocalIndex(args, false);
         methodNode.maxLocals = 0;

diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinPreProcessorInterface.java
@@ -27,6 +27,8 @@
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.tree.AnnotationNode;
 import org.objectweb.asm.tree.FieldNode;
+import org.spongepowered.asm.mixin.MixinEnvironment;
+import org.spongepowered.asm.mixin.MixinEnvironment.CompatibilityLevel.LanguageFeature;
 import org.spongepowered.asm.mixin.transformer.ClassInfo.Method;
 import org.spongepowered.asm.mixin.transformer.MixinInfo.MixinClassNode;
 import org.spongepowered.asm.mixin.transformer.MixinInfo.MixinMethodNode;
@@ -58,8 +60,11 @@ class MixinPreProcessorInterface extends MixinPreProcessorStandard {
     protected void prepareMethod(MixinMethodNode mixinMethod, Method method) {
         // Userland interfaces should not have non-public methods except for lambda bodies
         if (!Bytecode.hasFlag(mixinMethod, Opcodes.ACC_PUBLIC) && !Bytecode.hasFlag(mixinMethod, Opcodes.ACC_SYNTHETIC)) {
-            throw new InvalidInterfaceMixinException(this.mixin, "Interface mixin contains a non-public method! Found " + method + " in "
-                    + this.mixin);
+            //On versions that support it private methods are also allowed
+            if (!Bytecode.hasFlag(mixinMethod, Opcodes.ACC_PRIVATE) || !MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.PRIVATE_METHODS_IN_INTERFACES)) {
+                throw new InvalidInterfaceMixinException(this.mixin, "Interface mixin contains a non-public method! Found " + method + " in "
+                        + this.mixin);
+            }
         }
 
         super.prepareMethod(mixinMethod, method);