only run modifications if the modifers don't match

Author: Machine-Maker

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);

src/main/java/io/papermc/restamp/at/ModifierTransformer.java

@@ -30,6 +30,44 @@ public class ModifierTransformer {
         J.Modifier.Type.Final
     );
 
+    /**
+     * Checks if the provided list of modifiers doesn't already match the requested
+     * access transform.
+     * <p>
+     * Assumes that 1 or 0 visibility modifiers are present in the list.
+     *
+     * @param accessTransform the access transform to check against.
+     * @param modifiers the modifiers to check.
+     * @return true if the modifiers should be transformed, false otherwise.
+     */
+    public boolean shouldTransform(@NotNull final AccessTransform accessTransform,
+                                   @NotNull final List<J.Modifier> modifiers) {
+        final AccessChange accessChange = accessTransform.getAccess();
+        @Nullable final J.Modifier.Type accessTypeToKeep = RecipeHelper.typeFromAccessChange(accessChange);
+
+        boolean foundFinal = false;
+        for (final J.Modifier modifier : modifiers) {
+            if (!KNOWN_MUTABLE_TYPES.contains(modifier.getType())) {
+                continue;
+            }
+
+            if (modifier.getType() == J.Modifier.Type.Final) {
+                if (accessTransform.getFinal() == ModifierChange.REMOVE) {
+                    return true;
+                } else {
+                    foundFinal = true;
+                }
+                continue;
+            }
+
+            if (modifier.getType() != accessTypeToKeep) {
+                return true;
+            }
+        }
+
+        return !foundFinal && accessTransform.getFinal() == ModifierChange.ADD;
+    }
+
     /**
      * Transforms the modifiers passed in as the list to match the access transform goal passed to the method.
      * <p>

src/main/java/io/papermc/restamp/recipe/ClassATMutator.java

@@ -51,7 +51,7 @@ public J.ClassDeclaration visitClassDeclaration(@NotNull final J.ClassDeclaratio
                 if (transformerClass == null) return classDeclaration;
 
                 final AccessTransform accessTransform = transformerClass.get();
-                if (accessTransform.isEmpty()) return classDeclaration;
+                if (accessTransform.isEmpty() || !modifierTransformer.shouldTransform(accessTransform, classDeclaration.getModifiers())) return classDeclaration;
 
                 transformerClass.replace(AccessTransform.EMPTY); // Mark as consumed
 

src/main/java/io/papermc/restamp/recipe/FieldATMutator.java

@@ -64,7 +64,7 @@ public J.VariableDeclarations visitVariableDeclarations(@NotNull final J.Variabl
                     .filter(Objects::nonNull)
                     .reduce(AccessTransform::merge)
                     .orElse(AccessTransform.EMPTY);
-                if (accessTransformToApply.isEmpty()) return variableDeclarations;
+                if (accessTransformToApply.isEmpty() || !modifierTransformer.shouldTransform(accessTransformToApply, variableDeclarations.getModifiers())) return variableDeclarations;
 
                 // Compute and set new módifiers
                 final ModifierTransformationResult transformationResult = modifierTransformer.transformModifiers(

src/main/java/io/papermc/restamp/recipe/MethodATMutator.java

@@ -103,7 +103,7 @@ public J.MethodDeclaration visitMethodDeclaration(@NotNull final J.MethodDeclara
                 final AccessTransform accessTransform = transformerClass.replaceMethod(new MethodSignature(
                     atMethodName, new MethodDescriptor(parameterTypes, returnType)
                 ), AccessTransform.EMPTY);
-                if (accessTransform == null || accessTransform.isEmpty()) return methodDeclaration;
+                if (accessTransform == null || accessTransform.isEmpty() || !modifierTransformer.shouldTransform(accessTransform, methodDeclaration.getModifiers())) return methodDeclaration;
 
                 final TypeTree returnTypeExpression = methodDeclaration.getReturnTypeExpression();
                 final ModifierTransformationResult transformationResult = modifierTransformer.transformModifiers(

src/test/java/io/papermc/restamp/at/ModifierTransformerTest.java

@@ -10,21 +10,62 @@
 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) {
+        Assertions.assertEquals(expected, this.transformer.shouldTransform(transform, modifiers));
+    }
+
     @ParameterizedTest
     @ArgumentsSource(RestampFunctionTestHelper.CartesianVisibilityArgumentProvider.class)
     public void testModifyVisibilityTransformation(@NotNull final AccessTransform current,