Skip to content

Commit

Permalink
Treat annotations on inner classes as if on outer (#278)
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 authored Jun 5, 2024
1 parent f6b2319 commit 0bef82e
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 0bef82e

Please sign in to comment.