Skip to content

Commit

Permalink
Skip method references and lambdas in body (#72)
Browse files Browse the repository at this point in the history 
* Skip method references and lambdas in body

* Skip after the first invalid template is found

* Sort reported messages
  • Loading branch information
@timtebeek
timtebeek authored Feb 4, 2024
1 parent efe3c56 commit b8b8cd2
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 21 deletions.
9 changes: 5 additions & 4 deletions build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,15 @@ configure<PublishingExtension> {
suppressPomMetadataWarningsFor("runtimeElements")

pom.withXml {
(asElement().getElementsByTagName("dependencies").item(0) as org.w3c.dom.Element?)?.let { dependencies ->
(asElement().getElementsByTagName("dependencies")
.item(0) as org.w3c.dom.Element?)?.let { dependencies ->
dependencies.getElementsByTagName("dependency").let { dependencyList ->
var i = 0
var length = dependencyList.length
while (i < length) {
(dependencyList.item(i) as org.w3c.dom.Element).let { dependency ->
if ((dependency.getElementsByTagName("scope")
.item(0) as org.w3c.dom.Element).textContent == "provided"
.item(0) as org.w3c.dom.Element).textContent == "provided"
) {
dependencies.removeChild(dependency)
i--
Expand All @@ -156,12 +157,12 @@ configure<PublishingExtension> {
val signingKey: String? by project
val signingPassword: String? by project
val requireSigning = project.hasProperty("forceSigning") || project.hasProperty("releasing")
if(signingKey != null && signingPassword != null) {
if (signingKey != null && signingPassword != null) {
signing {
isRequired = requireSigning
useInMemoryPgpKeys(signingKey, signingPassword)
sign(publishing.publications["nebula"])
}
} else if(requireSigning) {
} else if (requireSigning) {
throw RuntimeException("Artifact signing is required, but signingKey and/or signingPassword are null")
}
19 changes: 17 additions & 2 deletions src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
maybeGenerateTemplateSources(jcCompilationUnit);
}
}

// Inform how many rules were skipped and why; useful for debugging, but not enabled by default
//printedMessages.entrySet().stream().sorted(Map.Entry.comparingByValue())
// .forEach(entry -> processingEnv.getMessager().printMessage(Kind.NOTE, entry.toString()));

// Give other annotation processors a chance to process the same annotations, for dual use of Refaster templates
return false;
}
Expand Down Expand Up @@ -750,9 +755,9 @@ private TemplateDescriptor validate(Context context, JCCompilationUnit cu) {
boolean valid = resolve(context, cu);
if (valid) {
for (JCTree.JCMethodDecl template : beforeTemplates) {
valid &= validateTemplateMethod(template);
valid = valid && validateTemplateMethod(template);
}
valid &= validateTemplateMethod(afterTemplate);
valid = valid && validateTemplateMethod(afterTemplate);
}
return valid ? this : null;
}
Expand Down Expand Up @@ -784,6 +789,16 @@ private boolean validateTemplateMethod(JCTree.JCMethodDecl template) {
printNoteOnce("If statements are currently not supported", classDecl.sym);
return false;
}
if (template.body.stats.get(0) instanceof JCTree.JCReturn) {
JCTree.JCExpression expr = ((JCTree.JCReturn) template.body.stats.get(0)).expr;
if (expr instanceof JCTree.JCLambda) {
printNoteOnce("Lambdas are currently not supported", classDecl.sym);
return false;
} else if (expr instanceof JCTree.JCMemberReference) {
printNoteOnce("Method references are currently not supported", classDecl.sym);
return false;
}
}
return new TreeScanner() {
boolean valid = true;

Expand Down
35 changes: 27 additions & 8 deletions src/test/java/org/openrewrite/java/template/RefasterTemplateProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.testing.compile.Compilation;
import com.google.testing.compile.JavaFileObjects;
import org.intellij.lang.annotations.Language;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.openrewrite.java.template.processor.RefasterTemplateProcessor;
import org.openrewrite.java.template.processor.TypeAwareProcessor;

import javax.tools.JavaFileObject;
import java.io.File;
import java.net.URL;
import java.util.Arrays;
Expand All @@ -45,7 +47,7 @@ class RefasterTemplateProcessorTest {
"SimplifyBooleans",
})
void generateRecipe(String recipeName) {
Compilation compilation = compile("refaster/" + recipeName + ".java");
Compilation compilation = compileResource("refaster/" + recipeName + ".java");
assertThat(compilation).succeeded();
assertThat(compilation).hadNoteCount(0);
assertThat(compilation)
Expand All @@ -55,7 +57,7 @@ void generateRecipe(String recipeName) {

@Test
void generateRecipeInDefaultPackage() {
Compilation compilation = compile("refaster/UnnamedPackage.java");
Compilation compilation = compileResource("refaster/UnnamedPackage.java");
assertThat(compilation).succeeded();
assertThat(compilation).hadNoteCount(0);
assertThat(compilation)
Expand All @@ -69,7 +71,7 @@ void generateRecipeInDefaultPackage() {
"RefasterAnyOf",
})
void skipRecipeGeneration(String recipeName) {
Compilation compilation = compile("refaster/" + recipeName + ".java");
Compilation compilation = compileResource("refaster/" + recipeName + ".java");
assertThat(compilation).succeeded();
assertEquals(0, compilation.generatedSourceFiles().size(), "Should not generate recipe for " + recipeName);
}
Expand All @@ -85,20 +87,37 @@ void skipRecipeGeneration(String recipeName) {
"SimplifyTernary",
})
void nestedRecipes(String recipeName) {
Compilation compilation = compile("refaster/" + recipeName + ".java");
Compilation compilation = compileResource("refaster/" + recipeName + ".java");
assertThat(compilation).succeeded();
assertThat(compilation).hadNoteCount(0);
assertThat(compilation) // Recipes (plural)
.generatedSourceFile("foo/" + recipeName + "Recipes")
.hasSourceEquivalentTo(JavaFileObjects.forResource("refaster/" + recipeName + "Recipes.java"));
}

private static Compilation compile(String resourceName) {
return compile(resourceName, new RefasterTemplateProcessor());
@Test
void stringIsEmptyPredicate() {
Compilation compilation = compileResource("refaster/StringIsEmptyPredicate.java");
assertThat(compilation).succeeded();
assertThat(compilation).hadNoteCount(1);
assertThat(compilation).hadNoteContaining("Lambdas are currently not supported");
assertEquals(0, compilation.generatedSourceFiles().size(), "Not yet supported");
}

private static Compilation compileResource(String resourceName) {
return compileResource(resourceName, new RefasterTemplateProcessor());
}

static Compilation compile(String resourceName, TypeAwareProcessor processor) {
static Compilation compileResource(String resourceName, TypeAwareProcessor processor) {
// As per https://github.com/google/compile-testing/blob/v0.21.0/src/main/java/com/google/testing/compile/package-info.java#L53-L55
return compileResource(JavaFileObjects.forResource(resourceName), processor);
}

static Compilation compileSource(String fqn, @Language("java") String source, TypeAwareProcessor processor) {
return compileResource(JavaFileObjects.forSourceString(fqn, source), processor);
}

static Compilation compileResource(JavaFileObject javaFileObject, TypeAwareProcessor processor) {
return javac()
.withProcessors(processor)
.withClasspath(Arrays.asList(
Expand All @@ -110,7 +129,7 @@ static Compilation compile(String resourceName, TypeAwareProcessor processor) {
fileForClass(org.slf4j.Logger.class),
fileForClass(Primitive.class)
))
.compile(JavaFileObjects.forResource(resourceName));
.compile(javaFileObject);
}

// As per https://github.com/google/auto/blob/auto-value-1.10.2/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java#L99
Expand Down
14 changes: 7 additions & 7 deletions src/test/java/org/openrewrite/java/template/TemplateProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.openrewrite.java.template.processor.TemplateProcessor;

import static com.google.testing.compile.CompilationSubject.assertThat;
import static org.openrewrite.java.template.RefasterTemplateProcessorTest.compile;
import static org.openrewrite.java.template.RefasterTemplateProcessorTest.compileResource;

class TemplateProcessorTest {
@ParameterizedTest
Expand All @@ -35,7 +35,7 @@ class TemplateProcessorTest {
})
void qualification(String qualifier) {
// As per https://github.com/google/compile-testing/blob/v0.21.0/src/main/java/com/google/testing/compile/package-info.java#L53-L55
Compilation compilation = compile("template/ShouldAddClasspathRecipes.java", new TemplateProcessor());
Compilation compilation = compileResource("template/ShouldAddClasspathRecipes.java", new TemplateProcessor());
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("foo/ShouldAddClasspathRecipes$" + qualifier + "Recipe$1_before")
Expand All @@ -47,7 +47,7 @@ void qualification(String qualifier) {

@Test
void parameterReuse() {
Compilation compilation = compile("template/ParameterReuseRecipe.java", new TemplateProcessor());
Compilation compilation = compileResource("template/ParameterReuseRecipe.java", new TemplateProcessor());
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("foo/ParameterReuseRecipe$1_before")
Expand All @@ -56,7 +56,7 @@ void parameterReuse() {

@Test
void parserClasspath() {
Compilation compilation = compile("template/LoggerRecipe.java", new TemplateProcessor());
Compilation compilation = compileResource("template/LoggerRecipe.java", new TemplateProcessor());
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("template/LoggerRecipe$1_logger")
Expand All @@ -68,7 +68,7 @@ void parserClasspath() {

@Test
void generics() {
Compilation compilation = compile("template/Generics.java", new TemplateProcessor());
Compilation compilation = compileResource("template/Generics.java", new TemplateProcessor());
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("template/Generics$1_before")
Expand All @@ -80,7 +80,7 @@ void generics() {

@Test
void throwNew() {
Compilation compilation = compile("template/ThrowNewRecipe.java", new TemplateProcessor());
Compilation compilation = compileResource("template/ThrowNewRecipe.java", new TemplateProcessor());
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("template/ThrowNewRecipe$1_template")
Expand All @@ -89,7 +89,7 @@ void throwNew() {

@Test
void unnamedPackage() {
Compilation compilation = compile("template/UnnamedPackage.java", new TemplateProcessor());
Compilation compilation = compileResource("template/UnnamedPackage.java", new TemplateProcessor());
assertThat(compilation).succeeded();
assertThat(compilation)
.generatedSourceFile("UnnamedPackage$1_message")
Expand Down
33 changes: 33 additions & 0 deletions src/test/resources/refaster/StringIsEmptyPredicate.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* 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 refaster;

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

import java.util.function.Predicate;

class StringIsEmptyPredicate {
@BeforeTemplate
Predicate<String> before() {
return s -> s.isEmpty();
}

@AfterTemplate
Predicate<String> after() {
return String::isEmpty;
}
}

0 comments on commit b8b8cd2

Please sign in to comment.