Skip to content

Commit 0bef82e

Browse files
authored
Treat annotations on inner classes as if on outer (#278)
This was the behaviour before 8c5954b, and is a behaviour we want to preserve (at least until we have a way of expressing desires more explicitly here). Concretely, this means that things like `-java-annotation-to-attribute=com.example.annotation.FlakyTest=flaky=True` will allow inner annotated classes to affect the generation of targets, rather than ignoring their annotations.
1 parent f6b2319 commit 0bef82e

File tree

6 files changed

+85
-7
lines changed

6 files changed

+85
-7
lines changed

java/gazelle/generate.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -425,10 +425,19 @@ func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], from
425425
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) {
426426
if cfg.IsJavaTestFile(filepath.Base(file.pathRelativeToBazelWorkspaceRoot)) {
427427
annotationClassNames := sorted_set.NewSortedSetFn[types.ClassName](nil, types.ClassNameLess)
428-
metadataForClass := perClassMetadata[file.ClassName().FullyQualifiedClassName()]
429-
annotationClassNames.AddAll(metadataForClass.AnnotationClassNames)
430-
for _, key := range metadataForClass.MethodAnnotationClassNames.Keys() {
431-
annotationClassNames.AddAll(metadataForClass.MethodAnnotationClassNames.Values(key))
428+
// We attribute annotations on inner classes as if they apply to the outer class, so we need to strip inner class names when comparing.
429+
for class, metadataForClass := range perClassMetadata {
430+
className, err := types.ParseClassName(class)
431+
if err != nil {
432+
log.Warn().Err(err).Str("class-name", class).Msg("Failed to parse class name which was seen to have an annotation")
433+
continue
434+
}
435+
if className.FullyQualifiedOuterClassName() == file.ClassName().FullyQualifiedOuterClassName() {
436+
annotationClassNames.AddAll(metadataForClass.AnnotationClassNames)
437+
for _, key := range metadataForClass.MethodAnnotationClassNames.Keys() {
438+
annotationClassNames.AddAll(metadataForClass.MethodAnnotationClassNames.Values(key))
439+
}
440+
}
432441
}
433442

434443
perFileAttrs := make(map[string]bzl.Expr)

java/gazelle/private/types/types.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ func (c *ClassName) BareOuterClassName() string {
4343
return c.bareOuterClassName
4444
}
4545

46+
func (c *ClassName) FullyQualifiedOuterClassName() string {
47+
var parts []string
48+
if c.packageName.Name != "" {
49+
parts = append(parts, strings.Split(c.packageName.Name, ".")...)
50+
}
51+
parts = append(parts, c.bareOuterClassName)
52+
return strings.Join(parts, ".")
53+
}
54+
4655
func (c *ClassName) FullyQualifiedClassName() string {
4756
var parts []string
4857
if c.packageName.Name != "" {
@@ -68,7 +77,12 @@ func ParseClassName(fullyQualified string) (*ClassName, error) {
6877

6978
indexOfOuterClassName := len(parts) - 1
7079
for i := len(parts) - 1; i >= 0; i-- {
71-
if unicode.IsUpper([]rune(parts[i])[0]) {
80+
runes := []rune(parts[i])
81+
if len(runes) == 0 {
82+
// Anonymous inner classes end up getting parsed as having name "", so we need to do an "empty" check before looking at the first letter.
83+
// This means we skip over empty class names when trying to find outer classes.
84+
continue
85+
} else if unicode.IsUpper(runes[0]) {
7286
indexOfOuterClassName = i
7387
} else {
7488
break

java/gazelle/private/types/types_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ func TestParseClassName(t *testing.T) {
4141
innerClassNames: []string{"Inner", "Nested"},
4242
},
4343
},
44+
"anonymous inner": {
45+
from: "com.example.Simple.",
46+
want: &ClassName{
47+
packageName: NewPackageName("com.example"),
48+
bareOuterClassName: "Simple",
49+
innerClassNames: []string{""},
50+
},
51+
},
4452
} {
4553
t.Run(name, func(t *testing.T) {
4654
got, err := ParseClassName(tc.from)

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@ java_test_suite(
1515
],
1616
)
1717

18+
java_junit5_test(
19+
name = "FlakyInnerTest",
20+
srcs = ["FlakyInnerTest.java"],
21+
flaky = True,
22+
test_class = "com.example.myproject.FlakyInnerTest",
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 = "MixedTest",
2036
srcs = ["MixedTest.java"],
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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 FlakyInnerTest {
11+
@FlakyTest
12+
public static class Nested {
13+
@Test
14+
public void unreliableTest() {
15+
Random random = new Random();
16+
int r = random.nextInt(2);
17+
assertEquals(r, 0);
18+
}
19+
}
20+
}

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,10 +337,21 @@ public void testAnonymousInnerClass() throws IOException {
337337
"/workspace/com/gazelle/java/javaparser/generators/AnonymousInnerClass.java"));
338338
parser.parseClasses(files);
339339

340-
Set<String> expected =
340+
Set<String> expectedTypes =
341341
Set.of(
342342
"java.util.HashMap", "javax.annotation.Nullable", "org.jetbrains.annotations.Nullable");
343-
assertEquals(expected, parser.getUsedTypes());
343+
assertEquals(expectedTypes, parser.getUsedTypes());
344+
345+
Map<String, ClasspathParser.PerClassData> expectedPerClassMetadata = new TreeMap<>();
346+
TreeMap<String, SortedSet<String>> expectedPerMethodAnnotations = new TreeMap<>();
347+
expectedPerMethodAnnotations.put(
348+
"containsValue", treeSet("Override", "javax.annotation.Nullable"));
349+
// This anonymous inner class really has a name like $1, but we don't know what number it will
350+
// end up getting given, so we just use the empty string for anonymous inner classes.
351+
expectedPerClassMetadata.put(
352+
"workspace.com.gazelle.java.javaparser.generators.AnonymousInnerClass.",
353+
new ClasspathParser.PerClassData(treeSet(), expectedPerMethodAnnotations, new TreeMap<>()));
354+
assertEquals(expectedPerClassMetadata, parser.perClassData);
344355
}
345356

346357
@Test

0 commit comments

Comments
 (0)