Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* 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
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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;

import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.*;

import java.time.Duration;
import java.util.List;

public class ArgumentArrayToVarargs extends Recipe {
// Match logger methods that end with Object[] - but we'll verify if it's varargs later
private static final MethodMatcher LOGGER_METHOD = new MethodMatcher("*..*Log* *(.., Object[])");

@Override
public String getDisplayName() {
return "Convert argument array to varargs";
}

@Override
public String getDescription() {
return "Converts method calls that use an array of arguments to use varargs instead.";
}

@Override
public Duration getEstimatedEffortPerOccurrence() {
return Duration.ofMinutes(2);
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(new UsesMethod<>(LOGGER_METHOD), new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
if (LOGGER_METHOD.matches(mi)) {
return mi.withArguments(ListUtils.flatMap(mi.getArguments(), (index, lastArg) -> {
// Check if the last argument is a new Object[] array
if (index == mi.getArguments().size() - 1 && lastArg instanceof J.NewArray) {
// Verify it's an Object[] array
J.NewArray arrayArg = (J.NewArray) lastArg;
if (arrayArg.getType() instanceof JavaType.Array &&
TypeUtils.isObject(((JavaType.Array) arrayArg.getType()).getElemType())) {
// Only make changes if the method has a varargs parameter
if (mi.getMethodType() == null || mi.getMethodType().hasFlags(Flag.Varargs)) {
List<Expression> arrayElements = arrayArg.getInitializer();
if (arrayElements == null || arrayElements.isEmpty() || arrayElements.get(0) instanceof J.Empty) {
return null; // Remove empty array argument
}
return ListUtils.mapFirst(arrayElements, first -> first.withPrefix(lastArg.getPrefix()));
}
}
}
return lastArg;
}));
}
return mi;
}
});
}
}
28 changes: 28 additions & 0 deletions src/main/resources/META-INF/rewrite/jboss.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#
# Copyright 2025 the original author or authors.
# <p>
# 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
# <p>
# https://docs.moderne.io/licensing/moderne-source-available-license
# <p>
# 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.
#
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.logging.jboss.JBossLoggingBestPractices
displayName: JBoss Logging Best Practices
description: |-
This recipe applies best practices for logging in JBoss applications.
It includes converting argument arrays to varargs for better readability and performance.
tags:
- logging
- jboss
recipeList:
- org.openrewrite.java.logging.jboss.FormattedArgumentsToVMethodRecipes
- org.openrewrite.java.logging.ArgumentArrayToVarargs
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 @@ -138,6 +138,7 @@ tags:
- logging
- slf4j
recipeList:
- org.openrewrite.java.logging.ArgumentArrayToVarargs
- org.openrewrite.java.logging.slf4j.LoggersNamedForEnclosingClass
- org.openrewrite.java.logging.slf4j.Slf4jLogShouldBeConstant
- org.openrewrite.java.logging.slf4j.ParameterizedLogging
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* 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
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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;

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

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

@SuppressWarnings("RedundantArrayCreation")
class ArgumentArrayToVarargsTest implements RewriteTest {

@Override
public void defaults(RecipeSpec spec) {
spec.recipe(new ArgumentArrayToVarargs())
.parser(JavaParser.fromJavaVersion()
.classpathFromResources(new InMemoryExecutionContext(), "slf4j-api-2.1.+"));
Comment on lines +31 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are passing without the extra config

Suggested change
public void defaults(RecipeSpec spec) {
spec.recipe(new ArgumentArrayToVarargs())
.parser(JavaParser.fromJavaVersion()
.classpathFromResources(new InMemoryExecutionContext(), "slf4j-api-2.1.+"));
spec.recipe(new ArgumentArrayToVarargs());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that's a result of a leaky classloader, as we do really need the SLF4J classes on the classpath, which ought not to be present by default. Hence why I'd added them here as that might change in the future.

}

@DocumentExample
@Test
void objectArrayToVarargs() {
rewriteRun(
//language=java
java(
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.info("Message {} {} {}", new Object[]{"old", "style", "args"});
}
}
""",
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.info("Message {} {} {}", "old", "style", "args");
}
}
"""
)
);
}

@Test
void emptyObjectArray() {
rewriteRun(
//language=java
java(
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.info("Message without placeholders", new Object[]{});
}
}
""",
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.info("Message without placeholders");
}
}
"""
)
);
}

@Test
void singleElementArray() {
rewriteRun(
//language=java
java(
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.warn("Single placeholder: {}", new Object[]{"value"});
}
}
""",
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.warn("Single placeholder: {}", "value");
}
}
"""
)
);
}

@SuppressWarnings("ConfusingArgumentToVarargsMethod")
@Test
void nonObjectArrayNotConverted() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not support arrays of any type? slf4j ends up calling .toString() on the arguments no matter their type anyways?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd excluded these as there's a warning in my IDE with this type of usage
image

We're broadly matching logger methods through "*..*Log* *(.., Object[])", and for now I figured safest to only make changes in known-safe places, as opposed to figuring out when this might potentially also be safe.

rewriteRun(
//language=java
java(
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.info("Message {}", new String[]{"test"});
}
}
"""
)
);
}

@Test
void notLastArgumentNotConverted() {
rewriteRun(
//language=java
java(
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
logger.info("Message {} {}", new Object[]{"test"}, "other");
}
}
"""
)
);
}

@Test
void variableArrayNotConverted() {
rewriteRun(
//language=java
java(
"""
import org.slf4j.Logger;
class Test {
Logger logger;
void method() {
Object[] args = {"old", "style", "args"};
logger.info("Message {} {} {}", args);
}
}
"""
)
);
}

@Test
void notVarargsMethodParameterTypeNotConverted() {
rewriteRun(
//language=java
java(
"""
import java.util.logging.Level;
import java.util.logging.Logger;
class Test {
Logger logger;
void method(Level level, String msg, Object o) {
logger.log(level, msg, new Object[]{o});
}
}
"""
)
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* 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
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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.jboss;

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 JBossLoggingBestPracticesTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.recipeFromResource(
"/META-INF/rewrite/jboss.yml",
"org.openrewrite.java.logging.jboss.JBossLoggingBestPractices"
);
}

@DocumentExample
@Test
void convertInfo() {
rewriteRun(
//language=java
java(
"""
import org.jboss.logging.Logger;

class Test {
void test(Logger logger, String msg, Object o) {
logger.info(msg, new Object[]{o});
}
}
""",
"""
import org.jboss.logging.Logger;

class Test {
void test(Logger logger, String msg, Object o) {
logger.infov(msg, o);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's subtle, but this chains back to back two recipes to go from info to infov and from that object array to just the arguments taken from it.

}
}
"""
)
);
}
}