From 8f952a48269a167401033af7823b976be902cbd8 Mon Sep 17 00:00:00 2001 From: Jake Potrebic Date: Sat, 30 Mar 2024 14:38:01 -0700 Subject: [PATCH] only run modifications if the modifers don't match --- .../restamp/RestampContextConfiguration.java | 2 +- .../restamp/recipe/ClassATMutator.java | 6 ++- .../restamp/recipe/FieldATMutator.java | 8 +++- .../restamp/recipe/MethodATMutator.java | 11 ++++- .../restamp/at/ModifierTransformerTest.java | 42 +++++++++++++++++++ 5 files changed, 63 insertions(+), 6 deletions(-) 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/recipe/ClassATMutator.java b/src/main/java/io/papermc/restamp/recipe/ClassATMutator.java index 1043155..189727d 100644 --- 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 index b257170..c58f29e 100644 --- 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 index 540be24..2f647a9 100644 --- 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 index d45b66b..32e64cb 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,45 @@ 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) { + 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,