only run modifications if the modifers don't match

PaperMC · Mar 30, 2024 · 8f952a4 · 8f952a4
1 parent e0b76de
commit 8f952a4
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 6 deletions.
diff --git a/src/main/java/io/papermc/restamp/RestampContextConfiguration.java b/src/main/java/io/papermc/restamp/RestampContextConfiguration.java
@@ -120,7 +120,7 @@ public Builder accessTransformers(@NotNull final Path accessTransformerPath, @No
          * @return this builder.
          */
         @NotNull
-        @Contract(value = "_,_ -> this", mutates = "this")
+        @Contract(value = "_ -> this", mutates = "this")
         public Builder accessTransformSet(@NotNull final AccessTransformSet accessTransformSet) {
             this.accessTransformSet = AccessTransformSet.create();
             this.accessTransformSet.merge(accessTransformSet);

diff --git a/src/main/java/io/papermc/restamp/recipe/ClassATMutator.java b/src/main/java/io/papermc/restamp/recipe/ClassATMutator.java
@@ -53,13 +53,15 @@ public J.ClassDeclaration visitClassDeclaration(@NotNull final J.ClassDeclaratio
                 final AccessTransform accessTransform = transformerClass.get();
                 if (accessTransform.isEmpty()) return classDeclaration;
 
-                transformerClass.replace(AccessTransform.EMPTY); // Mark as consumed
-
                 final ModifierTransformationResult transformationResult = modifierTransformer.transformModifiers(
                     accessTransform,
                     classDeclaration.getModifiers(),
                     classDeclaration.getAnnotations().getKind().getPrefix()
                 );
+                if (transformationResult.newModifiers().equals(classDeclaration.getModifiers())) {
+                    return classDeclaration;
+                }
+                transformerClass.replace(AccessTransform.EMPTY); // Mark as consumed
 
                 return classDeclaration
                     .withModifiers(transformationResult.newModifiers())

diff --git a/src/main/java/io/papermc/restamp/recipe/FieldATMutator.java b/src/main/java/io/papermc/restamp/recipe/FieldATMutator.java
@@ -60,7 +60,7 @@ public J.VariableDeclarations visitVariableDeclarations(@NotNull final J.Variabl
 
                 // Fetch access transformer to apply to specific field.
                 final AccessTransform accessTransformToApply = variableDeclarations.getVariables().stream()
-                    .map(n -> transformerClass.replaceField(n.getSimpleName(), AccessTransform.EMPTY))
+                    .map(n -> transformerClass.getField(n.getSimpleName()))
                     .filter(Objects::nonNull)
                     .reduce(AccessTransform::merge)
                     .orElse(AccessTransform.EMPTY);
@@ -72,6 +72,12 @@ public J.VariableDeclarations visitVariableDeclarations(@NotNull final J.Variabl
                     variableDeclarations.getModifiers(),
                     Optional.ofNullable(variableDeclarations.getTypeExpression()).map(J::getPrefix).orElse(Space.EMPTY)
                 );
+
+                if (transformationResult.newModifiers().equals(variableDeclarations.getModifiers())) {
+                    return variableDeclarations;
+                }
+                variableDeclarations.getVariables().forEach(n -> transformerClass.replaceField(n.getSimpleName(), AccessTransform.EMPTY)); // Mark as consumed
+
                 final J.VariableDeclarations updated = variableDeclarations
                     .withModifiers(transformationResult.newModifiers())
                     .withTypeExpression(variableDeclarations.getTypeExpression().withPrefix(transformationResult.parentSpace()));

diff --git a/src/main/java/io/papermc/restamp/recipe/MethodATMutator.java b/src/main/java/io/papermc/restamp/recipe/MethodATMutator.java
@@ -100,9 +100,9 @@ public J.MethodDeclaration visitMethodDeclaration(@NotNull final J.MethodDeclara
                     returnType = VoidType.INSTANCE;
                 }
 
-                final AccessTransform accessTransform = transformerClass.replaceMethod(new MethodSignature(
+                final AccessTransform accessTransform = transformerClass.getMethod(new MethodSignature(
                     atMethodName, new MethodDescriptor(parameterTypes, returnType)
-                ), AccessTransform.EMPTY);
+                ));
                 if (accessTransform == null || accessTransform.isEmpty()) return methodDeclaration;
 
                 final TypeTree returnTypeExpression = methodDeclaration.getReturnTypeExpression();
@@ -112,6 +112,13 @@ atMethodName, new MethodDescriptor(parameterTypes, returnType)
                     Optional.ofNullable(returnTypeExpression).map(J::getPrefix).orElse(methodDeclaration.getName().getPrefix())
                 );
 
+                if (transformationResult.newModifiers().equals(methodDeclaration.getModifiers())) {
+                    return methodDeclaration;
+                }
+                transformerClass.replaceMethod(new MethodSignature(
+                    atMethodName, new MethodDescriptor(parameterTypes, returnType)
+                ), AccessTransform.EMPTY); // Mark as consumed
+
                 J.MethodDeclaration updated = methodDeclaration.withModifiers(transformationResult.newModifiers());
                 if (returnTypeExpression != null) {
                     updated = updated.withReturnTypeExpression(returnTypeExpression.withPrefix(transformationResult.parentSpace()));

diff --git a/src/test/java/io/papermc/restamp/at/ModifierTransformerTest.java b/src/test/java/io/papermc/restamp/at/ModifierTransformerTest.java
@@ -10,21 +10,63 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.ArgumentsSource;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.Space;
 import org.openrewrite.java.tree.TextComment;
 import org.openrewrite.marker.Markers;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Stream;
 
 import static io.papermc.restamp.RestampFunctionTestHelper.modifierFrom;
 
 class ModifierTransformerTest {
 
     private final ModifierTransformer transformer = new ModifierTransformer();
 
+    static Stream<Arguments> testSkipModifyTransformation() {
+        return Stream.of(
+            Arguments.of(
+                AccessTransform.of(AccessChange.PUBLIC, ModifierChange.REMOVE),
+                List.of(modifierFrom(Space.EMPTY, J.Modifier.Type.Public)),
+                false
+            ),
+            Arguments.of(
+                AccessTransform.of(AccessChange.PACKAGE_PRIVATE, ModifierChange.ADD),
+                List.of(modifierFrom(Space.EMPTY, J.Modifier.Type.Final)),
+                false
+            ),
+            Arguments.of(
+                AccessTransform.of(AccessChange.PUBLIC),
+                List.of(modifierFrom(Space.EMPTY, J.Modifier.Type.Private)),
+                true
+            ),
+            Arguments.of(
+                AccessTransform.of(AccessChange.PUBLIC),
+                List.of(modifierFrom(Space.EMPTY, J.Modifier.Type.Private), modifierFrom(Space.EMPTY, J.Modifier.Type.Final)),
+                true
+            ),
+            Arguments.of(
+                AccessTransform.of(ModifierChange.REMOVE),
+                List.of(modifierFrom(Space.EMPTY, J.Modifier.Type.Public), modifierFrom(Space.EMPTY, J.Modifier.Type.Final)),
+                true
+            )
+        );
+    }
+
+    @MethodSource()
+    @ParameterizedTest
+    public void testSkipModifyTransformation(@NotNull final AccessTransform transform,
+                                             @NotNull final List<J.Modifier> modifiers,
+                                             final boolean expected) {
+        final ModifierTransformationResult result = this.transformer.transformModifiers(transform, modifiers, Space.EMPTY);
+        Assertions.assertEquals(expected, !result.newModifiers().equals(modifiers));
+    }
+
     @ParameterizedTest
     @ArgumentsSource(RestampFunctionTestHelper.CartesianVisibilityArgumentProvider.class)
     public void testModifyVisibilityTransformation(@NotNull final AccessTransform current,