diff --git a/src/main/java/io/papermc/restamp/RestampContextConfiguration.java b/src/main/java/io/papermc/restamp/RestampContextConfiguration.java index dd3fc81..c30090a 100644 --- 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/at/ModifierTransformer.java b/src/main/java/io/papermc/restamp/at/ModifierTransformer.java index b56d65c..9e66fe8 100644 --- a/src/main/java/io/papermc/restamp/at/ModifierTransformer.java +++ b/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. + *

+ * 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 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. *

diff --git a/src/main/java/io/papermc/restamp/recipe/ClassATMutator.java b/src/main/java/io/papermc/restamp/recipe/ClassATMutator.java index 1043155..6e76c95 100644 --- a/src/main/java/io/papermc/restamp/recipe/ClassATMutator.java +++ b/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 diff --git a/src/main/java/io/papermc/restamp/recipe/FieldATMutator.java b/src/main/java/io/papermc/restamp/recipe/FieldATMutator.java index b257170..69b01eb 100644 --- a/src/main/java/io/papermc/restamp/recipe/FieldATMutator.java +++ b/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( diff --git a/src/main/java/io/papermc/restamp/recipe/MethodATMutator.java b/src/main/java/io/papermc/restamp/recipe/MethodATMutator.java index 540be24..d5592ea 100644 --- a/src/main/java/io/papermc/restamp/recipe/MethodATMutator.java +++ b/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( diff --git a/src/test/java/io/papermc/restamp/at/ModifierTransformerTest.java b/src/test/java/io/papermc/restamp/at/ModifierTransformerTest.java index d45b66b..c0854de 100644 --- a/src/test/java/io/papermc/restamp/at/ModifierTransformerTest.java +++ b/src/test/java/io/papermc/restamp/at/ModifierTransformerTest.java @@ -10,7 +10,9 @@ 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; @@ -18,6 +20,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.stream.Stream; import static io.papermc.restamp.RestampFunctionTestHelper.modifierFrom; @@ -25,6 +28,44 @@ class ModifierTransformerTest { private final ModifierTransformer transformer = new ModifierTransformer(); + static Stream 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 modifiers, + final boolean expected) { + Assertions.assertEquals(expected, this.transformer.shouldTransform(transform, modifiers)); + } + @ParameterizedTest @ArgumentsSource(RestampFunctionTestHelper.CartesianVisibilityArgumentProvider.class) public void testModifyVisibilityTransformation(@NotNull final AccessTransform current,