[epilogue] Improved opt-in logging support (#7437)

2026-06-19 00:41:43 +00:00 · 2024-12-03 23:30:55 -05:00
parent 54f0e11fc0
commit 60198c0bec
5 changed files with 215 additions and 18 deletions
--- a/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/AnnotationProcessor.java
+++ b/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/AnnotationProcessor.java
@@ -11,9 +11,12 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.processing.AbstractProcessor;
 import javax.annotation.processing.RoundEnvironment;
 import javax.annotation.processing.SupportedAnnotationTypes;
@@ -90,15 +93,15 @@ public class AnnotationProcessor extends AbstractProcessor {
                      e);
            });

+    var loggedTypes = getLoggedTypes(roundEnv);
+
    // Handlers are declared in order of priority. If an element could be logged in more than one
    // way (eg a class implements both Sendable and StructSerializable), the order of the handlers
    // in this list will determine how it gets logged.
    m_handlers =
        List.of(
            new LoggableHandler(
-                processingEnv,
-                roundEnv.getElementsAnnotatedWith(
-                    Logged.class)), // prioritize epilogue logging over Sendable
+                processingEnv, loggedTypes), // prioritize epilogue logging over Sendable
            new ConfiguredLoggerHandler(
                processingEnv, customLoggers), // then customized logging configs
            new ArrayHandler(processingEnv),
@@ -118,12 +121,39 @@ public class AnnotationProcessor extends AbstractProcessor {
        .findAny()
        .ifPresent(
            epilogue -> {
-              processEpilogue(roundEnv, epilogue);
+              processEpilogue(roundEnv, epilogue, loggedTypes);
            });

    return false;
  }

