Skip to content

Commit 249944e

Browse files
Add ArgumentArrayToVarargs for logger methods that take a var args argument Object array (#246)
* Add start for converting Object[] to ... * Implement ArgumentArrayToVarargs * Add recipe to new JBossLoggingBestPractices * Add a case that should not be converted * Only convert when the method type has varargs flag set * Minor fixes and add to Slf4J best practices too * Update ArgumentArrayToVarargs.java * JulParameterizedArguments reuses ArgumentArrayToVarargs * Remove now unused methods * Slight polish --------- Co-authored-by: Pierre Delagrave <[email protected]>
1 parent bb34ecf commit 249944e

File tree

6 files changed

+401
-41
lines changed

6 files changed

+401
-41
lines changed
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
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;
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.*;
27+
28+
import java.time.Duration;
29+
import java.util.List;
30+
31+
public class ArgumentArrayToVarargs extends Recipe {
32+
// Match logger methods that end with Object[] - but we'll verify if it's varargs later
33+
private static final MethodMatcher LOGGER_METHOD = new MethodMatcher("*..*Log* *(.., Object[])");
34+
35+
@Override
36+
public String getDisplayName() {
37+
return "Unpack Logger method `new Object[] {...}` into varargs";
38+
}
39+
40+
@Override
41+
public String getDescription() {
42+
return "For Logger methods that support varargs, convert any final explicit `Object[]` arguments into their unpacked values.";
43+
}
44+
45+
@Override
46+
public Duration getEstimatedEffortPerOccurrence() {
47+
return Duration.ofMinutes(2);
48+
}
49+
50+
@Override
51+
public TreeVisitor<?, ExecutionContext> getVisitor() {
52+
return Preconditions.check(new UsesMethod<>(LOGGER_METHOD), new JavaIsoVisitor<ExecutionContext>() {
53+
@Override
54+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
55+
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
56+
if (LOGGER_METHOD.matches(mi)) {
57+
return mi.withArguments(ListUtils.flatMap(mi.getArguments(), (index, lastArg) -> {
58+
// Check if the last argument is a new Object[] array
59+
if (index == mi.getArguments().size() - 1 && lastArg instanceof J.NewArray) {
60+
// Verify it's an Object[] array
61+
J.NewArray arrayArg = (J.NewArray) lastArg;
62+
if (arrayArg.getType() instanceof JavaType.Array &&
63+
TypeUtils.isObject(((JavaType.Array) arrayArg.getType()).getElemType())) {
64+
// Only make changes if the method has a varargs parameter
65+
if (mi.getMethodType() == null || mi.getMethodType().hasFlags(Flag.Varargs)) {
66+
List<Expression> arrayElements = arrayArg.getInitializer();
67+
if (arrayElements == null || arrayElements.isEmpty() || arrayElements.get(0) instanceof J.Empty) {
68+
return null; // Remove empty array argument
69+
}
70+
return ListUtils.mapFirst(arrayElements, first -> first.withPrefix(lastArg.getPrefix()));
71+
}
72+
}
73+
}
74+
return lastArg;
75+
}));
76+
}
77+
return mi;
78+
}
79+
});
80+
}
81+
}

src/main/java/org/openrewrite/java/logging/slf4j/JulParameterizedArguments.java

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,21 @@
2020
import org.openrewrite.Preconditions;
2121
import org.openrewrite.Recipe;
2222
import org.openrewrite.TreeVisitor;
23+
import org.openrewrite.internal.ListUtils;
2324
import org.openrewrite.java.JavaIsoVisitor;
24-
import org.openrewrite.java.JavaParser;
2525
import org.openrewrite.java.JavaTemplate;
2626
import org.openrewrite.java.MethodMatcher;
27+
import org.openrewrite.java.logging.ArgumentArrayToVarargs;
2728
import org.openrewrite.java.search.UsesMethod;
2829
import org.openrewrite.java.tree.*;
29-
import org.openrewrite.marker.Markers;
3030

31-
import java.util.ArrayList;
32-
import java.util.List;
33-
import java.util.Objects;
31+
import java.util.*;
3432
import java.util.regex.Matcher;
3533
import java.util.regex.Pattern;
3634

3735
import static java.util.Collections.emptyList;
38-
import static java.util.Collections.singletonList;
39-
import static org.openrewrite.Tree.randomId;
36+
import static java.util.Objects.requireNonNull;
37+
import static java.util.stream.Collectors.toList;
4038

