[epilogue] Fix epilogue generating incorrect packages for inner classes (#7439)

2026-06-19 00:41:43 +00:00 · 2024-11-28 02:02:00 -05:00
parent 6ef5b85758
commit 9607c6c10d
3 changed files with 163 additions and 48 deletions
--- a/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/LoggerGenerator.java
+++ b/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/LoggerGenerator.java
@@ -15,7 +15,9 @@ import edu.wpi.first.epilogue.Logged;
 import edu.wpi.first.epilogue.NotLogged;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
+import java.util.Deque;
 import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.List;
@@ -29,6 +31,7 @@ import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.Name;
+import javax.lang.model.element.PackageElement;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
 import javax.tools.Diagnostic;
@@ -150,36 +153,28 @@ public class LoggerGenerator {
          }
        });

-    writeLoggerFile(clazz.getQualifiedName().toString(), config, fieldsToLog, methodsToLog);
+    writeLoggerFile(clazz, config, fieldsToLog, methodsToLog);
  }

  private void writeLoggerFile(
-      String className,
+      TypeElement clazz,
      Logged classConfig,
      List<VariableElement> loggableFields,
      List<ExecutableElement> loggableMethods)
      throws IOException {
-    String packageName = null;
-    int lastDot = className.lastIndexOf('.');
-    if (lastDot > 0) {
-      packageName = className.substring(0, lastDot);
+    // Walk nesting levels, to support inner classes
+    Deque<String> nesting = new ArrayDeque<>();
+    Element enclosing = clazz.getEnclosingElement();
+    while (!(enclosing instanceof PackageElement p)) {
+      nesting.addFirst(enclosing.getSimpleName().toString());
+      enclosing = enclosing.getEnclosingElement();
    }
+    String packageName = p.getQualifiedName().toString();
+    nesting.addLast(clazz.getSimpleName().toString());
+    String simpleClassName = String.join(".", nesting);

-    String simpleClassName = StringUtils.simpleName(className);
-    String loggerClassName = className + "Logger";
-    String loggerSimpleClassName = loggerClassName.substring(lastDot + 1);
-
-    // Use the name on the class config to set the generated logger names
-    // This helps to avoid naming conflicts
-    if (!classConfig.name().isBlank()) {
-      loggerSimpleClassName =
-          StringUtils.capitalize(classConfig.name().replaceAll(" ", "")) + "Logger";
-      if (lastDot > 0) {
-        loggerClassName = packageName + "." + loggerSimpleClassName;
-      } else {
-        loggerClassName = loggerSimpleClassName;
-      }
-    }
+    String loggerClassName = StringUtils.loggerClassName(clazz);
+    String loggerSimpleClassName = StringUtils.simpleName(loggerClassName);

    var loggerFile = m_processingEnv.getFiler().createSourceFile(loggerClassName);

@@ -220,13 +215,13 @@ public class LoggerGenerator {
        }
        out.println();

-        var clazz = simpleClassName + ".class";
+        var classReference = simpleClassName + ".class";

        out.println("  static {");
        out.println("    try {");
        out.println(
            "      var lookup = MethodHandles.privateLookupIn("
-                + clazz
+                + classReference
                + ", MethodHandles.lookup());");

        for (var privateField : privateFields) {
@@ -235,7 +230,7 @@ public class LoggerGenerator {
              "      $"
                  + fieldName
                  + " = lookup.findVarHandle("
-                  + clazz
+                  + classReference
                  + ", \""
                  + fieldName
                  + "\", "
--- a/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/StringUtils.java
+++ b/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/StringUtils.java
@@ -5,9 +5,13 @@
 package edu.wpi.first.epilogue.processor;

 import edu.wpi.first.epilogue.Logged;
+import java.util.ArrayDeque;
 import java.util.Arrays;
+import java.util.Deque;
 import java.util.List;
 import java.util.stream.Collectors;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.PackageElement;
 import javax.lang.model.element.TypeElement;

 public final class StringUtils {
@@ -108,34 +112,24 @@ public final class StringUtils {
   */
  public static String loggerClassName(TypeElement clazz) {
    var config = clazz.getAnnotation(Logged.class);
-    var className = clazz.getQualifiedName().toString();

-    String packageName;
-    int lastDot = className.lastIndexOf('.');
-    if (lastDot <= 0) {
-      packageName = null;
+    Deque<String> nesting = new ArrayDeque<>();
+    Element enclosing = clazz.getEnclosingElement();
+    while (!(enclosing instanceof PackageElement p)) {
+      nesting.addFirst(enclosing.getSimpleName().toString());
+      enclosing = enclosing.getEnclosingElement();
+    }
+    nesting.addLast(clazz.getSimpleName().toString());
+    String packageName = p.getQualifiedName().toString();
+
+    String className;
+    if (config.name().isEmpty()) {
+      className = String.join("$", nesting);
    } else {
-      packageName = className.substring(0, lastDot);
+      className = capitalize(config.name()).replaceAll(" ", "");
    }

-    String loggerClassName;
-
-    // Use the name on the class config to set the generated logger names
-    // This helps to avoid naming conflicts
-    if (config.name().isBlank()) {
-      loggerClassName = className + "Logger";
-    } else {
-      String cleaned = config.name().replaceAll(" ", "");
-
-      var loggerSimpleClassName = StringUtils.capitalize(cleaned) + "Logger";
-      if (packageName != null) {
-        loggerClassName = packageName + "." + loggerSimpleClassName;
-      } else {
-        loggerClassName = loggerSimpleClassName;
-      }
-    }
-
-    return loggerClassName;
+    return packageName + "." + className + "Logger";
  }

  /**
--- a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java
+++ b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java
@@ -1166,6 +1166,132 @@ class AnnotationProcessorTest {
    assertLoggerGenerates(source, expectedRootLogger);
  }

+  @Test
+  void innerClasses() {
+    String source =
+        """
+        package edu.wpi.first.epilogue;
+
+        class Outer {
+          @Logged
+          class Example { // Deliberately nonstatic
+            double x;
+          }
+        }
+        """;
+
+    String expectedRootLogger =
+        """
+        package edu.wpi.first.epilogue;
+
+        import edu.wpi.first.epilogue.Logged;
+        import edu.wpi.first.epilogue.Epilogue;
+        import edu.wpi.first.epilogue.logging.ClassSpecificLogger;
+        import edu.wpi.first.epilogue.logging.DataLogger;
+
+        public class Outer$ExampleLogger extends ClassSpecificLogger<Outer.Example> {
+          public Outer$ExampleLogger() {
+            super(Outer.Example.class);
+          }
+
+          @Override
+          public void update(DataLogger dataLogger, Outer.Example object) {
+            if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
+              dataLogger.log("x", object.x);
+            }
+          }
+        }
+        """;
+
+    assertLoggerGenerates(source, expectedRootLogger);
+  }
+
+  @Test
+  void highlyNestedInnerClasses() {
+    String source =
+        """
+        package edu.wpi.first.epilogue;
+
+        class A {
+          class B {
+            class C {
+              class D {
+                @Logged
+                class Example {
+                  double x;
+                }
+              }
+            }
+          }
+        }
+        """;
+
+    String expectedRootLogger =
+        """
+        package edu.wpi.first.epilogue;
+
+        import edu.wpi.first.epilogue.Logged;
+        import edu.wpi.first.epilogue.Epilogue;
+        import edu.wpi.first.epilogue.logging.ClassSpecificLogger;
+        import edu.wpi.first.epilogue.logging.DataLogger;
+
+        public class A$B$C$D$ExampleLogger extends ClassSpecificLogger<A.B.C.D.Example> {
+          public A$B$C$D$ExampleLogger() {
+            super(A.B.C.D.Example.class);
+          }
+
+          @Override
+          public void update(DataLogger dataLogger, A.B.C.D.Example object) {
+            if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
+              dataLogger.log("x", object.x);
+            }
+          }
+        }
+        """;
+
+    assertLoggerGenerates(source, expectedRootLogger);
+  }
+
+  @Test
+  void renamedInnerClass() {
+    String source =
+        """
+        package edu.wpi.first.epilogue;
+
+        class Outer {
+          @Logged(name = "Custom Example") // For the sake of testing, needs "Example" somewhere in the name
+          class Example {
+            double x;
+          }
+        }
+        """;
+
+    String expectedRootLogger =
+        """
+        package edu.wpi.first.epilogue;
+
+        import edu.wpi.first.epilogue.Logged;
+        import edu.wpi.first.epilogue.Epilogue;
+        import edu.wpi.first.epilogue.logging.ClassSpecificLogger;
+        import edu.wpi.first.epilogue.logging.DataLogger;
+
+        public class CustomExampleLogger extends ClassSpecificLogger<Outer.Example> {
+          public CustomExampleLogger() {
+            super(Outer.Example.class);
+          }
+
+          @Override
+          public void update(DataLogger dataLogger, Outer.Example object) {
+            if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
+              dataLogger.log("x", object.x);
+            }
+          }
+        }
+        """;
+
+    assertLoggerGenerates(source, expectedRootLogger);
+  }
+
  @Test
  void diamondInheritance() {
    String source =