Skip to content

Commit

Permalink
Treat annotations on inner classes as if on outer
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
illicitonion committed Jun 5, 2024
1 parent f6b2319 commit 963c565
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 7 deletions.
17 changes: 13 additions & 4 deletions java/gazelle/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,19 @@ 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 := sorted_set.NewSortedSetFn[types.ClassName](nil, types.ClassNameLess)
metadataForClass := perClassMetadata[file.ClassName().FullyQualifiedClassName()]
annotationClassNames.AddAll(metadataForClass.AnnotationClassNames)
for _, key := range metadataForClass.MethodAnnotationClassNames.Keys() {
annotationClassNames.AddAll(metadataForClass.MethodAnnotationClassNames.Values(key))
// We attribute annotations on inner classes as if they apply to the outer class, so we need to strip inner class names when comparing.
for class, metadataForClass := range perClassMetadata {
className, err := types.ParseClassName(class)
if err != nil {
log.Warn().Err(err).Str("class-name", class).Msg("Failed to parse class name which was seen to have an annotation")
continue
}
if className.FullyQualifiedOuterClassName() == file.ClassName().FullyQualifiedOuterClassName() {
annotationClassNames.AddAll(metadataForClass.AnnotationClassNames)
for _, key := range metadataForClass.MethodAnnotationClassNames.Keys() {
annotationClassNames.AddAll(metadataForClass.MethodAnnotationClassNames.Values(key))
}
}
}

perFileAttrs := make(map[string]bzl.Expr)
Expand Down
16 changes: 15 additions & 1 deletion java/gazelle/private/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ func (c *ClassName) BareOuterClassName() string {
return c.bareOuterClassName
}

func (c *ClassName) FullyQualifiedOuterClassName() string {
var parts []string
if c.packageName.Name != "" {
parts = append(parts, strings.Split(c.packageName.Name, ".")...)
}
parts = append(parts, c.bareOuterClassName)
return strings.Join(parts, ".")
}

func (c *ClassName) FullyQualifiedClassName() string {
var parts []string
if c.packageName.Name != "" {
Expand All @@ -68,7 +77,12 @@ func ParseClassName(fullyQualified string) (*ClassName, error) {

indexOfOuterClassName := len(parts) - 1
for i := len(parts) - 1; i >= 0; i-- {
if unicode.IsUpper([]rune(parts[i])[0]) {
runes := []rune(parts[i])
if len(runes) == 0 {
// Anonymous inner classes end up getting parsed as having name "", so we need to do an "empty" check before looking at the first letter.
// This means we skip over empty class names when trying to find outer classes.
continue
} else if unicode.IsUpper(runes[0]) {
indexOfOuterClassName = i
} else {
break
Expand Down
8 changes: 8 additions & 0 deletions java/gazelle/private/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ func TestParseClassName(t *testing.T) {
innerClassNames: []string{"Inner", "Nested"},
},
},
"anonymous inner": {
from: "com.example.Simple.",
want: &ClassName{
packageName: NewPackageName("com.example"),
bareOuterClassName: "Simple",
innerClassNames: []string{""},
},
},
} {
t.Run(name, func(t *testing.T) {
got, err := ParseClassName(tc.from)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,22 @@ java_test_suite(
],
)

java_junit5_test(
name = "FlakyInnerTest",
srcs = ["FlakyInnerTest.java"],
flaky = True,
test_class = "com.example.myproject.FlakyInnerTest",
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 = "MixedTest",
srcs = ["MixedTest.java"],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
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 FlakyInnerTest {
@FlakyTest
public static class Nested {
@Test
public void unreliableTest() {
Random random = new Random();
int r = random.nextInt(2);
assertEquals(r, 0);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,21 @@ public void testAnonymousInnerClass() throws IOException {
"/workspace/com/gazelle/java/javaparser/generators/AnonymousInnerClass.java"));
parser.parseClasses(files);

Set<String> expected =
Set<String> expectedTypes =
Set.of(
"java.util.HashMap", "javax.annotation.Nullable", "org.jetbrains.annotations.Nullable");
assertEquals(expected, parser.getUsedTypes());
assertEquals(expectedTypes, parser.getUsedTypes());

Map<String, ClasspathParser.PerClassData> expectedPerClassMetadata = new TreeMap<>();
TreeMap<String, SortedSet<String>> expectedPerMethodAnnotations = new TreeMap<>();
expectedPerMethodAnnotations.put(
"containsValue", treeSet("Override", "javax.annotation.Nullable"));
// This anonymous inner class really has a name like $1, but we don't know what number it will
// end up getting given, so we just use the empty string for anonymous inner classes.
expectedPerClassMetadata.put(
"workspace.com.gazelle.java.javaparser.generators.AnonymousInnerClass.",
new ClasspathParser.PerClassData(treeSet(), expectedPerMethodAnnotations, new TreeMap<>()));
assertEquals(expectedPerClassMetadata, parser.perClassData);
}

@Test
Expand Down

0 comments on commit 963c565

Please sign in to comment.