4139
public class JulParameterizedArguments extends Recipe {
4240
private static final MethodMatcher METHOD_MATCHER_PARAM = new MethodMatcher("java.util.logging.Logger log(java.util.logging.Level, java.lang.String, java.lang.Object)");
@@ -85,41 +83,56 @@ public static boolean isStringLiteral(Expression expression) {
8583
return null;
8684
}
8785

88-
private static J.Literal buildStringLiteral(String string) {
89-
return new J.Literal(randomId(), Space.EMPTY, Markers.EMPTY, string, String.format("\"%s\"", string), null, JavaType.Primitive.String);
90-
}
91-
9286
@Override
9387
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
9488
if (METHOD_MATCHER_ARRAY.matches(method) || METHOD_MATCHER_PARAM.matches(method)) {
9589
List<Expression> originalArguments = method.getArguments();
9690

9791
Expression levelArgument = originalArguments.get(0);
9892
Expression messageArgument = originalArguments.get(1);
93+
Expression stringFormatArgument = originalArguments.get(2);
9994

10095
if (!(levelArgument instanceof J.FieldAccess || levelArgument instanceof J.Identifier) ||
101-
!isStringLiteral(messageArgument)) {
96+
!isStringLiteral(messageArgument)) {
10297
return method;
10398
}
10499
String newName = getMethodIdentifier(levelArgument);
105-
if(newName == null) {
100+
if (newName == null) {
106101
return method;
107102
}
108103
maybeRemoveImport("java.util.logging.Level");
109104

110-
String originalFormatString = Objects.requireNonNull((String) ((J.Literal) messageArgument).getValue());
105+
String originalFormatString = requireNonNull((String) ((J.Literal) messageArgument).getValue());
111106
List<Integer> originalIndices = originalLoggedArgumentIndices(originalFormatString);
112-
List<Expression> originalParameters = originalParameters(originalArguments.get(2));
113-
114-
List<Expression> targetArguments = new ArrayList<>(2);
115-
targetArguments.add(buildStringLiteral(originalFormatString.replaceAll("\\{\\d*}", "{}")));
116-
originalIndices.forEach(i -> targetArguments.add(originalParameters.get(i)));
117-
return JavaTemplate.builder(createTemplateString(newName, targetArguments))
118-
.contextSensitive()
119-
.javaParser(JavaParser.fromJavaVersion()
120-
.classpathFromResources(ctx, "slf4j-api-2.1.+"))
107+
108+
Expression updatedStringFormatArgument = stringFormatArgument;
109+
if (stringFormatArgument instanceof J.NewArray) {
110+
J.NewArray newArray = (J.NewArray) stringFormatArgument;
111+
List<Expression> arrayContent = newArray.getInitializer() == null ? emptyList() : newArray.getInitializer();
112+
updatedStringFormatArgument = newArray
113+
.withInitializer(originalIndices.stream().map(arrayContent::get).collect(toList()))
114+
// Also unpack `new String[]{ ... }`, as `ArgumentArrayToVarargs` requires `Object[]`
115+
.withType(((JavaType.Array) requireNonNull(newArray.getType())).withElemType(JavaType.ShallowClass.build("java.lang.Object")));
116+
}
117+
118+
J.MethodInvocation updatedMi = JavaTemplate.builder(newName + "(\"#{}\",#{anyArray(Object)})")
121119
.build()
122-
.apply(getCursor(), method.getCoordinates().replaceMethod(), targetArguments.toArray());
120+
.apply(
121+
getCursor(),
122+
method.getCoordinates().replaceMethod(),
123+
originalFormatString.replaceAll("\\{\\d*}", "{}"),
124+
updatedStringFormatArgument
125+
);
126+
127+
// In case of logger.log(Level.INFO, "Hello {0}, {0}", "foo")
128+
if (!(stringFormatArgument instanceof J.NewArray) && originalIndices.size() > 1) {
129+
return updatedMi.withArguments(ListUtils.concatAll(updatedMi.getArguments(), Collections.nCopies(originalIndices.size() - 1, updatedStringFormatArgument)));
130+
}
131+
// Delegate to ArgumentArrayToVarargs to convert the array argument to varargs
132+
doAfterVisit(new ArgumentArrayToVarargs().getVisitor());
133+
Set<Flag> flags = new HashSet<>(requireNonNull(updatedMi.getMethodType()).getFlags());
134+
flags.add(Flag.Varargs);
135+
return updatedMi.withMethodType(updatedMi.getMethodType().withFlags(flags));
123136
}
124137
return super.visitMethodInvocation(method, ctx);
125138
}
@@ -133,22 +146,5 @@ private List<Integer> originalLoggedArgumentIndices(String strFormat) {
133146
}
134147
return loggedArgumentIndices;
135148
}
136-
137-
private static List<Expression> originalParameters(Expression logParameters) {
138-
if (logParameters instanceof J.NewArray) {
139-
final List<Expression> initializer = ((J.NewArray) logParameters).getInitializer();
140-
if (initializer == null || initializer.isEmpty()) {
141-
return emptyList();
142-
}
143-
return initializer;
144-
}
145-
return singletonList(logParameters);
146-
}
147-
148-
private static String createTemplateString(String newName, List<Expression> targetArguments) {
149-
List<String> targetArgumentsStrings = new ArrayList<>();
150-
targetArguments.forEach(targetArgument -> targetArgumentsStrings.add("#{any()}"));
151-
return newName + '(' + String.join(",", targetArgumentsStrings) + ')';
152-
}
153149
}
154150
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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+
---
17+
type: specs.openrewrite.org/v1beta/recipe
18+
name: org.openrewrite.java.logging.jboss.JBossLoggingBestPractices
19+
displayName: JBoss Logging Best Practices
20+
description: |-
21+
This recipe applies best practices for logging in JBoss applications.
22+
It includes converting argument arrays to varargs for better readability and performance.
23+
tags:
24+
- logging
25+
- jboss
26+
recipeList:
27+
- org.openrewrite.java.logging.jboss.FormattedArgumentsToVMethodRecipes
28+
- org.openrewrite.java.logging.ArgumentArrayToVarargs

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ tags:
138138
- logging
139139
- slf4j
140140
recipeList:
141+
- org.openrewrite.java.logging.ArgumentArrayToVarargs
141142
- org.openrewrite.java.logging.slf4j.LoggersNamedForEnclosingClass
142143
- org.openrewrite.java.logging.slf4j.Slf4jLogShouldBeConstant
143144
- org.openrewrite.java.logging.slf4j.ParameterizedLogging

0 commit comments

Comments
 (0)