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 scope to injected classes #248

Merged
merged 7 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
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,96 @@
/*
* Copyright 2023 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.migrate.javax;

import org.openrewrite.ExecutionContext;
import org.openrewrite.ScanningRecipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;

import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;

public class AddScopeToInjectedClass extends ScanningRecipe<Set<JavaType.FullyQualified>> {
private static final String JAVAX_INJECT_INJECT = "javax.inject.Inject";
private static final String JAVAX_ENTERPRISE_CONTEXT_DEPENDENT = "javax.enterprise.context.Dependent";
private static final Collection<String> TYPES_PROMPTING_SCOPE_ADDITION = Arrays.asList(JAVAX_INJECT_INJECT);

@Override
public String getDisplayName() {
return "Add scope annotation to injected classes";
}

@Override
public String getDescription() {
return "Finds member variables annotated with `@Inject' and applies `@Dependent` scope annotation to the variable's type.";
}

@Override
public Set<JavaType.FullyQualified> getInitialValue(ExecutionContext ctx) {
return new HashSet<>();
}

@Override
public TreeVisitor<?, ExecutionContext> getScanner(Set<JavaType.FullyQualified> injectedTypes) {
return new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext executionContext) {
J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, executionContext);
for (JavaType.Variable variable : cd.getType().getMembers()) {
if (variableTypeRequiresScope(variable)) {
injectedTypes.add((JavaType.FullyQualified) variable.getType());
}
}
return cd;
}

private boolean variableTypeRequiresScope(@Nullable JavaType.Variable memberVariable) {
if (memberVariable == null) {
return false;
}

for (JavaType.FullyQualified annotation : memberVariable.getAnnotations()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Detail: You can instead declare an AnnotationMatcher for javax.inject.Inject. That would be slightly more OpenRewrite idiomatic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea behind storing the annotation to look for in a Collection was that the recipe might be parameterized in future, allowing to search for more than one annotation at once. However this isn't happening at the moment so it's probably best to code for the now

Copy link
Contributor

Choose a reason for hiding this comment

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

Even then you could have a collection of AnnotationMatchers which you match in a loop.

Copy link
Contributor Author

@m-brophy m-brophy Jul 7, 2023

Choose a reason for hiding this comment

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

I'm trying to implement this now but I'm struggling a bit because JavaType.Variable getAnnotations() method returns a collection of FullyQualified and when I apply the matcher to that it doesn't compile because it expects J.Annotation. Is there a way to construct a J.Annotation from a FullyQualified? Or will it mean entirely rewriting to use cd.getAnnotations()?

Copy link
Contributor

Choose a reason for hiding this comment

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

AnnotationMatcher has a method called matchesAnnotationOrMetaAnnotation(JavaType.FullyQualified). That is probably what you are looking for, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (TYPES_PROMPTING_SCOPE_ADDITION.contains(annotation.getFullyQualifiedName())) {
return true;
}
}
return false;
}
};
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor(Set<JavaType.FullyQualified> injectedTypes) {
return new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.CompilationUnit visitCompilationUnit(J.CompilationUnit compilationUnit, ExecutionContext executionContext) {
knutwannheden marked this conversation as resolved.
Show resolved Hide resolved
J.CompilationUnit cu = super.visitCompilationUnit(compilationUnit, executionContext);
for (J.ClassDeclaration aClass : cu.getClasses()) {
if (injectedTypes.contains(aClass.getType())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that JavaType.Class does not implement hashCode() so you cannot really add these to a HashSet. This could be problematic when the recipe scheduler executes a recipe against a large repository, because then it breaks up the sources into "chunks", which each have their own set of JavaType objects (i.e. not referential equality).

For the same reason it isn't really a good idea to add JavaType objects to the recipe's accumulator in the first place. You should instead get the type's fully qualified name using JavaType.FullyQualified#getFullyQualifiedName() and add these strings to the set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return new AnnotateTypesVisitor(JAVAX_ENTERPRISE_CONTEXT_DEPENDENT)
.visitCompilationUnit(cu, injectedTypes);
}
}
return cu;
}
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright 2023 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.migrate.javax;

import org.openrewrite.java.AnnotationMatcher;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;

import java.util.Comparator;
import java.util.Set;

public class AnnotateTypesVisitor extends JavaIsoVisitor<Set<JavaType.FullyQualified>> {
private final String annotationToBeAdded;
private final AnnotationMatcher annotationMatcher;
private final JavaTemplate template;

public AnnotateTypesVisitor(String annotationToBeAdded) {
this.annotationToBeAdded = annotationToBeAdded;
String[] split = this.annotationToBeAdded.split("\\.");
String className = split[split.length - 1];
String packageName = this.annotationToBeAdded.substring(0, this.annotationToBeAdded.lastIndexOf("."));
this.annotationMatcher = new AnnotationMatcher("@" + this.annotationToBeAdded);
String interfaceAsString = String.format("package %s; public @interface %s {}", packageName, className);
this.template = JavaTemplate.builder("@" + className)
.imports(this.annotationToBeAdded)
.javaParser(JavaParser.fromJavaVersion().dependsOn(interfaceAsString))
.build();
}

@Override
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Set<JavaType.FullyQualified> injectedTypes) {
J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, injectedTypes);
if (injectedTypes.contains(TypeUtils.asFullyQualified(cd.getType()))
&& cd.getLeadingAnnotations().stream().noneMatch(annotationMatcher::matches)) {
maybeAddImport(annotationToBeAdded);
return template.apply(getCursor(), cd.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::getSimpleName)));
}
return cd;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
/*
* Copyright 2023 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.migrate.javax;

import org.openrewrite.DocumentExample;
import org.openrewrite.java.migrate.javax.AddScopeToInjectedClass;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.java.JavaParser;
import org.junit.jupiter.api.Test;

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

class AddScopeToInjectedClassTest implements RewriteTest {

@Override
public void defaults(RecipeSpec spec) {

spec.recipe(new AddScopeToInjectedClass());
spec.parser(JavaParser.fromJavaVersion().dependsOn("""
package javax.enterprise.context;
@Target({ElementType.Type})
@Retention(RetentionPolicy.RUNTIME)
public @interface Dependent {
}
""",
"""
package javax.inject;
@Target({ElementType.Type})
@Retention(RetentionPolicy.RUNTIME)
public @interface Inject {
}
"""));
}

@DocumentExample
@Test
void scopeRequired() {
rewriteRun(spec ->
java("""
package com.sample.service;

public class Bar {}

""",
"""
package com.sample.service;

import javax.enterprise.context.Dependent;

@Dependent
public class Bar {}

"""),

java("""
package com.sample;

import javax.inject.Inject;
import com.sample.service.Bar;

public class Foo{

@javax.inject.Inject
Bar service;
}
"""));
}


@Test
void noMemberVariableAnnotation() {
rewriteRun(spec ->
java("""
package com.sample.service;

public class Bar {}

"""),

java("""
package com.sample;

import com.sample.service.Bar;

public class Foo{

Bar service;
}
"""));
}

@Test
void nonInjectAnnotation() {
rewriteRun(spec ->
java("""
package com.sample.service;

public class Bar {}

"""),

java("""
package com.sample;

import com.sample.service.Bar;
import javax.inject.NotInject;

public class Foo{
@NotInject
Bar service;
}
"""),
java("""
package javax.inject;
@Target({ElementType.Type})
@Retention(RetentionPolicy.RUNTIME)
public @interface NotInject {
}
"""));
}


@Test
void scopeAnnotationAlreadyExists() {
rewriteRun(spec ->
java("""
package com.sample.service;

import javax.enterprise.context.Dependent;

@Dependent
public class Bar {}

"""),

java("""
package com.sample;

import javax.inject.Inject;
import com.sample.service.Bar;

public class Foo{

@javax.inject.Inject
Bar service;
}
"""));
}

}