From 8c5954bcdfb84eb7588d5c00f734d08a4cf70dac Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 25 Jan 2024 15:59:56 +0000 Subject: [PATCH] Look for annotations on methods as well as classes Currently using something like `@FlakyTest` only works if a whole class is annotated. It can be preferable to just annotate the single flaky method, rather than the whole class. This allows that. --- java/gazelle/generate.go | 8 +- java/gazelle/lang.go | 2 +- java/gazelle/private/java/BUILD.bazel | 1 + java/gazelle/private/java/package.go | 4 +- java/gazelle/private/javaparser/BUILD.bazel | 1 + java/gazelle/private/javaparser/javaparser.go | 10 +- .../java/javaparser/v0/javaparser.proto | 8 +- .../src/test/com/example/myproject/BUILD.out | 16 ++++ .../test/com/example/myproject/MixedTest.java | 25 +++++ .../generators/ClasspathParser.java | 92 ++++++++++++++++--- .../javaparser/generators/GrpcServer.java | 22 +++-- .../generators/ClasspathParserTest.java | 32 +++++-- .../AnnotationAfterImportOnMethod.java | 8 ++ 13 files changed, 196 insertions(+), 33 deletions(-) create mode 100644 java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/MixedTest.java create mode 100644 java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/AnnotationAfterImportOnMethod.java diff --git a/java/gazelle/generate.go b/java/gazelle/generate.go index 4480102d..0467c9de 100644 --- a/java/gazelle/generate.go +++ b/java/gazelle/generate.go @@ -415,7 +415,13 @@ func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], from func accumulateJavaFile(cfg *javaconfig.Config, testJavaFiles, testHelperJavaFiles *sorted_set.SortedSet[javaFile], separateTestJavaFiles map[javaFile]separateJavaTestReasons, file javaFile, perClassMetadata map[string]java.PerClassMetadata, log zerolog.Logger) { if cfg.IsJavaTestFile(filepath.Base(file.pathRelativeToBazelWorkspaceRoot)) { - annotationClassNames := perClassMetadata[file.ClassName().FullyQualifiedClassName()].AnnotationClassNames + annotationClassNames := sorted_set.NewSortedSet[string](nil) + metadataForClass := perClassMetadata[file.ClassName().FullyQualifiedClassName()] + annotationClassNames.AddAll(metadataForClass.AnnotationClassNames) + for _, key := range metadataForClass.MethodAnnotationClassNames.Keys() { + annotationClassNames.AddAll(metadataForClass.MethodAnnotationClassNames.Values(key)) + } + perFileAttrs := make(map[string]bzl.Expr) wrapper := "" for _, annotationClassName := range annotationClassNames.SortedSlice() { diff --git a/java/gazelle/lang.go b/java/gazelle/lang.go index 1340fc61..c293d8eb 100644 --- a/java/gazelle/lang.go +++ b/java/gazelle/lang.go @@ -177,7 +177,7 @@ func (l javaLang) Loads() []rule.LoadInfo { for _, name := range s.Keys() { loads = append(loads, rule.LoadInfo{ Name: name, - Symbols: s.Values(name), + Symbols: s.SortedValues(name), }) } return loads diff --git a/java/gazelle/private/java/BUILD.bazel b/java/gazelle/private/java/BUILD.bazel index 2a71e9fb..c657de7d 100644 --- a/java/gazelle/private/java/BUILD.bazel +++ b/java/gazelle/private/java/BUILD.bazel @@ -12,6 +12,7 @@ go_library( # Allow visibility for plugins like Kotlin that don't live in this repo visibility = ["//visibility:public"], deps = [ + "//java/gazelle/private/sorted_multiset", "//java/gazelle/private/sorted_set", "//java/gazelle/private/types", ], diff --git a/java/gazelle/private/java/package.go b/java/gazelle/private/java/package.go index 25ff6bfc..e09072a8 100644 --- a/java/gazelle/private/java/package.go +++ b/java/gazelle/private/java/package.go @@ -1,6 +1,7 @@ package java import ( + "github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_multiset" "github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set" "github.com/bazel-contrib/rules_jvm/java/gazelle/private/types" ) @@ -21,5 +22,6 @@ type Package struct { } type PerClassMetadata struct { - AnnotationClassNames *sorted_set.SortedSet[string] + AnnotationClassNames *sorted_set.SortedSet[string] + MethodAnnotationClassNames *sorted_multiset.SortedMultiSet[string, string] } diff --git a/java/gazelle/private/javaparser/BUILD.bazel b/java/gazelle/private/javaparser/BUILD.bazel index 7cc99bc2..d15a4e1c 100644 --- a/java/gazelle/private/javaparser/BUILD.bazel +++ b/java/gazelle/private/javaparser/BUILD.bazel @@ -9,6 +9,7 @@ go_library( "//java/gazelle/private/java", "//java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0:javaparser", "//java/gazelle/private/servermanager", + "//java/gazelle/private/sorted_multiset", "//java/gazelle/private/sorted_set", "//java/gazelle/private/types", "@com_github_rs_zerolog//:zerolog", diff --git a/java/gazelle/private/javaparser/javaparser.go b/java/gazelle/private/javaparser/javaparser.go index 5411f8ff..24052362 100644 --- a/java/gazelle/private/javaparser/javaparser.go +++ b/java/gazelle/private/javaparser/javaparser.go @@ -8,6 +8,7 @@ import ( "github.com/bazel-contrib/rules_jvm/java/gazelle/private/java" pb "github.com/bazel-contrib/rules_jvm/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0" "github.com/bazel-contrib/rules_jvm/java/gazelle/private/servermanager" + "github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_multiset" "github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set" "github.com/bazel-contrib/rules_jvm/java/gazelle/private/types" "github.com/rs/zerolog" @@ -64,8 +65,15 @@ func (r Runner) ParsePackage(ctx context.Context, in *ParsePackageRequest) (*jav perClassMetadata := make(map[string]java.PerClassMetadata, len(resp.GetPerClassMetadata())) for k, v := range resp.GetPerClassMetadata() { + methodAnnotationClassNames := sorted_multiset.NewSortedMultiSet[string, string]() + for method, perMethod := range v.GetPerMethodMetadata() { + for _, annotation := range perMethod.AnnotationClassNames { + methodAnnotationClassNames.Add(method, annotation) + } + } metadata := java.PerClassMetadata{ - AnnotationClassNames: sorted_set.NewSortedSet(v.GetAnnotationClassNames()), + AnnotationClassNames: sorted_set.NewSortedSet(v.GetAnnotationClassNames()), + MethodAnnotationClassNames: methodAnnotationClassNames, } perClassMetadata[k] = metadata } diff --git a/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto b/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto index 772a446d..7d8162f1 100644 --- a/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto +++ b/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto @@ -51,7 +51,13 @@ message Package { } message PerClassMetadata { - repeated string annotation_class_names = 1; + repeated string annotation_class_names = 1; + // Not all methods will be present here, only those with something interesting to report. + map per_method_metadata = 2; +} + +message PerMethodMetadata { + repeated string annotation_class_names = 1; } service Lifecycle { diff --git a/java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/BUILD.out b/java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/BUILD.out index 04d2ee80..e4641b3b 100644 --- a/java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/BUILD.out +++ b/java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/BUILD.out @@ -15,6 +15,22 @@ java_test_suite( ], ) +java_junit5_test( + name = "MixedTest", + srcs = ["MixedTest.java"], + flaky = True, + test_class = "com.example.myproject.MixedTest", + runtime_deps = [ + "@maven//:org_junit_jupiter_junit_jupiter_engine", + "@maven//:org_junit_platform_junit_platform_launcher", + "@maven//:org_junit_platform_junit_platform_reporting", + ], + deps = [ + "//src/main/com/example/annotation", + "@maven//:org_junit_jupiter_junit_jupiter_api", + ], +) + java_junit5_test( name = "RandomTest", srcs = ["RandomTest.java"], diff --git a/java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/MixedTest.java b/java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/MixedTest.java new file mode 100644 index 00000000..c51ebaeb --- /dev/null +++ b/java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/MixedTest.java @@ -0,0 +1,25 @@ +package com.example.myproject; + +import com.example.annotation.FlakyTest; +import org.junit.jupiter.api.Test; + +import java.util.Random; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class MixedTest { + @Test + @FlakyTest + public void unreliableTest() { + Random random = new Random(); + int r = random.nextInt(2); + assertEquals(r, 0); + } + + @Test + public void reliableTest() { + Random random = new Random(); + int r = random.nextInt(2); + assertTrue(r < 2); + } +} diff --git a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java index 77f25c85..dd9ca9ae 100644 --- a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java +++ b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java @@ -5,9 +5,7 @@ import static javax.lang.model.element.Modifier.STATIC; import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Lists; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.ArrayTypeTree; @@ -31,7 +29,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.SortedMap; import java.util.SortedSet; import java.util.TreeMap; import java.util.TreeSet; @@ -57,7 +57,7 @@ public class ClasspathParser { // Mapping from fully-qualified class-name to class-names of annotations on that class. // Annotations will be fully-qualified where that's known, and not where not known. - private final Map> annotatedClasses = new TreeMap<>(); + final Map perClassData = new TreeMap<>(); // get the system java compiler instance private static final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); @@ -67,6 +67,46 @@ public ClasspathParser() { // Doesn't need to do anything currently } + static class PerClassData { + public PerClassData() { + this(new TreeSet<>(), new TreeMap<>()); + } + + @Override + public String toString() { + return "PerClassData{" + + "annotations=" + + annotations + + ", perMethodAnnotations=" + + perMethodAnnotations + + '}'; + } + + public PerClassData( + SortedSet annotations, SortedMap> perMethodAnnotations) { + this.annotations = annotations; + this.perMethodAnnotations = perMethodAnnotations; + } + + final SortedSet annotations; + + final SortedMap> perMethodAnnotations; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PerClassData that = (PerClassData) o; + return Objects.equals(annotations, that.annotations) + && Objects.equals(perMethodAnnotations, that.perMethodAnnotations); + } + + @Override + public int hashCode() { + return Objects.hash(annotations, perMethodAnnotations); + } + } + public ImmutableSet getUsedTypes() { return ImmutableSet.copyOf(usedTypes); } @@ -87,15 +127,6 @@ public ImmutableSet getMainClasses() { return ImmutableSet.copyOf(mainClasses); } - public ImmutableMap> getAnnotatedClasses() { - ImmutableMap.Builder> builder = - ImmutableMap.builderWithExpectedSize(annotatedClasses.size()); - for (Map.Entry> entry : annotatedClasses.entrySet()) { - builder.put(entry.getKey(), ImmutableSortedSet.copyOf(entry.getValue())); - } - return builder.build(); - } - public void parseClasses(Path directory, List files) throws IOException { StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, null, null); List objectFiles = @@ -245,6 +276,20 @@ public Void visitMethod(com.sun.source.tree.MethodTree m, Void v) { checkFullyQualifiedType(param.getType()); handleAnnotations(param.getModifiers().getAnnotations()); } + + for (AnnotationTree annotation : m.getModifiers().getAnnotations()) { + String annotationClassName = annotation.getAnnotationType().toString(); + String importedFullyQualified = currentFileImports.get(annotationClassName); + String currentFullyQualifiedClass = fullyQualify(currentPackage, currentClassName); + if (importedFullyQualified != null) { + noteAnnotatedMethod( + currentFullyQualifiedClass, m.getName().toString(), importedFullyQualified); + } else { + noteAnnotatedMethod( + currentFullyQualifiedClass, m.getName().toString(), annotationClassName); + } + } + return super.visitMethod(m, v); } @@ -333,10 +378,27 @@ private Set checkFullyQualifiedType(Tree identifier) { private void noteAnnotatedClass( String annotatedFullyQualifiedClassName, String annotationFullyQualifiedClassName) { - if (!annotatedClasses.containsKey(annotatedFullyQualifiedClassName)) { - annotatedClasses.put(annotatedFullyQualifiedClassName, new TreeSet<>()); + if (!perClassData.containsKey(annotatedFullyQualifiedClassName)) { + perClassData.put(annotatedFullyQualifiedClassName, new PerClassData()); + } + perClassData + .get(annotatedFullyQualifiedClassName) + .annotations + .add(annotationFullyQualifiedClassName); + } + + private void noteAnnotatedMethod( + String annotatedFullyQualifiedClassName, + String methodName, + String annotationFullyQualifiedClassName) { + if (!perClassData.containsKey(annotatedFullyQualifiedClassName)) { + perClassData.put(annotatedFullyQualifiedClassName, new PerClassData()); + } + PerClassData data = perClassData.get(annotatedFullyQualifiedClassName); + if (!data.perMethodAnnotations.containsKey(methodName)) { + data.perMethodAnnotations.put(methodName, new TreeSet<>()); } - annotatedClasses.get(annotatedFullyQualifiedClassName).add(annotationFullyQualifiedClassName); + data.perMethodAnnotations.get(methodName).add(annotationFullyQualifiedClassName); } private String fullyQualify(String packageName, String className) { diff --git a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java index 3b18b5a5..660754c7 100644 --- a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java +++ b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java @@ -7,9 +7,9 @@ import com.gazelle.java.javaparser.v0.Package.Builder; import com.gazelle.java.javaparser.v0.ParsePackageRequest; import com.gazelle.java.javaparser.v0.PerClassMetadata; +import com.gazelle.java.javaparser.v0.PerMethodMetadata; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import io.grpc.Server; import io.grpc.ServerBuilder; @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.SortedSet; import java.util.concurrent.TimeUnit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -154,13 +155,20 @@ private Package getImports(ParsePackageRequest request) { .addAllImportedPackagesWithoutSpecificClasses( parser.getUsedPackagesWithoutSpecificTypes()) .addAllMains(parser.getMainClasses()); - for (Map.Entry> annotations : - parser.getAnnotatedClasses().entrySet()) { - packageBuilder.putPerClassMetadata( - annotations.getKey(), + for (Map.Entry classEntry : + parser.perClassData.entrySet()) { + PerClassMetadata.Builder perClassMetadata = PerClassMetadata.newBuilder() - .addAllAnnotationClassNames(annotations.getValue()) - .build()); + .addAllAnnotationClassNames(classEntry.getValue().annotations); + for (Map.Entry> methodEntry : + classEntry.getValue().perMethodAnnotations.entrySet()) { + perClassMetadata.putPerMethodMetadata( + methodEntry.getKey(), + PerMethodMetadata.newBuilder() + .addAllAnnotationClassNames(methodEntry.getValue()) + .build()); + } + packageBuilder.putPerClassMetadata(classEntry.getKey(), perClassMetadata.build()); } return packageBuilder.build(); } diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java index 3cb2b0f6..6a7dab8e 100644 --- a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java @@ -14,6 +14,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeMap; import java.util.TreeSet; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -191,8 +193,26 @@ public void testAnnotationAfterImport() throws IOException { assertEquals( Map.of( "workspace.com.gazelle.java.javaparser.generators.AnnotationAfterImport", - treeSet("com.example.FlakyTest")), - parser.getAnnotatedClasses()); + new ClasspathParser.PerClassData(treeSet("com.example.FlakyTest"), new TreeMap<>())), + parser.perClassData); + } + + @Test + public void testAnnotationAfterImportOnMethod() throws IOException { + List files = + List.of( + testFiles.get( + "/workspace/com/gazelle/java/javaparser/generators/AnnotationAfterImportOnMethod.java")); + parser.parseClasses(files); + + TreeMap> expectedPerMethodAnnotations = new TreeMap<>(); + expectedPerMethodAnnotations.put("someTest", treeSet("org.junit.jupiter.api.Test")); + + assertEquals( + Map.of( + "workspace.com.gazelle.java.javaparser.generators.AnnotationAfterImportOnMethod", + new ClasspathParser.PerClassData(new TreeSet<>(), expectedPerMethodAnnotations)), + parser.perClassData); } @Test @@ -208,8 +228,8 @@ public void testAnnotationFromJavaStandardLibrary() throws IOException { assertEquals( Map.of( "workspace.com.gazelle.java.javaparser.generators.AnnotationFromJavaStandardLibrary", - treeSet("Deprecated")), - parser.getAnnotatedClasses()); + new ClasspathParser.PerClassData(treeSet("Deprecated"), new TreeMap<>())), + parser.perClassData); } @Test @@ -225,8 +245,8 @@ public void testAnnotationWithoutImport() throws IOException { assertEquals( Map.of( "workspace.com.gazelle.java.javaparser.generators.AnnotationWithoutImport", - treeSet("WhoKnowsWhereIAmFrom")), - parser.getAnnotatedClasses()); + new ClasspathParser.PerClassData(treeSet("WhoKnowsWhereIAmFrom"), new TreeMap<>())), + parser.perClassData); } @Test diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/AnnotationAfterImportOnMethod.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/AnnotationAfterImportOnMethod.java new file mode 100644 index 00000000..27ba45fb --- /dev/null +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/AnnotationAfterImportOnMethod.java @@ -0,0 +1,8 @@ +package workspace.com.gazelle.java.javaparser.generators; + +import org.junit.jupiter.api.Test; + +public class AnnotationAfterImportOnMethod { + @Test + public void someTest() {} +}