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

Support Arrays and ignore if statements #56

Merged
merged 1 commit into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -494,15 +494,9 @@ private String recipeDescriptor(JCTree.JCClassDecl classDecl, String defaultDisp

private void maybeRemoveImports(Map<JCTree.JCMethodDecl, Set<String>> importsByTemplate, StringBuilder recipe, JCTree.JCMethodDecl beforeTemplate, JCTree.JCMethodDecl afterTemplate) {
Set<String> beforeImports = getBeforeImportsAsStrings(importsByTemplate, beforeTemplate);
Set<String> afterImports = getImportsAsStrings(importsByTemplate, afterTemplate);
for (String anImport : beforeImports) {
if (anImport.startsWith("java.lang.")) {
continue;
}
if (beforeImports.contains(anImport) && !afterImports.contains(anImport)) {
recipe.append(" maybeRemoveImport(\"").append(anImport).append("\");\n");
}
}
beforeImports.removeAll(getImportsAsStrings(importsByTemplate, afterTemplate));
beforeImports.removeIf(i -> i.startsWith("java.lang."));
beforeImports.forEach(anImport -> recipe.append(" maybeRemoveImport(\"").append(anImport).append("\");\n"));
}

private Set<String> getBeforeImportsAsStrings(Map<JCTree.JCMethodDecl, Set<String>> importsByTemplate, JCTree.JCMethodDecl templateMethod) {
Expand Down Expand Up @@ -663,7 +657,7 @@ private String toLambda(JCTree.JCMethodDecl method) {

StringJoiner joiner = new StringJoiner(", ", "(", ")");
for (JCTree.JCVariableDecl parameter : method.getParameters()) {
String paramType = parameter.getType().type.tsym.getQualifiedName().toString();
String paramType = parameter.getType().type.toString();
if (!getBoxedPrimitive(paramType).equals(paramType)) {
paramType = "@Primitive " + getBoxedPrimitive(paramType);
} else if (paramType.startsWith("java.lang.")) {
Expand Down Expand Up @@ -726,18 +720,17 @@ private TemplateDescriptor validate(Context context, JCCompilationUnit cu) {
return null;
}

boolean valid = true;
for (JCTree member : classDecl.getMembers()) {
if (member instanceof JCTree.JCMethodDecl && !beforeTemplates.contains(member) && member != afterTemplate) {
for (JCTree.JCAnnotation annotation : getTemplateAnnotations(((JCTree.JCMethodDecl) member), UNSUPPORTED_ANNOTATIONS::contains)) {
printNoteOnce("@" + annotation.annotationType + " is currently not supported", classDecl.sym);
valid = false;
return null;
}
}
}

// resolve so that we can inspect the template body
valid &= resolve(context, cu);
boolean valid = resolve(context, cu);
if (valid) {
for (JCTree.JCMethodDecl template : beforeTemplates) {
valid &= validateTemplateMethod(template);
Expand All @@ -748,27 +741,29 @@ private TemplateDescriptor validate(Context context, JCCompilationUnit cu) {
}

private boolean validateTemplateMethod(JCTree.JCMethodDecl template) {
// TODO Additional Refaster features https://github.com/openrewrite/rewrite-templating/issues/47
boolean valid = true;
for (JCTree.JCAnnotation annotation : getTemplateAnnotations(template, UNSUPPORTED_ANNOTATIONS::contains)) {
printNoteOnce("@" + annotation.annotationType + " is currently not supported", classDecl.sym);
valid = false;
return false;
}
for (JCTree.JCVariableDecl parameter : template.getParameters()) {
for (JCTree.JCAnnotation annotation : getTemplateAnnotations(parameter, UNSUPPORTED_ANNOTATIONS::contains)) {
printNoteOnce("@" + annotation.annotationType + " is currently not supported", classDecl.sym);
valid = false;
return false;
}
if (parameter.vartype instanceof ParameterizedTypeTree || parameter.vartype.type instanceof Type.TypeVar) {
printNoteOnce("Generics are currently not supported", classDecl.sym);
valid = false;
return false;
}
}
if (template.restype instanceof ParameterizedTypeTree || template.restype.type instanceof Type.TypeVar) {
printNoteOnce("Generics are currently not supported", classDecl.sym);
valid = false;
return false;
}
if (template.body.stats.get(0) instanceof JCTree.JCIf) {
printNoteOnce("If statements are currently not supported", classDecl.sym);
return false;
}
valid &= new TreeScanner() {
return new TreeScanner() {
boolean valid = true;

boolean validate(JCTree tree) {
Expand All @@ -778,14 +773,14 @@ boolean validate(JCTree tree) {

@Override
public void visitIdent(JCTree.JCIdent jcIdent) {
if (jcIdent.sym != null
if (valid
&& jcIdent.sym != null
&& jcIdent.sym.packge().getQualifiedName().contentEquals("com.google.errorprone.refaster")) {
printNoteOnce(jcIdent.type.tsym.getQualifiedName() + " is currently not supported", classDecl.sym);
valid = false;
}
}
}.validate(template.getBody());
return valid;
}

public void beforeTemplate(JCTree.JCMethodDecl method) {
Expand All @@ -811,17 +806,16 @@ private boolean resolve(Context context, JCCompilationUnit cu) {
}
return true;
}

}

private final Set<String> printedMessages = new HashSet<>();
private final Map<String, Integer> printedMessages = new TreeMap<>();

/**
* @param message The message to print
* @param symbol The symbol to attach the message to; printed as clickable link to file
*/
private void printNoteOnce(String message, Symbol.ClassSymbol symbol) {
if (printedMessages.add(message)) {
if (printedMessages.compute(message, (k, v) -> v == null ? 1 : v + 1) == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change? Is this somehow more performant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to allow one to inspect the number of times recipes are excluded, as used in this #47 (comment)

Didn't want to make that part of the standard output though; I think once and linked to the source is enough, as opposed to just a summary of how often there are exclusions not tied to a particular instance. Having this counter allows us to easily print it as a one off though.

processingEnv.getMessager().printMessage(Kind.NOTE, message, symbol);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public void visitIdent(JCTree.JCIdent ident) {
for (JCTree.JCVariableDecl parameter : parameters) {
if (parameter.type.tsym instanceof Symbol.ClassSymbol) {
String paramType = parameter.type.tsym.getQualifiedName().toString();
if (!paramType.startsWith("java.lang")) {
if (!paramType.startsWith("java.lang") && !"Array".equals(paramType)) {
out.write("import " + paramType + ";\n");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.lang.reflect.Proxy;

public abstract class TypeAwareProcessor extends AbstractProcessor {
protected ProcessingEnvironment processingEnv;
protected JavacProcessingEnvironment javacProcessingEnv;
protected Trees trees;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
class RefasterTemplateProcessorTest {
@ParameterizedTest
@ValueSource(strings = {
"Arrays",
"MethodThrows",
"NestedPreconditions",
"ParameterReuse",
Expand Down
31 changes: 31 additions & 0 deletions src/test/resources/refaster/Arrays.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 foo;

import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;

public class Arrays {
@BeforeTemplate
String before(String[] strings) {
return String.join(", ", strings);
}

@AfterTemplate
String after(String[] strings) {
return String.join(":", strings);
}
}
79 changes: 79 additions & 0 deletions src/test/resources/refaster/ArraysRecipe.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* 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 foo;

import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.lang.NonNullApi;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.search.*;
import org.openrewrite.java.template.Primitive;
import org.openrewrite.java.template.Semantics;
import org.openrewrite.java.template.function.*;
import org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor;
import org.openrewrite.java.tree.*;

import java.util.*;

import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.*;

@SuppressWarnings("all")
@NonNullApi
public class ArraysRecipe extends Recipe {

public ArraysRecipe() {
}

@Override
public String getDisplayName() {
return "Refaster template `Arrays`";
}

@Override
public String getDescription() {
return "Recipe created for the following Refaster template:\n```java\npublic class Arrays {\n \n @BeforeTemplate()\n String before(String[] strings) {\n return String.join(\", \", strings);\n }\n \n @AfterTemplate()\n String after(String[] strings) {\n return String.join(\":\", strings);\n }\n}\n```\n.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
final JavaTemplate before = Semantics.expression(this, "before", (String[] strings) -> String.join(", ", strings)).build();
final JavaTemplate after = Semantics.expression(this, "after", (String[] strings) -> String.join(":", strings)).build();

@Override
public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
JavaTemplate.Matcher matcher;
if ((matcher = before.matcher(getCursor())).find()) {
return embed(
after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)),
getCursor(),
ctx,
SHORTEN_NAMES
);
}
return super.visitMethodInvocation(elem, ctx);
}

};
return Preconditions.check(
new UsesMethod<>("java.lang.String join(..)"),
javaVisitor
);
}
}