+  /**
+   * Gets the set of all loggable types in the compilation unit. A type is considered loggable if it
+   * is directly annotated with {@code @Logged} or contains a field or method with a {@code @Logged}
+   * annotation.
+   *
+   * @param roundEnv the compilation round environment
+   * @return the set of all loggable types
+   */
+  private Set<TypeElement> getLoggedTypes(RoundEnvironment roundEnv) {
+    // Fetches everything annotated with @Logged; classes, methods, values, etc.
+    var annotatedElements = roundEnv.getElementsAnnotatedWith(Logged.class);
+    return Stream.concat(
+            // 1. All type elements (classes, interfaces, or enums) with the @Logged annotation
+            annotatedElements.stream()
+                .filter(e -> e instanceof TypeElement)
+                .map(e -> (TypeElement) e),
+            // 2. All type elements containing a field or method with the @Logged annotation
+            annotatedElements.stream()
+                .filter(e -> e instanceof VariableElement || e instanceof ExecutableElement)
+                .map(Element::getEnclosingElement)
+                .filter(e -> e instanceof TypeElement)
+                .map(e -> (TypeElement) e))
+        .sorted(Comparator.comparing(e -> e.getSimpleName().toString()))
+        .collect(
+            Collectors.toCollection(LinkedHashSet::new)); // Collect to a set to avoid duplicates
+  }
+
  private boolean validateFields(Set<? extends Element> annotatedElements) {
    var fields =
        annotatedElements.stream()
@@ -340,7 +370,8 @@ public class AnnotationProcessor extends AbstractProcessor {
    return customLoggers;
  }

-  private void processEpilogue(RoundEnvironment roundEnv, TypeElement epilogueAnnotation) {
+  private void processEpilogue(
+      RoundEnvironment roundEnv, TypeElement epilogueAnnotation, Set<TypeElement> loggedTypes) {
    var annotatedElements = roundEnv.getElementsAnnotatedWith(epilogueAnnotation);

    List<String> loggerClassNames = new ArrayList<>();
@@ -358,12 +389,7 @@ public class AnnotationProcessor extends AbstractProcessor {
      return;
    }

-    var classes =
-        annotatedElements.stream()
-            .filter(e -> e instanceof TypeElement)
-            .map(e -> (TypeElement) e)
-            .toList();
-    for (TypeElement clazz : classes) {
+    for (TypeElement clazz : loggedTypes) {
      try {
        warnOfNonLoggableElements(clazz);
        m_loggerGenerator.writeLoggerFile(clazz);
@@ -391,7 +417,7 @@ public class AnnotationProcessor extends AbstractProcessor {

  private void warnOfNonLoggableElements(TypeElement clazz) {
    var config = clazz.getAnnotation(Logged.class);
-    if (config.strategy() == Logged.Strategy.OPT_IN) {
+    if (config == null || config.strategy() == Logged.Strategy.OPT_IN) {
      // field and method validations will have already checked everything
      return;
    }
--- a/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/LoggableHandler.java
+++ b/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/LoggableHandler.java
@@ -15,7 +15,6 @@ import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
-import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;

@@ -35,10 +34,8 @@ public class LoggableHandler extends ElementHandler {

  @Override
  public boolean isLoggable(Element element) {
-    var dataType = dataType(element);
-    return dataType.getAnnotation(Logged.class) != null
-        || dataType instanceof DeclaredType decl
-            && decl.asElement().getAnnotation(Logged.class) != null;
+    return m_processingEnv.getTypeUtils().asElement(dataType(element)) instanceof TypeElement t
+        && m_loggedTypes.contains(t);
  }

  @Override
--- 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,6 +15,7 @@ import edu.wpi.first.epilogue.Logged;
 import edu.wpi.first.epilogue.NotLogged;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.lang.annotation.Annotation;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Deque;
@@ -42,6 +43,33 @@ public class LoggerGenerator {
      LoggerGenerator::isBuiltInJavaMethod;
  private final ProcessingEnvironment m_processingEnv;
  private final List<ElementHandler> m_handlers;
+  private final Logged m_defaultConfig =
+      new Logged() {
+        @Override
+        public Class<? extends Annotation> annotationType() {
+          return Logged.class;
+        }
+
+        @Override
+        public String name() {
+          return "";
+        }
+
+        @Override
+        public Strategy strategy() {
+          return Strategy.OPT_IN;
+        }
+
+        @Override
+        public Importance importance() {
+          return Importance.DEBUG;
+        }
+
+        @Override
+        public Naming defaultNaming() {
+          return Naming.USE_CODE_NAME;
+        }
+      };

  public LoggerGenerator(ProcessingEnvironment processingEnv, List<ElementHandler> handlers) {
    this.m_processingEnv = processingEnv;
@@ -76,6 +104,9 @@ public class LoggerGenerator {
   */
  public void writeLoggerFile(TypeElement clazz) throws IOException {
    var config = clazz.getAnnotation(Logged.class);
+    if (config == null) {
+      config = m_defaultConfig;
+    }
    boolean requireExplicitOptIn = config.strategy() == Logged.Strategy.OPT_IN;

    Predicate<Element> notSkipped = LoggerGenerator::isNotSkipped;
--- 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
@@ -123,7 +123,7 @@ public final class StringUtils {
    String packageName = p.getQualifiedName().toString();

    String className;
-    if (config.name().isEmpty()) {
+    if (config == null || config.name().isEmpty()) {
      className = String.join("$", nesting);
    } else {
      className = capitalize(config.name()).replaceAll(" ", "");
--- 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
@@ -9,6 +9,7 @@ import static com.google.testing.compile.Compiler.javac;
 import static edu.wpi.first.epilogue.processor.CompileTestOptions.kJavaVersionOptions;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;

 import com.google.testing.compile.Compilation;
 import com.google.testing.compile.JavaFileObjects;
@@ -59,6 +60,107 @@ class AnnotationProcessorTest {
    assertLoggerGenerates(source, expectedGeneratedSource);
  }

+  @Test
+  void optInFields() {
+    String source =
+        """
+      package edu.wpi.first.epilogue;
+
+      class Example {
+        @Logged double x;
+        @Logged int y;
+      }
+    """;
+
+    String expectedGeneratedSource =
+        """
+      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.EpilogueBackend;
+
+      public class ExampleLogger extends ClassSpecificLogger<Example> {
+        public ExampleLogger() {
+          super(Example.class);
+        }
+
+        @Override
+        public void update(EpilogueBackend backend, Example object) {
+          if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
+            backend.log("x", object.x);
+            backend.log("y", object.y);
+          }
+        }
+      }
+      """;
+
+    assertLoggerGenerates(source, expectedGeneratedSource);
+  }
+
+  @Test
+  void optInMethods() {
+    String source =
+        """
+      package edu.wpi.first.epilogue;
+
+      class Example {
+        @Logged public double getValue() { return 2.0; }
+        @Logged public String getName() { return "Example"; }
+      }
+    """;
+
+    String expectedGeneratedSource =
+        """
+      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.EpilogueBackend;
+
+      public class ExampleLogger extends ClassSpecificLogger<Example> {
+        public ExampleLogger() {
+          super(Example.class);
+        }
+
+        @Override
+        public void update(EpilogueBackend backend, Example object) {
+          if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
+            backend.log("getValue", object.getValue());
+            backend.log("getName", object.getName());
+          }
+        }
+      }
+      """;
+
+    assertLoggerGenerates(source, expectedGeneratedSource);
+  }
+
+  @Test
+  void shouldNotLog() {
+    String source =
+        """
+      class Example {
+        public double getValue() { return 2.0; }
+        public String getName() { return "Example"; }
+      }
+    """;
+
+    Compilation compilation =
+        javac()
+            .withOptions(kJavaVersionOptions)
+            .withProcessors(new AnnotationProcessor())
+            .compile(JavaFileObjects.forSourceString("edu.wpi.first.epilogue.Example", source));
+
+    assertThat(compilation).succeeded();
+    // nothing is annotated with @Logged; so, no logger file should be generated
+    assertTrue(
+        compilation.generatedSourceFiles().stream()
+            .noneMatch(jfo -> jfo.getName().contains("Example")));
+  }
+
  @Test
  void multiple() {
    String source =
@@ -1416,6 +1518,47 @@ class AnnotationProcessorTest {
    assertLoggerGenerates(source, expectedRootLogger);
  }

+  @Test
+  void nestedOptIn() {
+    String source =
+        """
+        package edu.wpi.first.epilogue;
+
+        class Implicit {
+          @Logged double x;
+        }
+
+        class Example {
+          @Logged Implicit i;
+        }
+        """;
+
+    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.EpilogueBackend;
+
+        public class ExampleLogger extends ClassSpecificLogger<Example> {
+          public ExampleLogger() {
+            super(Example.class);
+          }
+
+          @Override
+          public void update(EpilogueBackend backend, Example object) {
+            if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
+              Epilogue.implicitLogger.tryUpdate(backend.getNested("i"), object.i, Epilogue.getConfig().errorHandler);
+            }
+          }
+        }
+        """;
+
+    assertLoggerGenerates(source, expectedRootLogger);
+  }
+
  @Test
  void customLogger() {
    String source =