Skip to content

Commit 5804997

Browse files
illicitonionillicitonion
committed
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.
1 parent f2a4de0 commit 5804997

File tree

13 files changed

+196
-33
lines changed

13 files changed

+196
-33
lines changed

‎java/gazelle/generate.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,13 @@ func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], from
415415

416416
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) {
417417
if cfg.IsJavaTestFile(filepath.Base(file.pathRelativeToBazelWorkspaceRoot)) {
418-
annotationClassNames := perClassMetadata[file.ClassName().FullyQualifiedClassName()].AnnotationClassNames
418+
annotationClassNames := sorted_set.NewSortedSet[string](nil)
419+
metadataForClass := perClassMetadata[file.ClassName().FullyQualifiedClassName()]
420+
annotationClassNames.AddAll(metadataForClass.AnnotationClassNames)
421+
for _, key := range metadataForClass.MethodAnnotationClassNames.Keys() {
422+
annotationClassNames.AddAll(metadataForClass.MethodAnnotationClassNames.Values(key))
423+
}
424+
419425
perFileAttrs := make(map[string]bzl.Expr)
420426
wrapper := ""
421427
for _, annotationClassName := range annotationClassNames.SortedSlice() {

‎java/gazelle/lang.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (l javaLang) Loads() []rule.LoadInfo {
177177
for _, name := range s.Keys() {
178178
loads = append(loads, rule.LoadInfo{
179179
Name: name,
180-
Symbols: s.Values(name),
180+
Symbols: s.SortedValues(name),
181181
})
182182
}
183183
return loads

‎java/gazelle/private/java/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ go_library(
1212
# Allow visibility for plugins like Kotlin that don't live in this repo
1313
visibility = ["//visibility:public"],
1414
deps = [
15+
"//java/gazelle/private/sorted_multiset",
1516
"//java/gazelle/private/sorted_set",
1617
"//java/gazelle/private/types",
1718
],

‎java/gazelle/private/java/package.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package java
22

33
import (
4+
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_multiset"
45
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set"
56
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/types"
67
)
@@ -21,5 +22,6 @@ type Package struct {
2122
}
2223

2324
type PerClassMetadata struct {
24-
AnnotationClassNames *sorted_set.SortedSet[string]
25+
AnnotationClassNames *sorted_set.SortedSet[string]
26+
MethodAnnotationClassNames *sorted_multiset.SortedMultiSet[string, string]
2527
}

‎java/gazelle/private/javaparser/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ go_library(
99
"//java/gazelle/private/java",
1010
"//java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0:javaparser",
1111
"//java/gazelle/private/servermanager",
12+
"//java/gazelle/private/sorted_multiset",
1213
"//java/gazelle/private/sorted_set",
1314
"//java/gazelle/private/types",
1415
"@com_github_rs_zerolog//:zerolog",

‎java/gazelle/private/javaparser/javaparser.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/java"
99
pb "github.com/bazel-contrib/rules_jvm/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0"
1010
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/servermanager"
11+
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_multiset"
1112
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set"
1213
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/types"
1314
"github.com/rs/zerolog"
@@ -64,8 +65,15 @@ func (r Runner) ParsePackage(ctx context.Context, in *ParsePackageRequest) (*jav
6465

6566
perClassMetadata := make(map[string]java.PerClassMetadata, len(resp.GetPerClassMetadata()))
6667
for k, v := range resp.GetPerClassMetadata() {
68+
methodAnnotationClassNames := sorted_multiset.NewSortedMultiSet[string, string]()
69+
for method, perMethod := range v.GetPerMethodMetadata() {
70+
for _, annotation := range perMethod.AnnotationClassNames {
71+
methodAnnotationClassNames.Add(method, annotation)
72+
}
73+
}
6774
metadata := java.PerClassMetadata{
68-
AnnotationClassNames: sorted_set.NewSortedSet(v.GetAnnotationClassNames()),
75+
AnnotationClassNames: sorted_set.NewSortedSet(v.GetAnnotationClassNames()),
76+
MethodAnnotationClassNames: methodAnnotationClassNames,
6977
}
7078
perClassMetadata[k] = metadata
7179
}

‎java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto

+7-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,13 @@ message Package {
5151
}
5252

5353
message PerClassMetadata {
54-
repeated string annotation_class_names = 1;
54+
repeated string annotation_class_names = 1;
55+
// Not all methods will be present here, only those with something interesting to report.
56+
map<string, PerMethodMetadata> per_method_metadata = 2;
57+
}
58+
59+
message PerMethodMetadata {
60+
repeated string annotation_class_names = 1;
5561
}
5662

5763
service Lifecycle {

‎java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/BUILD.out

+16
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@ java_test_suite(
1515
],
1616
)
1717

18+
java_junit5_test(
19+
name = "MixedTest",
20+
srcs = ["MixedTest.java"],
21+
flaky = True,
22+
test_class = "com.example.myproject.MixedTest",
23+
runtime_deps = [
24+
"@maven//:org_junit_jupiter_junit_jupiter_engine",
25+
"@maven//:org_junit_platform_junit_platform_launcher",
26+
"@maven//:org_junit_platform_junit_platform_reporting",
27+
],
28+
deps = [
29+
"//src/main/com/example/annotation",
30+
"@maven//:org_junit_jupiter_junit_jupiter_api",
31+
],
32+
)
33+
1834
java_junit5_test(
1935
name = "RandomTest",
2036
srcs = ["RandomTest.java"],

‎java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/MixedTest.java

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package com.example.myproject;
2+
3+
import com.example.annotation.FlakyTest;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.util.Random;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
10+
public class MixedTest {
11+
@Test
12+
@FlakyTest
13+
public void unreliableTest() {
14+
Random random = new Random();
15+
int r = random.nextInt(2);
16+
assertEquals(r, 0);
17+
}
18+
19+
@Test
20+
public void reliableTest() {
21+
Random random = new Random();
22+
int r = random.nextInt(2);
23+
assertTrue(r < 2);
24+
}
25+
}

‎java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java

+77-15
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@
55
import static javax.lang.model.element.Modifier.STATIC;
66

77
import com.google.common.base.Splitter;
8-
import com.google.common.collect.ImmutableMap;
98
import com.google.common.collect.ImmutableSet;
10-
import com.google.common.collect.ImmutableSortedSet;
119
import com.google.common.collect.Lists;
1210
import com.sun.source.tree.AnnotationTree;
1311
import com.sun.source.tree.ArrayTypeTree;
@@ -31,7 +29,9 @@
3129
import java.util.HashMap;
3230
import java.util.List;
3331
import java.util.Map;
32+
import java.util.Objects;
3433
import java.util.Set;
34+
import java.util.SortedMap;
3535
import java.util.SortedSet;
3636
import java.util.TreeMap;
3737
import java.util.TreeSet;
@@ -57,7 +57,7 @@ public class ClasspathParser {
5757

5858
// Mapping from fully-qualified class-name to class-names of annotations on that class.
5959
// Annotations will be fully-qualified where that's known, and not where not known.
60-
private final Map<String, SortedSet<String>> annotatedClasses = new TreeMap<>();
60+
final Map<String, PerClassData> perClassData = new TreeMap<>();
6161

6262
// get the system java compiler instance
6363
private static final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
@@ -67,6 +67,46 @@ public ClasspathParser() {
6767
// Doesn't need to do anything currently
6868
}
6969

70+
static class PerClassData {
71+
public PerClassData() {
72+
this(new TreeSet<>(), new TreeMap<>());
73+
}
74+
75+
@Override
76+
public String toString() {
77+
return "PerClassData{"
78+
+ "annotations="
79+
+ annotations
80+
+ ", perMethodAnnotations="
81+
+ perMethodAnnotations
82+
+ '}';
83+
}
84+
85+
public PerClassData(
86+
SortedSet<String> annotations, SortedMap<String, SortedSet<String>> perMethodAnnotations) {
87+
this.annotations = annotations;
88+
this.perMethodAnnotations = perMethodAnnotations;
89+
}
90+
91+
final SortedSet<String> annotations;
92+
93+
final SortedMap<String, SortedSet<String>> perMethodAnnotations;
94+
95+
@Override
96+
public boolean equals(Object o) {
97+
if (this == o) return true;
98+
if (o == null || getClass() != o.getClass()) return false;
99+
PerClassData that = (PerClassData) o;
100+
return Objects.equals(annotations, that.annotations)
101+
&& Objects.equals(perMethodAnnotations, that.perMethodAnnotations);
102+
}
103+
104+
@Override
105+
public int hashCode() {
106+
return Objects.hash(annotations, perMethodAnnotations);
107+
}
108+
}
109+
70110
public ImmutableSet<String> getUsedTypes() {
71111
return ImmutableSet.copyOf(usedTypes);
72112
}
@@ -87,15 +127,6 @@ public ImmutableSet<String> getMainClasses() {
87127
return ImmutableSet.copyOf(mainClasses);
88128
}
89129

90-
public ImmutableMap<String, ImmutableSortedSet<String>> getAnnotatedClasses() {
91-
ImmutableMap.Builder<String, ImmutableSortedSet<String>> builder =
92-
ImmutableMap.builderWithExpectedSize(annotatedClasses.size());
93-
for (Map.Entry<String, SortedSet<String>> entry : annotatedClasses.entrySet()) {
94-
builder.put(entry.getKey(), ImmutableSortedSet.copyOf(entry.getValue()));
95-
}
96-
return builder.build();
97-
}
98-
99130
public void parseClasses(Path directory, List<String> files) throws IOException {
100131
StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, null, null);
101132
List<? extends JavaFileObject> objectFiles =
@@ -245,6 +276,20 @@ public Void visitMethod(com.sun.source.tree.MethodTree m, Void v) {
245276
checkFullyQualifiedType(param.getType());
246277
handleAnnotations(param.getModifiers().getAnnotations());
247278
}
279+
280+
for (AnnotationTree annotation : m.getModifiers().getAnnotations()) {
281+
String annotationClassName = annotation.getAnnotationType().toString();
282+
String importedFullyQualified = currentFileImports.get(annotationClassName);
283+
String currentFullyQualifiedClass = fullyQualify(currentPackage, currentClassName);
284+
if (importedFullyQualified != null) {
285+
noteAnnotatedMethod(
286+
currentFullyQualifiedClass, m.getName().toString(), importedFullyQualified);
287+
} else {
288+
noteAnnotatedMethod(
289+
currentFullyQualifiedClass, m.getName().toString(), annotationClassName);
290+
}
291+
}
292+
248293
return super.visitMethod(m, v);
249294
}
250295

@@ -333,10 +378,27 @@ private Set<String> checkFullyQualifiedType(Tree identifier) {
333378

334379
private void noteAnnotatedClass(
335380
String annotatedFullyQualifiedClassName, String annotationFullyQualifiedClassName) {
336-
if (!annotatedClasses.containsKey(annotatedFullyQualifiedClassName)) {
337-
annotatedClasses.put(annotatedFullyQualifiedClassName, new TreeSet<>());
381+
if (!perClassData.containsKey(annotatedFullyQualifiedClassName)) {
382+
perClassData.put(annotatedFullyQualifiedClassName, new PerClassData());
383+
}
384+
perClassData
385+
.get(annotatedFullyQualifiedClassName)
386+
.annotations
387+
.add(annotationFullyQualifiedClassName);
388+
}
389+
390+
private void noteAnnotatedMethod(
391+
String annotatedFullyQualifiedClassName,
392+
String methodName,
393+
String annotationFullyQualifiedClassName) {
394+
if (!perClassData.containsKey(annotatedFullyQualifiedClassName)) {
395+
perClassData.put(annotatedFullyQualifiedClassName, new PerClassData());
396+
}
397+
PerClassData data = perClassData.get(annotatedFullyQualifiedClassName);
398+
if (!data.perMethodAnnotations.containsKey(methodName)) {
399+
data.perMethodAnnotations.put(methodName, new TreeSet<>());
338400
}
339-
annotatedClasses.get(annotatedFullyQualifiedClassName).add(annotationFullyQualifiedClassName);
401+
data.perMethodAnnotations.get(methodName).add(annotationFullyQualifiedClassName);
340402
}
341403

342404
private String fullyQualify(String packageName, String className) {

‎java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java

+15-7
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
import com.gazelle.java.javaparser.v0.Package.Builder;
88
import com.gazelle.java.javaparser.v0.ParsePackageRequest;
99
import com.gazelle.java.javaparser.v0.PerClassMetadata;
10+
import com.gazelle.java.javaparser.v0.PerMethodMetadata;
1011
import com.google.common.base.Joiner;
1112
import com.google.common.collect.ImmutableSet;
12-
import com.google.common.collect.ImmutableSortedSet;
1313
import com.google.common.collect.Iterables;
1414
import io.grpc.Server;
1515
import io.grpc.ServerBuilder;
@@ -26,6 +26,7 @@
2626
import java.util.List;
2727
import java.util.Map;
2828
import java.util.Set;
29+
import java.util.SortedSet;
2930
import java.util.concurrent.TimeUnit;
3031
import org.slf4j.Logger;
3132
import org.slf4j.LoggerFactory;
@@ -154,13 +155,20 @@ private Package getImports(ParsePackageRequest request) {
154155
.addAllImportedPackagesWithoutSpecificClasses(
155156
parser.getUsedPackagesWithoutSpecificTypes())
156157
.addAllMains(parser.getMainClasses());
157-
for (Map.Entry<String, ImmutableSortedSet<String>> annotations :
158-
parser.getAnnotatedClasses().entrySet()) {
159-
packageBuilder.putPerClassMetadata(
160-
annotations.getKey(),
158+
for (Map.Entry<String, ClasspathParser.PerClassData> classEntry :
159+
parser.perClassData.entrySet()) {
160+
PerClassMetadata.Builder perClassMetadata =
161161
PerClassMetadata.newBuilder()
162-
.addAllAnnotationClassNames(annotations.getValue())
163-
.build());
162+
.addAllAnnotationClassNames(classEntry.getValue().annotations);
163+
for (Map.Entry<String, SortedSet<String>> methodEntry :
164+
classEntry.getValue().perMethodAnnotations.entrySet()) {
165+
perClassMetadata.putPerMethodMetadata(
166+
methodEntry.getKey(),
167+
PerMethodMetadata.newBuilder()
168+
.addAllAnnotationClassNames(methodEntry.getValue())
169+
.build());
170+
}
171+
packageBuilder.putPerClassMetadata(classEntry.getKey(), perClassMetadata.build());
164172
}
165173
return packageBuilder.build();
166174
}

‎java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java

+26-6
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import java.util.List;
1515
import java.util.Map;
1616
import java.util.Set;
17+
import java.util.SortedSet;
18+
import java.util.TreeMap;
1719
import java.util.TreeSet;
1820
import java.util.stream.Collectors;
1921
import java.util.stream.Stream;
@@ -191,8 +193,26 @@ public void testAnnotationAfterImport() throws IOException {
191193
assertEquals(
192194
Map.of(
193195
"workspace.com.gazelle.java.javaparser.generators.AnnotationAfterImport",
194-
treeSet("com.example.FlakyTest")),
195-
parser.getAnnotatedClasses());
196+
new ClasspathParser.PerClassData(treeSet("com.example.FlakyTest"), new TreeMap<>())),
197+
parser.perClassData);
198+
}
199+
200+
@Test
201+
public void testAnnotationAfterImportOnMethod() throws IOException {
202+
List<? extends JavaFileObject> files =
203+
List.of(
204+
testFiles.get(
205+
"/workspace/com/gazelle/java/javaparser/generators/AnnotationAfterImportOnMethod.java"));
206+
parser.parseClasses(files);
207+
208+
TreeMap<String, SortedSet<String>> expectedPerMethodAnnotations = new TreeMap<>();
209+
expectedPerMethodAnnotations.put("someTest", treeSet("org.junit.jupiter.api.Test"));
210+
211+
assertEquals(
212+
Map.of(
213+
"workspace.com.gazelle.java.javaparser.generators.AnnotationAfterImportOnMethod",
214+
new ClasspathParser.PerClassData(new TreeSet<>(), expectedPerMethodAnnotations)),
215+
parser.perClassData);
196216
}
197217

198218
@Test
@@ -208,8 +228,8 @@ public void testAnnotationFromJavaStandardLibrary() throws IOException {
208228
assertEquals(
209229
Map.of(
210230
"workspace.com.gazelle.java.javaparser.generators.AnnotationFromJavaStandardLibrary",
211-
treeSet("Deprecated")),
212-
parser.getAnnotatedClasses());
231+
new ClasspathParser.PerClassData(treeSet("Deprecated"), new TreeMap<>())),
232+
parser.perClassData);
213233
}
214234

215235
@Test
@@ -225,8 +245,8 @@ public void testAnnotationWithoutImport() throws IOException {
225245
assertEquals(
226246
Map.of(
227247
"workspace.com.gazelle.java.javaparser.generators.AnnotationWithoutImport",
228-
treeSet("WhoKnowsWhereIAmFrom")),
229-
parser.getAnnotatedClasses());
248+
new ClasspathParser.PerClassData(treeSet("WhoKnowsWhereIAmFrom"), new TreeMap<>())),
249+
parser.perClassData);
230250
}
231251

232252
@Test

0 commit comments

Comments
 (0)