Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for Lombok log annotations in three recipes #115

Merged
merged 6 commits into from
Oct 27, 2023
Merged

Support for Lombok log annotations in three recipes #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
74886eb
Add limited support for Lombok log annotations
timtebeek Oct 26, 2023
d24a86e
Remove JetBrains NotNull annotation
timtebeek Oct 26, 2023
e84f32a
Also support `@Sf4j` in SystemErrToLogging
timtebeek Oct 26, 2023
f19b1f0
Remove unused field from test
timtebeek Oct 26, 2023
751f9e9
Also cover SystemOutToLogging
timtebeek Oct 26, 2023
93410e1
Harden mavenProjectDependenciesUpdated() with new releases
timtebeek Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ dependencies {
testImplementation("org.openrewrite:rewrite-test")
testImplementation("org.openrewrite:rewrite-java-tck")

testImplementation("org.projectlombok:lombok:latest.release")
testImplementation("org.assertj:assertj-core:latest.release")

testRuntimeOnly("org.openrewrite:rewrite-java-17")
Expand Down
39 changes: 28 additions & 11 deletions src/main/java/org/openrewrite/java/logging/PrintStackTraceToLogError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,19 @@
import lombok.Value;
import org.openrewrite.*;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.AnnotationMatcher;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.FindFieldsOfType;
import org.openrewrite.java.search.UsesType;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.Space;
import org.openrewrite.marker.Markers;

import java.time.Duration;
import java.util.Collections;
import java.util.Set;
import java.util.UUID;

@Value
@EqualsAndHashCode(callSuper = false)
Expand Down Expand Up @@ -69,6 +74,7 @@ public Duration getEstimatedEffortPerOccurrence() {
public TreeVisitor<?, ExecutionContext> getVisitor() {
MethodMatcher printStackTrace = new MethodMatcher("java.lang.Throwable printStackTrace(..)");
LoggingFramework framework = LoggingFramework.fromOption(loggingFramework);
AnnotationMatcher lombokLogAnnotationMatcher = new AnnotationMatcher("@lombok.extern..*");

JavaIsoVisitor<ExecutionContext> visitor = new JavaIsoVisitor<ExecutionContext>() {
@Override
Expand All @@ -78,23 +84,34 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
J.ClassDeclaration clazz = getCursor().firstEnclosingOrThrow(J.ClassDeclaration.class);
Set<J.VariableDeclarations> loggers = FindFieldsOfType.find(clazz, framework.getLoggerType());
if (!loggers.isEmpty()) {
m = framework.getErrorTemplate(this, "\"Exception\"")
.apply(
new Cursor(getCursor().getParent(), m),
m.getCoordinates().replace(),
loggers.iterator().next().getVariables().get(0).getName(),
m.getSelect());
if (framework == LoggingFramework.JUL) {
maybeAddImport("java.util.logging.Level");
}
J.Identifier logField = loggers.iterator().next().getVariables().get(0).getName();
m = replaceMethodInvocation(m, logField);
} else if (clazz.getAllAnnotations().stream().anyMatch(lombokLogAnnotationMatcher::matches)) {
String fieldName = loggerName == null ? "log" : loggerName;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default fields are named log, as per: https://projectlombok.org/features/log

Didn't quite feel like parsing any lombok configuration files in case folks set a different lombok.log.fieldName.
If they do that, then can set the loggerName option to have that be used for the logging statements instead.

J.Identifier logField = new J.Identifier(UUID.randomUUID(), Space.SINGLE_SPACE, Markers.EMPTY, Collections.emptyList(), fieldName, null, null);
m = replaceMethodInvocation(m, logField);
} else if (addLogger != null && addLogger) {
doAfterVisit(AddLogger.addLogger(clazz, framework, loggerName == null ? "logger" : loggerName));
}
}
return m;
}
};

return addLogger != null && addLogger ? visitor : Preconditions.check(new UsesType<>(framework.getLoggerType(), null), visitor);
private J.MethodInvocation replaceMethodInvocation(J.MethodInvocation m, J.Identifier logField) {
if (framework == LoggingFramework.JUL) {
maybeAddImport("java.util.logging.Level");
}
return framework.getErrorTemplate(this, "\"Exception\"").apply(
new Cursor(getCursor().getParent(), m),
m.getCoordinates().replace(),
logField,
m.getSelect());
}
};
return addLogger != null && addLogger ? visitor : Preconditions.check(
Preconditions.or(
new UsesType<>(framework.getLoggerType(), null),
new UsesType<>("lombok.extern..*", null))
, visitor);
}
}
66 changes: 37 additions & 29 deletions src/main/java/org/openrewrite/java/logging/SystemErrToLogging.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
import org.openrewrite.java.*;
import org.openrewrite.java.search.FindFieldsOfType;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;

import java.time.Duration;
import java.util.Collections;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;

