Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recipe to replace Annotation #4217

Merged
merged 2 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.java;

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.openrewrite.*;
import org.openrewrite.java.search.UsesType;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;

@Value
@EqualsAndHashCode(callSuper = false)
public class ReplaceAnnotation extends Recipe {
@Option(displayName = "Annotation pattern to replace",
description = "An annotation pattern, expressed as a method pattern to replace.",
MBoegers marked this conversation as resolved.
Show resolved Hide resolved
example = "@org.jetbrains.annotations.NotNull(\"Test\")")
String annotationPatternToReplace;
@Option(displayName = "Annotation template to insert",
description = "An annotation template to add instead of original one, will be parsed with `JavaTemplate`.",
example = "@NonNull")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense with an example that includes a parameter, to make it clear that this is supported. Additionally, since a parameter could require a reference to a Java class, I think it would make sense for the JavaTemplate here to be required to be fully context-free and thus not rely on any imports being registered with the JavaTemplate. As a result I imagine the annotationFQN parameter can be dropped (didn't look at the code). Then we can apply the ShortenFullyQualifiedNames visitor to get imports added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried executing ShortenFullyQualifiedNames with doAfterVisit(new ShortenFullyQualifiedTypeReferences().getVisitor()); earlier, but this didn't make the expected changes. I'll try again and post a gist if not working.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the class declares a static utility method. We should find some uses of it in rewrite-static-analysis.

Copy link
Contributor Author

@MBoegers MBoegers May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With ShortenFullyQualifiedTypeReferences#modifyOnly the recipe looks better now, thanks. But now the tests are failing because ShortenFullyQualifiedTypeReferences handles annotations unexpected.

It seems to only shorten Annotations of the pattern ´packagename.@annotation´ (see ShortenFullyQualifiedTypeReferencesTest#typeFullyQualifiedAnnotatedField and ShortenFullyQualifiedTypeReferencesTest#annotatedFieldAccess).
In the ShortenFullyQualifiedTypeReferences fails if the annotation has the pattern @packagename.TypeName
According to the Java Language Spec 9.7.1 Annotations in FQN format have the pattern @packagename.TypeName.
I think there might be a bug in ShortenFullyQualifiedTypeReferences or am I missing something?

See https://gist.github.com/MBoegers/9c805b9caa35144466d9f380ddbb40bd for a reproducing test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite possible that we have a bug there. I will try to investigate next week.

String annotationTemplateToInsert;
@Option(displayName = "Type of inserted Annotation",
description = "The fully qualified class name of the annotation to insert.",
example = "lombok.NonNull")
String annotationFQN;
@Option(displayName = "Templates Artifact id",
description = "The Maven artifactId to load the inserted annotations type from, defaults to JDK internals.",
MBoegers marked this conversation as resolved.
Show resolved Hide resolved
example = "lombok",
required = false)
String artifactId;

@Override
public String getDisplayName() {
return "Replace Annotation";
}

@Override
public String getDescription() {
return "Replace an Annotation with another one if the annotation pattern matches. " +
"Only fixed parameters can be set in the replacement.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(new UsesType<>(annotationFQN, false),
new ReplaceAnnotationVisitor(
new AnnotationMatcher(annotationPatternToReplace),
JavaTemplate.builder(annotationTemplateToInsert)
.imports(annotationFQN)
.javaParser(JavaParser.fromJavaVersion().logCompilationWarningsAndErrors(true)
MBoegers marked this conversation as resolved.
Show resolved Hide resolved
.classpath(artifactId))
.build()));
}

public static class ReplaceAnnotationVisitor extends JavaIsoVisitor<ExecutionContext> {
private final AnnotationMatcher matcher;
private final JavaTemplate replacement;

public ReplaceAnnotationVisitor(AnnotationMatcher annotationMatcher, JavaTemplate replacement) {
super();
this.matcher = annotationMatcher;
this.replacement = replacement;
}

@Override
public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext ctx) {
annotation = super.visitAnnotation(annotation, ctx);

boolean replaceAnnotation = matcher.matches(annotation);
if (replaceAnnotation) {
JavaType replacedAnnotationType = annotation.getType();
annotation = replacement.apply(getCursor(), annotation.getCoordinates().replace());
JavaType insertedAnnotationType = annotation.getType();
maybeRemoveImport(TypeUtils.asFullyQualified(replacedAnnotationType));
maybeAddImport(TypeUtils.asFullyQualified(insertedAnnotationType).getFullyQualifiedName(), false);
}

return annotation;
}
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.java;

import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.java;

class ReplaceAnnotationTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.parser(JavaParser.fromJavaVersion()
.logCompilationWarningsAndErrors(false)
.classpath("lombok", "annotations"));
}

@Nested
class OnMatch {
@Test
void matchNoPrams() {
rewriteRun(
spec -> spec.recipe(new ReplaceAnnotation("@org.jetbrains.annotations.NotNull",
"@NonNull", "lombok.NonNull", "lombok")),
java(
"""
import org.jetbrains.annotations.NotNull;

class A {
@NotNull
String testMethod() {}
}
""", """
import lombok.NonNull;

class A {
@NonNull
String testMethod() {}
}
"""
));
}

@Test
@DocumentExample
void matchWithPrams() {
rewriteRun(
spec -> spec.recipe(new ReplaceAnnotation("@org.jetbrains.annotations.NotNull(\"Test\")",
"@NonNull", "lombok.NonNull", "lombok")),
java(
"""
import org.jetbrains.annotations.NotNull;

class A {
@NotNull("Test")
String testMethod() {}
}
""", """
import lombok.NonNull;

class A {
@NonNull
String testMethod() {}
}
"""
));
}

@Test
void insertWithParams() {
rewriteRun(
spec -> spec.recipe(new ReplaceAnnotation("@lombok.NonNull",
"@NotNull(\"Test\")", "org.jetbrains.annotations.NotNull", "annotations")),
java(
"""
import lombok.NonNull;

class A {
@NonNull
String testMethod() {}
}
""", """
import org.jetbrains.annotations.NotNull;

class A {
@NotNull("Test")
String testMethod() {}
}
"""
));
}
}

@Nested
class NoMatch {
@Test
void noMatchOtherType() {
rewriteRun(
spec -> spec.recipe(new ReplaceAnnotation("@org.jetbrains.annotations.NotNull",
"@NonNull", "lombok.NonNull", "lombok")),
java(
"""
import org.jetbrains.annotations.Nullable;

class A {
@Nullable("Test")
String testMethod() {}
}
"""
));
}

@Test
void noMatchParameter() {
rewriteRun(
spec -> spec.recipe(new ReplaceAnnotation("@org.jetbrains.annotations.NotNull(\"Test\")",
"@NonNull", "lombok.NonNull", "lombok")),
java(
"""
import org.jetbrains.annotations.Nullable;

class A {
@Nullable("Other")
String testMethod() {}
}
"""
));
}
}
}
Loading