Skip to content

Commit 85518ce

Browse files
pdelagravetimtebeek
pdelagrave
and
timtebeek
authored
Recipe to remove .toString() called on parameterized logging statement arguments (#224)
Add `StripToStringFromArguments` SLF4j recipe --------- Co-authored-by: Tim te Beek <[email protected]>
1 parent 4d92d04 commit 85518ce

File tree

3 files changed

+192
-0
lines changed

3 files changed

+192
-0
lines changed

‎src/main/java/org/openrewrite/java/logging/slf4j/StripToStringFromArguments.java‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.java.logging.slf4j;
17+
18+
import org.openrewrite.ExecutionContext;
19+
import org.openrewrite.Preconditions;
20+
import org.openrewrite.Recipe;
21+
import org.openrewrite.TreeVisitor;
22+
import org.openrewrite.internal.ListUtils;
23+
import org.openrewrite.java.JavaIsoVisitor;
24+
import org.openrewrite.java.MethodMatcher;
25+
import org.openrewrite.java.search.UsesMethod;
26+
import org.openrewrite.java.tree.J;
27+
import org.openrewrite.java.tree.TypeUtils;
28+
29+
public class StripToStringFromArguments extends Recipe {
30+
private static final MethodMatcher TRACE_MATCHER = new MethodMatcher("org.slf4j.Logger trace(..)");
31+
private static final MethodMatcher DEBUG_MATCHER = new MethodMatcher("org.slf4j.Logger debug(..)");
32+
private static final MethodMatcher INFO_MATCHER = new MethodMatcher("org.slf4j.Logger info(..)");
33+
private static final MethodMatcher WARN_MATCHER = new MethodMatcher("org.slf4j.Logger warn(..)");
34+
private static final MethodMatcher ERROR_MATCHER = new MethodMatcher("org.slf4j.Logger error(..)");
35+
36+
private static final MethodMatcher TO_STRING_MATCHER = new MethodMatcher("*..* toString()");
37+
38+
@Override
39+
public String getDisplayName() {
40+
return "Strip `toString()` from arguments";
41+
}
42+
43+
@Override
44+
public String getDescription() {
45+
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.";
46+
}
47+
48+
@Override
49+
public TreeVisitor<?, ExecutionContext> getVisitor() {
50+
return Preconditions.check(Preconditions.or(
51+
new UsesMethod<>(TRACE_MATCHER),
52+
new UsesMethod<>(DEBUG_MATCHER),
53+
new UsesMethod<>(INFO_MATCHER),
54+
new UsesMethod<>(WARN_MATCHER),
55+
new UsesMethod<>(ERROR_MATCHER)
56+
),
57+
new JavaIsoVisitor<ExecutionContext>() {
58+
@Override
59+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation m, ExecutionContext ctx) {
60+
m = super.visitMethodInvocation(m, ctx);
61+
62+
if (!(TRACE_MATCHER.matches(m) ||
63+
DEBUG_MATCHER.matches(m) ||
64+
INFO_MATCHER.matches(m) ||
65+
WARN_MATCHER.matches(m) ||
66+
ERROR_MATCHER.matches(m))) {
67+
return m;
68+
}
69+
70+
int firstFormatArgIndex = TypeUtils.isOfClassType(m.getArguments().get(0).getType(), "org.slf4j.Marker") ? 2 : 1;
71+
final int lastArgIndex = m.getArguments().size() - 1;
72+
73+
return m.withArguments(ListUtils.map(m.getArguments(), (index, arg) -> {
74+
if (index < firstFormatArgIndex) {
75+
return arg;
76+
}
77+
if (arg instanceof J.MethodInvocation) {
78+
J.MethodInvocation toStringInvocation = (J.MethodInvocation) arg;
79+
if (TO_STRING_MATCHER.matches(toStringInvocation.getMethodType()) &&
80+
toStringInvocation.getSelect() != null &&
81+
!(firstFormatArgIndex == lastArgIndex && TypeUtils.isAssignableTo("java.lang.Throwable", toStringInvocation.getSelect().getType()))) {
82+
// Strip the `.toString()` call
83+
return toStringInvocation.getSelect().withPrefix(toStringInvocation.getPrefix());
84+
}
85+
}
86+
return arg;
87+
}));
88+
}
89+
});
90+
}
91+
}

