diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index 8ff03d2a..8dbc6cb6 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -277,6 +277,11 @@ refaster-compiler ${project.version} + + ${project.groupId} + refaster-rule-benchmark-generator + ${project.version} + ${project.groupId} refaster-support @@ -285,6 +290,7 @@ -Xplugin:DocumentationGenerator -XoutputDirectory=${project.build.directory}/docs + -Xplugin:RefasterRuleBenchmarkGenerator -XoutputDirectory=${project.build.directory}/generated-test-sources/test-annotations diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/ReactorRulesBenchmarks.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/ReactorRulesBenchmarks.java index d425c2ca..2a891179 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/ReactorRulesBenchmarks.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/ReactorRulesBenchmarks.java @@ -58,6 +58,12 @@ public class ReactorRulesBenchmarks { Mono after(Optional optional, Mono mono) { return Mono.justOrEmpty(optional).switchIfEmpty(mono); } + + // XXX: Methods such as this one could perhaps be inferred in the common case. + @Benchmarked.OnResult + T subscribe(Mono mono) { + return mono.block(); + } } // XXX: Variations like this would be generated. diff --git a/error-prone-guidelines/pom.xml b/error-prone-guidelines/pom.xml index a283915f..fc6609cf 100644 --- a/error-prone-guidelines/pom.xml +++ b/error-prone-guidelines/pom.xml @@ -120,6 +120,7 @@ -Xplugin:DocumentationGenerator -XoutputDirectory=${project.build.directory}/docs + diff --git a/pom.xml b/pom.xml index 53996bac..5c28bda7 100644 --- a/pom.xml +++ b/pom.xml @@ -45,6 +45,7 @@ error-prone-guidelines error-prone-utils refaster-compiler + refaster-rule-benchmark-generator refaster-runner refaster-support refaster-test-support diff --git a/refaster-rule-benchmark-generator/pom.xml b/refaster-rule-benchmark-generator/pom.xml new file mode 100644 index 00000000..59b4935c --- /dev/null +++ b/refaster-rule-benchmark-generator/pom.xml @@ -0,0 +1,117 @@ + + + 4.0.0 + + + tech.picnic.error-prone-support + error-prone-support + 0.16.2-SNAPSHOT + + + refaster-rule-benchmark-generator + + Picnic :: Error Prone Support :: Refaster Rule Benchmark Generator + + Compiler plugin that derives JMH benchmarks from Refaster Rules. + https://error-prone.picnic.tech + + + + + ${groupId.error-prone} + error_prone_annotation + + + ${groupId.error-prone} + error_prone_annotations + provided + + + ${groupId.error-prone} + error_prone_check_api + + + ${groupId.error-prone} + error_prone_core + + provided + + + ${groupId.error-prone} + error_prone_test_helpers + test + + + ${project.groupId} + refaster-support + + test + + + com.fasterxml.jackson.core + jackson-annotations + + + com.fasterxml.jackson.core + jackson-databind + + + com.fasterxml.jackson.datatype + jackson-datatype-guava + + + com.fasterxml.jackson.module + jackson-module-parameter-names + + + com.google.auto + auto-common + + + com.google.auto.service + auto-service-annotations + provided + + + com.google.auto.value + auto-value-annotations + provided + + + com.google.guava + guava + + + io.projectreactor + reactor-core + provided + + + org.assertj + assertj-core + test + + + org.jspecify + jspecify + provided + + + org.junit.jupiter + junit-jupiter-api + test + + + + org.junit.jupiter + junit-jupiter-engine + test + + + org.junit.jupiter + junit-jupiter-params + test + + + diff --git a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterBenchmarkGenerator.java b/refaster-rule-benchmark-generator/src/main/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGenerator.java similarity index 82% rename from documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterBenchmarkGenerator.java rename to refaster-rule-benchmark-generator/src/main/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGenerator.java index a1c3d917..559e6491 100644 --- a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterBenchmarkGenerator.java +++ b/refaster-rule-benchmark-generator/src/main/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGenerator.java @@ -1,4 +1,4 @@ -package tech.picnic.errorprone.documentation; +package tech.picnic.errorprone.refaster.benchmark; import static com.google.common.base.Preconditions.checkArgument; @@ -13,16 +13,15 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; /** A compiler {@link Plugin} that generates JMH benchmarks for Refaster rules. */ -// XXX: Move logic to separate package and Maven module. // XXX: Review whether this can be an annotation processor instead. @AutoService(Plugin.class) -public final class RefasterBenchmarkGenerator implements Plugin { +public final class RefasterRuleBenchmarkGenerator implements Plugin { @VisibleForTesting static final String OUTPUT_DIRECTORY_FLAG = "-XoutputDirectory"; private static final Pattern OUTPUT_DIRECTORY_FLAG_PATTERN = Pattern.compile(Pattern.quote(OUTPUT_DIRECTORY_FLAG) + "=(.*)"); - /** Instantiates a new {@link RefasterBenchmarkGenerator} instance. */ - public RefasterBenchmarkGenerator() {} + /** Instantiates a new {@link RefasterRuleBenchmarkGenerator} instance. */ + public RefasterRuleBenchmarkGenerator() {} @Override public String getName() { @@ -34,9 +33,9 @@ public final class RefasterBenchmarkGenerator implements Plugin { checkArgument(args.length == 1, "Precisely one path must be provided"); // XXX: Drop all path logic: instead generate in the same package as the Refaster rule - // collection. + // collection. (But how do we then determine the base directory?) javacTask.addTaskListener( - new RefasterBenchmarkGeneratorTaskListener( + new RefasterRuleBenchmarkGeneratorTaskListener( ((BasicJavacTask) javacTask).getContext(), getOutputPath(args[0]))); } diff --git a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterBenchmarkGeneratorTaskListener.java b/refaster-rule-benchmark-generator/src/main/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTaskListener.java similarity index 56% rename from documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterBenchmarkGeneratorTaskListener.java rename to refaster-rule-benchmark-generator/src/main/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTaskListener.java index 12253aad..f9a0bcfa 100644 --- a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterBenchmarkGeneratorTaskListener.java +++ b/refaster-rule-benchmark-generator/src/main/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTaskListener.java @@ -1,10 +1,18 @@ -package tech.picnic.errorprone.documentation; +package tech.picnic.errorprone.refaster.benchmark; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.hasAnnotation; + +import com.google.common.collect.ImmutableList; import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.Matchers; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; import com.sun.source.tree.ClassTree; import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.MethodTree; import com.sun.source.tree.Tree; import com.sun.source.util.TaskEvent; import com.sun.source.util.TaskEvent.Kind; @@ -14,19 +22,22 @@ import com.sun.source.util.TreePathScanner; import com.sun.tools.javac.api.JavacTrees; import com.sun.tools.javac.util.Context; import java.io.IOException; -import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import javax.tools.JavaFileObject; import org.jspecify.annotations.Nullable; // XXX: Document. -final class RefasterBenchmarkGeneratorTaskListener implements TaskListener { +final class RefasterRuleBenchmarkGeneratorTaskListener implements TaskListener { + private static final Matcher IS_TEMPLATE = + anyOf(hasAnnotation(BeforeTemplate.class), hasAnnotation(AfterTemplate.class)); + private static final Matcher IS_BENCHMARKED = + Matchers.hasAnnotation("tech.picnic.errorprone.refaster.annotation.Benchmarked"); + private final Context context; private final Path outputPath; - RefasterBenchmarkGeneratorTaskListener(Context context, Path outputPath) { + RefasterRuleBenchmarkGeneratorTaskListener(Context context, Path outputPath) { this.context = context; this.outputPath = outputPath; } @@ -55,18 +66,15 @@ final class RefasterBenchmarkGeneratorTaskListener implements TaskListener { VisitorState.createForUtilityPurposes(context) .withPath(new TreePath(new TreePath(compilationUnit), classTree)); - // XXX: Make static. - Matcher isBenchmarked = - Matchers.hasAnnotation("tech.picnic.errorprone.refaster.annotation.Benchmarked"); - new TreePathScanner<@Nullable Void, Boolean>() { @Override public @Nullable Void visitClass(ClassTree classTree, Boolean doBenchmark) { // XXX: Validate that `@Benchmarked` is only placed in contexts with at least one Refaster // rule. - boolean inspectClass = doBenchmark || isBenchmarked.matches(classTree, state); + boolean inspectClass = doBenchmark || IS_BENCHMARKED.matches(classTree, state); if (inspectClass) { + System.out.println(handle(classTree, state)); // XXX: If this class has a `@BeforeTemplate` method, generate a benchmark for it. } @@ -75,6 +83,29 @@ final class RefasterBenchmarkGeneratorTaskListener implements TaskListener { }.scan(compilationUnit, false); } + // XXX: Name? Scope? + private static Rule handle(ClassTree classTree, VisitorState state) { + ImmutableList methods = + classTree.getMembers().stream() + .filter(m -> IS_TEMPLATE.matches(m, state)) + .map(m -> process((MethodTree) m, state)) + .collect(toImmutableList()); + + Rule rule = new Rule(classTree, methods); + return rule; + } + + private static Rule.Method process(MethodTree methodTree, VisitorState state) { + // XXX: Initially, disallow `Refaster.x` usages. + // XXX: Initially, disallow references to `@Placeholder` methods. + return new Rule.Method(methodTree); + } + + // XXX: Move types down. + record Rule(ClassTree tree, ImmutableList methods) { + record Method(MethodTree tree) {} + } + private void createOutputDirectory() { try { Files.createDirectories(outputPath); @@ -83,12 +114,4 @@ final class RefasterBenchmarkGeneratorTaskListener implements TaskListener { String.format("Error while creating directory with path '%s'", outputPath), e); } } - - private void writeToFile(String identifier, String className, T data) { - Json.write(outputPath.resolve(String.format("%s-%s.json", identifier, className)), data); - } - - private static String getSimpleClassName(URI path) { - return Paths.get(path).getFileName().toString().replace(".java", ""); - } } diff --git a/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/Compilation.java b/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/Compilation.java new file mode 100644 index 00000000..bbf1a487 --- /dev/null +++ b/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/Compilation.java @@ -0,0 +1,72 @@ +package tech.picnic.errorprone.refaster.benchmark; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.errorprone.FileManagers; +import com.google.errorprone.FileObjects; +import com.sun.tools.javac.api.JavacTaskImpl; +import com.sun.tools.javac.api.JavacTool; +import com.sun.tools.javac.file.JavacFileManager; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import javax.tools.Diagnostic; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileObject; + +// XXX: This code is a near-duplicate of the identically named class in `documentation-support`. +public final class Compilation { + private Compilation() {} + + public static void compileWithRefasterRuleBenchmarkGenerator( + Path outputDirectory, String path, String... lines) { + compileWithRefasterRuleBenchmarkGenerator( + outputDirectory.toAbsolutePath().toString(), path, lines); + } + + public static void compileWithRefasterRuleBenchmarkGenerator( + String outputDirectory, String path, String... lines) { + /* + * The compiler options specified here largely match those used by Error Prone's + * `CompilationTestHelper`. A key difference is the stricter linting configuration. When + * compiling using JDK 21+, these lint options also require that certain JDK modules are + * explicitly exported. + */ + compile( + ImmutableList.of( + "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + "-encoding", + "UTF-8", + "-parameters", + "-proc:none", + "-Werror", + "-Xlint:all,-serial", + "-Xplugin:RefasterRuleBenchmarkGenerator -XoutputDirectory=" + outputDirectory, + "-XDdev", + "-XDcompilePolicy=simple"), + FileObjects.forSourceLines(path, lines)); + } + + private static void compile(ImmutableList options, JavaFileObject javaFileObject) { + JavacFileManager javacFileManager = FileManagers.testFileManager(); + JavaCompiler compiler = JavacTool.create(); + + List> diagnostics = new ArrayList<>(); + JavacTaskImpl task = + (JavacTaskImpl) + compiler.getTask( + null, + javacFileManager, + diagnostics::add, + options, + ImmutableList.of(), + ImmutableList.of(javaFileObject)); + + Boolean result = task.call(); + assertThat(diagnostics).isEmpty(); + assertThat(result).isTrue(); + } +} diff --git a/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTaskListenerTest.java b/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTaskListenerTest.java new file mode 100644 index 00000000..4015d9e1 --- /dev/null +++ b/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTaskListenerTest.java @@ -0,0 +1,41 @@ +package tech.picnic.errorprone.refaster.benchmark; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.file.Path; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +// XXX: Add relevant tests also present in `DocumentationGeneratorTaskListenerTest`. +// XXX: Test package placement. +final class RefasterRuleBenchmarkGeneratorTaskListenerTest { + // XXX: Rename! + @Test + void xxx(@TempDir Path outputDirectory) { + Compilation.compileWithRefasterRuleBenchmarkGenerator( + outputDirectory, + "TestCheckerWithoutAnnotation.java", + """ + import com.google.errorprone.refaster.annotation.AfterTemplate; + import com.google.errorprone.refaster.annotation.BeforeTemplate; + import java.util.Optional; + import reactor.core.publisher.Mono; + import tech.picnic.errorprone.refaster.annotation.Benchmarked; + + @Benchmarked + final class MonoFromOptionalSwitchIfEmpty { + @BeforeTemplate + Mono before(Optional optional, Mono mono) { + return optional.map(Mono::just).orElse(mono); + } + + @AfterTemplate + Mono after(Optional optional, Mono mono) { + return Mono.justOrEmpty(optional).switchIfEmpty(mono); + } + } + """); + + assertThat(outputDirectory.toAbsolutePath()).isEmptyDirectory(); + } +} diff --git a/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTest.java b/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTest.java new file mode 100644 index 00000000..ae93a8df --- /dev/null +++ b/refaster-rule-benchmark-generator/src/test/java/tech/picnic/errorprone/refaster/benchmark/RefasterRuleBenchmarkGeneratorTest.java @@ -0,0 +1,5 @@ +package tech.picnic.errorprone.refaster.benchmark; + +final class RefasterRuleBenchmarkGeneratorTest { + // XXX: TBD. See `DocumentationGeneratorTest`. +} diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/Benchmarked.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/Benchmarked.java index 4f872f12..4ca612ea 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/Benchmarked.java +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/Benchmarked.java @@ -18,10 +18,14 @@ public @interface Benchmarked { // - Specify warmup and measurement iterations. // - Specify output time unit. // - Value generation hints.f - // Once configuration is supported, annotations on nested classes should override the configuration specified by outer classes. + // Once configuration is supported, annotations on nested classes should override the + // configuration specified by outer classes. // XXX: Explain use. Allow restriction by name? - public @interface Param { + @Target(ElementType.FIELD) + @interface Param {} - } + // XXX: Explain use + @Target(ElementType.METHOD) + @interface OnResult {} }