Expand Down Expand Up @@ -77,6 +77,7 @@ public String getDescription() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
LoggingFramework framework = LoggingFramework.fromOption(loggingFramework);
AnnotationMatcher lombokLogAnnotationMatcher = new AnnotationMatcher("@lombok.extern..*");

return Preconditions.check(new UsesMethod<>(systemErrPrint), new JavaIsoVisitor<ExecutionContext>() {
@Override
Expand Down Expand Up @@ -142,39 +143,46 @@ private J.MethodInvocation logInsteadOfPrint(Cursor printCursor, ExecutionContex
Set<J.VariableDeclarations> loggers = FindFieldsOfType.find(clazz, framework.getLoggerType());
if (!loggers.isEmpty()) {
J.Identifier computedLoggerName = loggers.iterator().next().getVariables().get(0).getName();
if (exceptionPrintStackTrace == null) {
print = getErrorTemplateNoException(this)
.apply(
printCursor,
print.getCoordinates().replace(),
computedLoggerName,
print.getArguments().get(0));
} else {
print = framework.getErrorTemplate(this, "#{any(String)}")
.apply(
printCursor,
print.getCoordinates().replace(),
computedLoggerName,
print.getArguments().get(0),
exceptionPrintStackTrace);
}

print = (J.MethodInvocation) new ParameterizedLogging(framework.getLoggerType() + " error(..)", false)
.getVisitor()
.visitNonNull(print, ctx, printCursor);

if (framework == LoggingFramework.JUL) {
maybeAddImport("java.util.logging.Level");
}
print = replaceMethodInvocation(printCursor, ctx, exceptionPrintStackTrace, print, computedLoggerName);
} else if (clazz.getAllAnnotations().stream().anyMatch(lombokLogAnnotationMatcher::matches)) {
String fieldName = loggerName == null ? "log" : loggerName;
J.Identifier logField = new J.Identifier(UUID.randomUUID(), Space.SINGLE_SPACE, Markers.EMPTY, Collections.emptyList(), fieldName, null, null);
print = replaceMethodInvocation(printCursor, ctx, exceptionPrintStackTrace, print, logField);
} else if (addLogger != null && addLogger) {
doAfterVisit(AddLogger.addLogger(clazz, framework, loggerName == null ? "logger" : loggerName));

// the print statement will be replaced on the subsequent pass
doAfterVisit(this);
}
return print;
}

private J.MethodInvocation replaceMethodInvocation(Cursor printCursor, ExecutionContext ctx, Expression exceptionPrintStackTrace, J.MethodInvocation print, J.Identifier computedLoggerName) {
if (exceptionPrintStackTrace == null) {
print = getErrorTemplateNoException(this)
.apply(
printCursor,
print.getCoordinates().replace(),
computedLoggerName,
print.getArguments().get(0));
} else {
print = framework.getErrorTemplate(this, "#{any(String)}")
.apply(
printCursor,
print.getCoordinates().replace(),
computedLoggerName,
print.getArguments().get(0),
exceptionPrintStackTrace);
}

if (framework == LoggingFramework.JUL) {
maybeAddImport("java.util.logging.Level");
}

return (J.MethodInvocation) new ParameterizedLogging(framework.getLoggerType() + " error(..)", false)
.getVisitor()
.visitNonNull(print, ctx, printCursor);
}

public <P> JavaTemplate getErrorTemplateNoException(JavaVisitor<P> visitor) {
switch (framework) {
case SLF4J:
Expand Down
44 changes: 27 additions & 17 deletions src/main/java/org/openrewrite/java/logging/SystemOutToLogging.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
import org.openrewrite.java.*;
import org.openrewrite.java.search.FindFieldsOfType;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;

import java.time.Duration;
import java.util.Collections;
import java.util.Set;
import java.util.UUID;

@Value
@EqualsAndHashCode(callSuper = false)
Expand Down Expand Up @@ -79,6 +79,7 @@ public String getDescription() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
LoggingFramework framework = LoggingFramework.fromOption(loggingFramework);
AnnotationMatcher lombokLogAnnotationMatcher = new AnnotationMatcher("@lombok.extern..*");

return Preconditions.check(new UsesMethod<>(systemOutPrint), new JavaIsoVisitor<ExecutionContext>() {
@Override
Expand All @@ -101,19 +102,11 @@ private J.MethodInvocation logInsteadOfPrint(Cursor printCursor, ExecutionContex
Set<J.VariableDeclarations> loggers = FindFieldsOfType.find(clazz, framework.getLoggerType());
if (!loggers.isEmpty()) {
J.Identifier computedLoggerName = loggers.iterator().next().getVariables().get(0).getName();
print = getInfoTemplate(this).apply(
printCursor,
print.getCoordinates().replace(),
computedLoggerName,
print.getArguments().get(0));

print = (J.MethodInvocation) new ParameterizedLogging(framework.getLoggerType() + " " + getLevel() + "(..)", false)
.getVisitor()
.visitNonNull(print, ctx, printCursor);

if (framework == LoggingFramework.JUL) {
maybeAddImport("java.util.logging.Level");
}
print = replaceMethodInvocation(printCursor, ctx, print, computedLoggerName);
} else if (clazz.getAllAnnotations().stream().anyMatch(lombokLogAnnotationMatcher::matches)) {
String fieldName = loggerName == null ? "log" : loggerName;
J.Identifier logField = new J.Identifier(UUID.randomUUID(), Space.SINGLE_SPACE, Markers.EMPTY, Collections.emptyList(), fieldName, null, null);
print = replaceMethodInvocation(printCursor, ctx, print, logField);
} else if (addLogger != null && addLogger) {
doAfterVisit(AddLogger.addLogger(clazz, framework, loggerName == null ? "logger" : loggerName));

Expand All @@ -123,6 +116,23 @@ private J.MethodInvocation logInsteadOfPrint(Cursor printCursor, ExecutionContex
return print;
}

private J.MethodInvocation replaceMethodInvocation(Cursor printCursor, ExecutionContext ctx, J.MethodInvocation print, J.Identifier computedLoggerName) {
print = getInfoTemplate(this).apply(
printCursor,
print.getCoordinates().replace(),
computedLoggerName,
print.getArguments().get(0));

print = (J.MethodInvocation) new ParameterizedLogging(framework.getLoggerType() + " " + getLevel() + "(..)", false)
.getVisitor()
.visitNonNull(print, ctx, printCursor);

if (framework == LoggingFramework.JUL) {
maybeAddImport("java.util.logging.Level");
}
return print;
}

private <P> JavaTemplate getInfoTemplate(JavaVisitor<P> visitor) {
String levelOrDefault = getLevel();
switch (framework) {
Expand Down
42 changes: 42 additions & 0 deletions src/test/java/org/openrewrite/java/logging/PrintStackTraceToLogErrorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.openrewrite.Issue;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import static org.openrewrite.java.Assertions.java;

Expand Down Expand Up @@ -235,4 +236,45 @@ public void close() {
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/114")
void supportLombokLogAnnotations() {
rewriteRun(
spec -> spec.recipe(new PrintStackTraceToLogError(null, null, null))
.parser(JavaParser.fromJavaVersion().classpath("slf4j-api", "lombok"))
.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the fields are absent, we have missing type information in the result. I figured that's ok & to be expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there is no way to get the type information from the annotation processor...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we don't run the Lombok annotation processor, so we don't have that information available; it's good enough to make this code change; not quite sure if any followup recipes would work, but the JavaTemplates we use should have the type information, as per

rewrite-logging-frameworks/src/main/java/org/openrewrite/java/logging/SystemErrToLogging.java

Lines 178 to 185 in 53f8787

public <P> JavaTemplate getErrorTemplateNoException(JavaVisitor<P> visitor) {
switch (framework) {
case SLF4J:
return JavaTemplate
.builder("#{any(org.slf4j.Logger)}.error(#{any(String)})")
.contextSensitive()
.javaParser(JavaParser.fromJavaVersion().classpath("slf4j-api"))
.build();

//language=java
java(
"""
import lombok.extern.slf4j.Slf4j;
@Slf4j
class Test {
void test() {
try {
} catch(Throwable t) {
t.printStackTrace();
t.printStackTrace(System.err);
t.printStackTrace(System.out);
}
}
}
""",
"""
import lombok.extern.slf4j.Slf4j;
@Slf4j
class Test {
void test() {
try {
} catch(Throwable t) {
log.error("Exception", t);
log.error("Exception", t);
log.error("Exception", t);
}
}
}
"""
)
);
}
}
44 changes: 44 additions & 0 deletions src/test/java/org/openrewrite/java/logging/SystemErrToLoggingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.Issue;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.logging.logback.Log4jAppenderToLogback;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import static org.openrewrite.java.Assertions.java;

Expand Down Expand Up @@ -162,4 +164,46 @@ void test() {
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/114")
void supportLombokLogAnnotations() {
rewriteRun(
spec -> spec.recipe(new SystemErrToLogging(null, null, null))
.parser(JavaParser.fromJavaVersion().classpath("slf4j-api", "lombok"))
.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
//language=java
java(
"""
import lombok.extern.slf4j.Slf4j;
@Slf4j
class Test {
int n;

void test() {
try {
} catch(Throwable t) {
System.err.println("Oh " + n + " no");
t.printStackTrace();
}
}
}
""",
"""
import lombok.extern.slf4j.Slf4j;
@Slf4j
class Test {
int n;

void test() {
try {
} catch(Throwable t) {
log.error("Oh {} no", n, t);
}
}
}
"""
)
);
}
}
Loading
Loading