Skip to content

Commit

Permalink
[epilogue] Improved opt-in logging support (#7437)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Daniel1464
Daniel1464 authored Dec 4, 2024
1 parent 54f0e11 commit 60198c0
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 18 deletions.
50 changes: 38 additions & 12 deletions epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/AnnotationProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@
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;
Expand Down Expand Up @@ -90,15 +93,15 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
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),
Expand All @@ -118,12 +121,39 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
.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()
Expand Down Expand Up @@ -340,7 +370,8 @@ private Map<DeclaredType, DeclaredType> processCustomLoggers(
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<>();
Expand All @@ -358,12 +389,7 @@ private void processEpilogue(RoundEnvironment roundEnv, TypeElement epilogueAnno
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);
Expand Down Expand Up @@ -391,7 +417,7 @@ private void processEpilogue(RoundEnvironment roundEnv, TypeElement epilogueAnno

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;
}
Expand Down
7 changes: 2 additions & 5 deletions epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/LoggableHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
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;

Expand All @@ -35,10 +34,8 @@ protected LoggableHandler(

@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
Expand Down
31 changes: 31 additions & 0 deletions epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/LoggerGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -76,6 +104,9 @@ private static boolean isBuiltInJavaMethod(ExecutableElement e) {
*/
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;
Expand Down
2 changes: 1 addition & 1 deletion epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/StringUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public static String loggerClassName(TypeElement clazz) {
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(" ", "");
Expand Down
143 changes: 143 additions & 0 deletions ...gue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand Down Expand Up @@ -59,6 +60,107 @@ public void update(EpilogueBackend backend, Example object) {
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 =
Expand Down Expand Up @@ -1416,6 +1518,47 @@ public void update(EpilogueBackend backend, Example object) {
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 =
Expand Down

0 comments on commit 60198c0

Please sign in to comment.