diff --git a/src/main/java/org/openrewrite/java/logging/slf4j/MatchIsLogLevelEnabledWithLogStatements.java b/src/main/java/org/openrewrite/java/logging/slf4j/MatchIsLogLevelEnabledWithLogStatements.java new file mode 100644 index 00000000..f9f90ebf --- /dev/null +++ b/src/main/java/org/openrewrite/java/logging/slf4j/MatchIsLogLevelEnabledWithLogStatements.java @@ -0,0 +1,144 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.logging.slf4j; + +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesMethod; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Statement; + +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.Comparator.comparing; +import static java.util.Comparator.nullsFirst; +import static java.util.Objects.requireNonNull; +import static java.util.function.BinaryOperator.maxBy; + +public class MatchIsLogLevelEnabledWithLogStatements extends Recipe { + + private static final MethodMatcher ENABLED_MATCHER = new MethodMatcher("org.slf4j.Logger is*Enabled()"); + private static final MethodMatcher LOG_MATCHER = new MethodMatcher("org.slf4j.Logger *(..)"); + + @Override + public String getDisplayName() { + return "Match `if (is*Enabled())` with logging statements"; + } + + @Override + public String getDescription() { + return "Change any `if (is*Enabled())` statements that do not match the maximum log level used in the `then` " + + "part to use the matching `is*Enabled()` method for that log level. " + + "This ensures that the logging condition is consistent with the actual logging statements."; + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check( + new UsesMethod<>(ENABLED_MATCHER), + new JavaIsoVisitor() { + @Override + public J.If visitIf(J.If iff, ExecutionContext ctx) { + J.If if_ = super.visitIf(iff, ctx); + if (if_.getIfCondition().getTree() instanceof J.MethodInvocation && if_.getElsePart() == null) { + J.MethodInvocation mi = (J.MethodInvocation) if_.getIfCondition().getTree(); + LogLevel conditionLogLevel = LogLevel.extractEnabledLogLevel(mi); + if (conditionLogLevel != null) { + LogLevel maxUsedLogLevel = findMaxUsedLogLevel(if_.getThenPart()); + if (maxUsedLogLevel != null && conditionLogLevel != maxUsedLogLevel) { + return if_.withIfCondition(if_.getIfCondition() + .withTree(maxUsedLogLevel.toEnabledInvocation(mi))); + } + } + } + return if_; + } + + private @Nullable LogLevel findMaxUsedLogLevel(Statement statement) { + return new JavaIsoVisitor>() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicReference<@Nullable LogLevel> reference) { + J.MethodInvocation mi = super.visitMethodInvocation(method, reference); + reference.accumulateAndGet(LogLevel.extractUsedLogLevel(mi), maxBy(nullsFirst(comparing(LogLevel::ordinal)))); + return mi; + } + + @Override + public J.Try.Catch visitCatch(J.Try.Catch catch_, AtomicReference<@Nullable LogLevel> logLevelAtomicReference) { + return catch_; + } + + @Override + public J.MultiCatch visitMultiCatch(J.MultiCatch multiCatch, AtomicReference<@Nullable LogLevel> logLevelAtomicReference) { + return multiCatch; + } + }.reduce(statement, new AtomicReference<>(null)).get(); + } + } + ); + } + + private enum LogLevel { + trace, + debug, + info, + warn, + error; + + public static @Nullable LogLevel extractEnabledLogLevel(J.MethodInvocation methodInvocation) { + if (ENABLED_MATCHER.matches(methodInvocation)) { + String methodName = methodInvocation.getSimpleName(); + String logLevel = methodName.substring(2, methodName.length() - 7); + for (LogLevel level : values()) { + if (level.name().equalsIgnoreCase(logLevel)) { + return level; + } + } + } + return null; + } + + public static @Nullable LogLevel extractUsedLogLevel(J.MethodInvocation methodInvocation) { + if (LOG_MATCHER.matches(methodInvocation)) { + for (LogLevel level : values()) { + if (level.name().equals(methodInvocation.getSimpleName())) { + return level; + } + } + } + return null; + } + + public J.MethodInvocation toEnabledInvocation(J.MethodInvocation methodInvocation) { + String enabledMethodName = String.format("is%s%sEnabled", + name().substring(0, 1).toUpperCase(), + name().substring(1)); + JavaType.Method type = requireNonNull(methodInvocation.getMethodType()) + .withName(enabledMethodName); + return methodInvocation + .withName(methodInvocation.getName() + .withSimpleName(enabledMethodName) + .withType(type)) + .withMethodType(type); + } + } +} diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 6fe32964..f05fd36a 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -886,6 +886,29 @@ examples: language: java --- type: specs.openrewrite.org/v1beta/example +recipeName: org.openrewrite.java.logging.slf4j.MatchIsLogLevelEnabledWithLogStatements +examples: +- description: '' + sources: + - before: | + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isDebugEnabled()) { + logger.info("message"); + } + } + } + after: | + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isInfoEnabled()) { + logger.info("message"); + } + } + } + language: java +--- +type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.java.logging.slf4j.Slf4jBestPractices examples: - description: '' diff --git a/src/main/resources/META-INF/rewrite/slf4j.yml b/src/main/resources/META-INF/rewrite/slf4j.yml index a4399905..4d29c4ba 100644 --- a/src/main/resources/META-INF/rewrite/slf4j.yml +++ b/src/main/resources/META-INF/rewrite/slf4j.yml @@ -144,6 +144,7 @@ recipeList: - org.openrewrite.java.logging.slf4j.CompleteExceptionLogging - org.openrewrite.java.logging.CatchBlockLogLevel - org.openrewrite.java.logging.ChangeLoggersToPrivate + - org.openrewrite.java.logging.slf4j.MatchIsLogLevelEnabledWithLogStatements - org.openrewrite.java.logging.slf4j.WrapExpensiveLogStatementsInConditionals --- type: specs.openrewrite.org/v1beta/recipe diff --git a/src/test/java/org/openrewrite/java/logging/slf4j/MatchIsLogLevelEnabledWithLogStatementsTest.java b/src/test/java/org/openrewrite/java/logging/slf4j/MatchIsLogLevelEnabledWithLogStatementsTest.java new file mode 100644 index 00000000..33b839af --- /dev/null +++ b/src/test/java/org/openrewrite/java/logging/slf4j/MatchIsLogLevelEnabledWithLogStatementsTest.java @@ -0,0 +1,226 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.logging.slf4j; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class MatchIsLogLevelEnabledWithLogStatementsTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new MatchIsLogLevelEnabledWithLogStatements()); + } + + @DocumentExample + @Test + void debugEnabledToInfo() { + rewriteRun( + java( + """ + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isDebugEnabled()) { + logger.info("message"); + } + } + } + """, + """ + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isInfoEnabled()) { + logger.info("message"); + } + } + } + """ + ) + ); + } + + @Test + void pickHighestEnabled() { + rewriteRun( + java( + """ + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isDebugEnabled()) { + logger.debug("message"); + logger.info("message"); + logger.warn("message"); + } + } + } + """, + """ + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isWarnEnabled()) { + logger.debug("message"); + logger.info("message"); + logger.warn("message"); + } + } + } + """ + ) + ); + } + + @Test + void pickLowerIfPossible() { + rewriteRun( + java( + """ + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isWarnEnabled()) { + logger.debug("message"); + } + } + } + """, + """ + class Test { + void test(org.slf4j.Logger logger) { + if (logger.isDebugEnabled()) { + logger.debug("message"); + } + } + } + """ + ) + ); + } + + @Nested + class NoChange { + @Test + void alreadyMatching() { + rewriteRun( + java( + """ + class Test { + void composed(org.slf4j.Logger logger) { + if (logger.isDebugEnabled()) { + logger.debug("message"); + } + } + } + """ + ) + ); + } + + @Test + void noChangeForNestedTryCatch() { + rewriteRun( + java( + """ + class Test { + void composed(org.slf4j.Logger logger) { + if (logger.isDebugEnabled()) { + try { + logger.debug("message"); + } catch (Exception e) { + // Fault handling for debug logging should not raise conditional logging + logger.warn("warning", e); + } + } + } + } + """ + ) + ); + } + + @Test + void noMatchingLoggingStatements() { + rewriteRun( + java( + """ + class Test { + void composed(org.slf4j.Logger logger) { + if (logger.isDebugEnabled()) { + // Some indirection to a logging call + } + } + } + """ + ) + ); + } + + @Test + void additionalDetailsInElse() { + rewriteRun( + java( + """ + class Test { + void composed(org.slf4j.Logger logger, Throwable t) { + if (logger.isDebugEnabled()) { + logger.warn("message", t); + } else { + logger.warn("message"); + } + } + } + """ + ) + ); + } + + @Test + void composedConditional() { + rewriteRun( + java( + """ + class Test { + void composed(org.slf4j.Logger logger, boolean condition) { + if (logger.isDebugEnabled() && condition) { + logger.info("message"); + } + } + } + """ + ) + ); + } + + @Test + void negatedConditional() { + rewriteRun( + java( + """ + class Test { + void negated(org.slf4j.Logger logger) { + if (!logger.isDebugEnabled()) { + logger.info("message"); + } + } + } + """ + ) + ); + } + } +}