diff --git a/build.gradle.kts b/build.gradle.kts index d28bf07..8cfb8fe 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -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") diff --git a/src/main/java/org/openrewrite/java/logging/PrintStackTraceToLogError.java b/src/main/java/org/openrewrite/java/logging/PrintStackTraceToLogError.java index 0733ef3..2d2b926 100644 --- a/src/main/java/org/openrewrite/java/logging/PrintStackTraceToLogError.java +++ b/src/main/java/org/openrewrite/java/logging/PrintStackTraceToLogError.java @@ -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) @@ -69,6 +74,7 @@ public Duration getEstimatedEffortPerOccurrence() { public TreeVisitor getVisitor() { MethodMatcher printStackTrace = new MethodMatcher("java.lang.Throwable printStackTrace(..)"); LoggingFramework framework = LoggingFramework.fromOption(loggingFramework); + AnnotationMatcher lombokLogAnnotationMatcher = new AnnotationMatcher("@lombok.extern..*"); JavaIsoVisitor visitor = new JavaIsoVisitor() { @Override @@ -78,23 +84,34 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu J.ClassDeclaration clazz = getCursor().firstEnclosingOrThrow(J.ClassDeclaration.class); Set 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; + 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); } } diff --git a/src/main/java/org/openrewrite/java/logging/SystemErrToLogging.java b/src/main/java/org/openrewrite/java/logging/SystemErrToLogging.java index 0d2128c..365dee7 100644 --- a/src/main/java/org/openrewrite/java/logging/SystemErrToLogging.java +++ b/src/main/java/org/openrewrite/java/logging/SystemErrToLogging.java @@ -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; @@ -77,6 +77,7 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { LoggingFramework framework = LoggingFramework.fromOption(loggingFramework); + AnnotationMatcher lombokLogAnnotationMatcher = new AnnotationMatcher("@lombok.extern..*"); return Preconditions.check(new UsesMethod<>(systemErrPrint), new JavaIsoVisitor() { @Override @@ -142,39 +143,46 @@ private J.MethodInvocation logInsteadOfPrint(Cursor printCursor, ExecutionContex Set 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

JavaTemplate getErrorTemplateNoException(JavaVisitor

visitor) { switch (framework) { case SLF4J: diff --git a/src/main/java/org/openrewrite/java/logging/SystemOutToLogging.java b/src/main/java/org/openrewrite/java/logging/SystemOutToLogging.java index 6402980..d54b682 100644 --- a/src/main/java/org/openrewrite/java/logging/SystemOutToLogging.java +++ b/src/main/java/org/openrewrite/java/logging/SystemOutToLogging.java @@ -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) @@ -79,6 +79,7 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { LoggingFramework framework = LoggingFramework.fromOption(loggingFramework); + AnnotationMatcher lombokLogAnnotationMatcher = new AnnotationMatcher("@lombok.extern..*"); return Preconditions.check(new UsesMethod<>(systemOutPrint), new JavaIsoVisitor() { @Override @@ -101,19 +102,11 @@ private J.MethodInvocation logInsteadOfPrint(Cursor printCursor, ExecutionContex Set 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)); @@ -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

JavaTemplate getInfoTemplate(JavaVisitor

visitor) { String levelOrDefault = getLevel(); switch (framework) { diff --git a/src/test/java/org/openrewrite/java/logging/PrintStackTraceToLogErrorTest.java b/src/test/java/org/openrewrite/java/logging/PrintStackTraceToLogErrorTest.java index 7507b89..33f3f2e 100644 --- a/src/test/java/org/openrewrite/java/logging/PrintStackTraceToLogErrorTest.java +++ b/src/test/java/org/openrewrite/java/logging/PrintStackTraceToLogErrorTest.java @@ -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; @@ -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()), + //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); + } + } + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/logging/SystemErrToLoggingTest.java b/src/test/java/org/openrewrite/java/logging/SystemErrToLoggingTest.java index f210c69..460bc00 100644 --- a/src/test/java/org/openrewrite/java/logging/SystemErrToLoggingTest.java +++ b/src/test/java/org/openrewrite/java/logging/SystemErrToLoggingTest.java @@ -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; @@ -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); + } + } + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/logging/SystemOutToLoggingTest.java b/src/test/java/org/openrewrite/java/logging/SystemOutToLoggingTest.java index 5911c39..c1137a5 100644 --- a/src/test/java/org/openrewrite/java/logging/SystemOutToLoggingTest.java +++ b/src/test/java/org/openrewrite/java/logging/SystemOutToLoggingTest.java @@ -17,8 +17,10 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +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; @@ -88,4 +90,39 @@ void test() { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/114") + void supportLombokLogAnnotations() { + rewriteRun( + spec -> spec.recipe(new SystemOutToLogging(null, null, null, "info")) + .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() { + System.out.println("Oh " + n + " no"); + } + } + """, + """ + import lombok.extern.slf4j.Slf4j; + @Slf4j + class Test { + int n; + + void test() { + log.info("Oh {} no", n); + } + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/logging/log4j/Log4j1ToLog4j2Test.java b/src/test/java/org/openrewrite/java/logging/log4j/Log4j1ToLog4j2Test.java index 780753e..0cb2d92 100644 --- a/src/test/java/org/openrewrite/java/logging/log4j/Log4j1ToLog4j2Test.java +++ b/src/test/java/org/openrewrite/java/logging/log4j/Log4j1ToLog4j2Test.java @@ -21,12 +21,15 @@ import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; -import org.openrewrite.xml.tree.Xml; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static org.openrewrite.java.Assertions.java; import static org.openrewrite.java.Assertions.mavenProject; import static org.openrewrite.java.Assertions.srcMainJava; import static org.openrewrite.maven.Assertions.pomXml; +import static org.junit.jupiter.api.Assertions.assertTrue; class Log4j1ToLog4j2Test implements RewriteTest { @@ -163,7 +166,11 @@ class Test { """, - """ + sourceSpecs -> sourceSpecs.after(after -> { + Matcher matcher = Pattern.compile("(2\\.2\\d\\.\\d)").matcher(after); + assertTrue(matcher.find()); + String version = matcher.group(1); + return """ com.mycompany.app my-app @@ -177,21 +184,22 @@ class Test { org.apache.logging.log4j log4j-api - 2.20.0 + %1$s org.apache.logging.log4j log4j-core - 2.20.0 + %1$s org.apache.logging.log4j log4j-slf4j-impl - 2.20.0 + %1$s - """ + """.formatted(version); + }) ) ) )