Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package org.openrewrite.java.logging.slf4j;

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.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.TypeUtils;

import java.util.ArrayList;
import java.util.List;

public class StripToStringFromArguments extends Recipe {
@Override
public String getDisplayName() {
return "Strip `toString()` from arguments";
}

@Override
public String getDescription() {
return "Remove `.toString()` from logger call arguments; SLF4J will automatically call `toString()` on an argument when not a string, and do so only if the log level is enabled.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(Preconditions.or(
new UsesMethod<>("org.slf4j.Logger trace(..)"),
new UsesMethod<>("org.slf4j.Logger debug(..)"),
new UsesMethod<>("org.slf4j.Logger info(..)"),
new UsesMethod<>("org.slf4j.Logger warn(..)"),
new UsesMethod<>("org.slf4j.Logger error(..)")
),
new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) {
final int firstFormatArgIndex = mi.getArguments().get(0).getType().toString() == "org.slf4j.Marker" ? 2 : 1;
final List<Expression> newArguments = new ArrayList<>(mi.getArguments().subList(0, firstFormatArgIndex));
for (int i = firstFormatArgIndex; i < mi.getArguments().size(); i++) {
final Expression arg = mi.getArguments().get(i);
Expression toAdd = arg;
if (arg instanceof J.MethodInvocation) {
J.MethodInvocation toStringInvocation = (J.MethodInvocation) arg;
if (toStringInvocation.getSimpleName().equals("toString") &&
toStringInvocation.getSelect() != null &&
toStringInvocation.getSelect().getType() != null &&
!TypeUtils.isAssignableTo("java.lang.Throwable", toStringInvocation.getSelect().getType())) {
toAdd = toStringInvocation.getSelect().withPrefix(toStringInvocation.getPrefix());
}
}
newArguments.add(toAdd);
}
return mi.withArguments(newArguments);
}
});
}
}
1 change: 1 addition & 0 deletions src/main/resources/META-INF/rewrite/slf4j.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ recipeList:
- org.openrewrite.java.logging.ChangeLoggersToPrivate
- org.openrewrite.java.logging.slf4j.MatchIsLogLevelEnabledWithLogStatements
- org.openrewrite.java.logging.slf4j.WrapExpensiveLogStatementsInConditionals
- org.openrewrite.java.logging.slf4j.StripToStringFromArguments
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.logging.slf4j.CommonsLogging1ToSlf4j1
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.openrewrite.java.logging.slf4j;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import java.util.ArrayList;
import java.util.List;

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

public class StripToStringFromArgumentsTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.recipe(new StripToStringFromArguments()).parser(JavaParser.fromJavaVersion().classpathFromResources(new InMemoryExecutionContext(), "slf4j-api-2.1.+"));
}

private static List<Arguments> stripToStringFromLogMethodArguments() {
List<Arguments> arguments = new ArrayList<>();
for (String marker : List.of("", "marker, ")) {
for (String method : List.of("trace", "debug", "info", "warn", "error")) {
arguments.addAll(List.of(
Arguments.of(method, marker,
List.of("o1.toString()"),
List.of("o1")),
Arguments.of(method, marker,
List.of("o1.toString()", "o2.toString()"),
List.of("o1", "o2")),
Arguments.of(method, marker,
List.of("o1.toString()", "o2.toString()", "o3.toString()"),
List.of("o1", "o2", "o3")),
Arguments.of(method, marker,
List.of("o1.toString()", "o2", "o3.toString()", "o1", "o3", "o1.toString()", "o2"),
List.of("o1", "o2", "o3", "o1", "o3", "o1", "o2")),
Arguments.of(method, marker,
List.of("exception.toString()"),
List.of("exception.toString()"))
));
}
}
return arguments;
}

@ParameterizedTest
@MethodSource
void stripToStringFromLogMethodArguments(String method, String marker, List<String> arguments, List<String> expectedArguments) {
//language=java
String testTemplate = """
import org.slf4j.Logger;
import org.slf4j.Marker;

class A {
void method(Logger log, Object o1, Object o2, Object o3, Exception exception, Marker marker) {
log.%s(%s"Hello", %s);
}
}
""";
String before = String.format(testTemplate, method, marker, String.join(", ", arguments));
String after = String.format(testTemplate, method, marker, String.join(", ", expectedArguments));

// Ideally we'd only call `rewriteRun(java(before, after));` but the only way to expect a no-change is to call `java(before)`
if (before.equals(after))
rewriteRun(java(before));
else
rewriteRun(java(before, after));
}
}
Loading