Don't validate mixin names with more than one target.

Add tests.
This commit is contained in:
modmuss50
2022-08-08 18:53:12 +01:00
parent 7d92647203
commit 813c5ead14
3 changed files with 141 additions and 21 deletions

View File

@@ -122,6 +122,10 @@ dependencies {
compileOnly 'org.jetbrains:annotations:23.0.0'
testCompileOnly 'org.jetbrains:annotations:23.0.0'
testCompileOnly ('net.fabricmc:sponge-mixin:0.11.4+mixin.0.8.5') {
transitive = false
}
}
jar {

View File

@@ -48,6 +48,7 @@ import org.gradle.workers.WorkParameters;
import org.gradle.workers.WorkQueue;
import org.gradle.workers.WorkerExecutor;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.VisibleForTesting;
import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
@@ -115,7 +116,7 @@ public abstract class ValidateMixinNameTask extends SourceTask {
}
final String mixinClassName = toSimpleName(mixin.className);
final String expectedMixinClassName = toSimpleName(mixin.target.getInternalName()).replace("$", "") + (mixin.accessor ? "Accessor" : "Mixin");
final String expectedMixinClassName = mixin.expectedClassName();
if (expectedMixinClassName.startsWith("class_")) {
// Don't enforce intermediary named mixins.
@@ -140,36 +141,48 @@ public abstract class ValidateMixinNameTask extends SourceTask {
throw new GradleException("Mixin name validation failed: " + errors.stream().collect(Collectors.joining(System.lineSeparator())));
}
}
private static String toSimpleName(String internalName) {
return internalName.substring(internalName.lastIndexOf("/") + 1);
}
private static String toSimpleName(String internalName) {
return internalName.substring(internalName.lastIndexOf("/") + 1);
}
@Nullable
private Mixin getMixin(File file) {
try (InputStream is = new FileInputStream(file)) {
ClassReader reader = new ClassReader(is);
var classVisitor = new MixinTargetClassVisitor();
reader.accept(classVisitor, ClassReader.SKIP_CODE);
if (classVisitor.mixinTarget != null) {
return new Mixin(classVisitor.className, classVisitor.mixinTarget, classVisitor.accessor);
}
} catch (IOException e) {
throw new UncheckedIOException("Failed to read input file: " + file, e);
}
return null;
@VisibleForTesting
public record Mixin(String className, Type target, boolean accessor) {
public String expectedClassName() {
return toSimpleName(target.getInternalName()).replace("$", "") + (accessor ? "Accessor" : "Mixin");
}
}
private record Mixin(String className, Type target, boolean accessor) { }
@Nullable
private static Mixin getMixin(File file) {
try (InputStream is = new FileInputStream(file)) {
return getMixin(is);
} catch (IOException e) {
throw new UncheckedIOException("Failed to read input file: " + file, e);
}
}
@Nullable
@VisibleForTesting
public static Mixin getMixin(InputStream is) throws IOException {
final ClassReader reader = new ClassReader(is);
var classVisitor = new MixinTargetClassVisitor();
reader.accept(classVisitor, ClassReader.SKIP_CODE);
if (classVisitor.mixinTarget != null && classVisitor.targets == 1) {
return new Mixin(classVisitor.className, classVisitor.mixinTarget, classVisitor.accessor);
}
return null;
}
private static class MixinTargetClassVisitor extends ClassVisitor {
Type mixinTarget;
String className;
boolean accessor;
int targets = 0;
boolean isInterface;
@@ -220,6 +233,7 @@ public abstract class ValidateMixinNameTask extends SourceTask {
@Override
public void visit(String name, Object value) {
mixinTarget = Objects.requireNonNull((Type) value);
targets++;
super.visit(name, value);
}

View File

@@ -0,0 +1,102 @@
/*
* This file is part of fabric-loom, licensed under the MIT License (MIT).
*
* Copyright (c) 2022 FabricMC
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
package net.fabricmc.loom.test.unit
import net.fabricmc.loom.task.ValidateMixinNameTask
import org.spongepowered.asm.mixin.Mixin
import org.spongepowered.asm.mixin.gen.Accessor
import spock.lang.Specification
class ValidateMixinNameTest extends Specification {
def "TestMixin"() {
when:
def mixin = getMixin(TestMixin.class)
then:
mixin.className() == "net/fabricmc/loom/test/unit/TestMixin"
mixin.target().internalName == "net/fabricmc/loom/test/unit/Test"
mixin.expectedClassName() == "TestMixin"
!mixin.accessor()
}
def "TestInnerMixin"() {
when:
def mixin = getMixin(TestInnerMixin.class)
then:
mixin.className() == "net/fabricmc/loom/test/unit/TestInnerMixin"
mixin.target().internalName == "net/fabricmc/loom/test/unit/Test\$Inner"
mixin.expectedClassName() == "TestInnerMixin"
!mixin.accessor()
}
def "TestAccessor"() {
when:
def mixin = getMixin(TestAccessor.class)
then:
mixin.className() == "net/fabricmc/loom/test/unit/TestAccessor"
mixin.target().internalName == "net/fabricmc/loom/test/unit/Test"
mixin.expectedClassName() == "TestAccessor"
mixin.accessor()
}
def "TestManyTargetsMixin"() {
when:
def mixin = getMixin(TestManyTargetsMixin.class)
then:
mixin == null
}
static ValidateMixinNameTask.Mixin getMixin(Class<?> clazz) {
return getInput(clazz).withCloseable {
return ValidateMixinNameTask.getMixin(it)
}
}
static InputStream getInput(Class<?> clazz) {
return clazz.classLoader.getResourceAsStream(clazz.name.replace('.', '/') + ".class")
}
}
@Mixin(Test.class)
class TestMixin {
}
@Mixin(Test.Inner.class)
class TestInnerMixin {
}
@Mixin(Test.class)
interface TestAccessor {
@Accessor
Object getNothing();
}
@Mixin([Test.class, Test.Inner.class])
class TestManyTargetsMixin {
}
class Test {
class Inner {
}
}