‎src/main/resources/META-INF/rewrite/slf4j.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ recipeList:
146146
- org.openrewrite.java.logging.ChangeLoggersToPrivate
147147
- org.openrewrite.java.logging.slf4j.MatchIsLogLevelEnabledWithLogStatements
148148
- org.openrewrite.java.logging.slf4j.WrapExpensiveLogStatementsInConditionals
149+
- org.openrewrite.java.logging.slf4j.StripToStringFromArguments
149150
---
150151
type: specs.openrewrite.org/v1beta/recipe
151152
name: org.openrewrite.java.logging.slf4j.CommonsLogging1ToSlf4j1

‎src/test/java/org/openrewrite/java/logging/slf4j/StripToStringFromArgumentsTest.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.java.logging.slf4j;
17+
18+
import org.intellij.lang.annotations.Language;
19+
import org.junit.jupiter.params.ParameterizedTest;
20+
import org.junit.jupiter.params.provider.Arguments;
21+
import org.junit.jupiter.params.provider.MethodSource;
22+
import org.openrewrite.InMemoryExecutionContext;
23+
import org.openrewrite.java.JavaParser;
24+
import org.openrewrite.test.RecipeSpec;
25+
import org.openrewrite.test.RewriteTest;
26+
27+
import java.util.List;
28+
import java.util.stream.Stream;
29+
30+
import static org.openrewrite.java.Assertions.java;
31+
32+
class StripToStringFromArgumentsTest implements RewriteTest {
33+
private static Stream<Arguments> stripToStringFromLogMethodArguments() {
34+
record TestCase(String originalArgs, String expectedArgs) {
35+
}
36+
List<TestCase> cases = List.of(
37+
new TestCase(
38+
"",
39+
""),
40+
new TestCase(
41+
", o1.toString()",
42+
", o1"),
43+
new TestCase(
44+
", o1.toString(), o2.toString()",
45+
", o1, o2"),
46+
new TestCase(
47+
", o1.toString(), o2.toString(), o3.toString()",
48+
", o1, o2, o3"),
49+
new TestCase(
50+
", o1.toString(), o2, o3.toString(), o1, o3, o1.toString(), o2",
51+
", o1, o2, o3, o1, o3, o1, o2"),
52+
new TestCase(
53+
", exception.toString()",
54+
", exception.toString()"),
55+
new TestCase(
56+
", exception.toString(), o1",
57+
", exception, o1"),
58+
new TestCase(
59+
", o1, exception.toString()",
60+
", o1, exception")
61+
);
62+
63+
return Stream.of("trace", "debug", "info", "warn", "error")
64+
.flatMap(method -> cases.stream().flatMap(testCase ->
65+
Stream.of(
66+
Arguments.of(method, "message" + testCase.originalArgs, "message" + testCase.expectedArgs),
67+
Arguments.of(method, "marker, message" + testCase.originalArgs, "marker, message" + testCase.expectedArgs)
68+
)
69+
));
70+
}
71+
72+
@Override
73+
public void defaults(RecipeSpec spec) {
74+
spec.recipe(new StripToStringFromArguments()).parser(JavaParser.fromJavaVersion().classpathFromResources(new InMemoryExecutionContext(), "slf4j-api-2.1.+"));
75+
}
76+
77+
@ParameterizedTest
78+
@MethodSource
79+
void stripToStringFromLogMethodArguments(String method, String arguments, String expectedArguments) {
80+
String testTemplate = """
81+
import org.slf4j.Logger;
82+
import org.slf4j.Marker;
83+
84+
class A {
85+
void method(Logger log, Marker marker, String message, Object o1, Object o2, Object o3, Exception exception) {
86+
log.%s(%s);
87+
}
88+
}
89+
""";
90+
@Language("java") String before = String.format(testTemplate, method, arguments);
91+
@Language("java") String after = String.format(testTemplate, method, expectedArguments);
92+
93+
// Ideally we'd only call `rewriteRun(java(before, after));` but the only way to expect a no-change is to call `rewrite(java(before))`
94+
if (before.equals(after)) {
95+
rewriteRun(java(before));
96+
} else {
97+
rewriteRun(java(before, after));
98+
}
99+
}
100+
}

0 commit comments

Comments
 (0)