diff --git a/src/main/java/org/openrewrite/java/logging/ArgumentArrayToVarargs.java b/src/main/java/org/openrewrite/java/logging/ArgumentArrayToVarargs.java new file mode 100644 index 00000000..229578b5 --- /dev/null +++ b/src/main/java/org/openrewrite/java/logging/ArgumentArrayToVarargs.java @@ -0,0 +1,81 @@ +/* + * 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; + +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 "Unpack Logger method `new Object[] {...}` into varargs"; + } + + @Override + public String getDescription() { + return "For Logger methods that support varargs, convert any final explicit `Object[]` arguments into their unpacked values."; + } + + @Override + public Duration getEstimatedEffortPerOccurrence() { + return Duration.ofMinutes(2); + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check(new UsesMethod<>(LOGGER_METHOD), new JavaIsoVisitor() { + @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 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; + } + }); + } +} diff --git a/src/main/java/org/openrewrite/java/logging/slf4j/JulParameterizedArguments.java b/src/main/java/org/openrewrite/java/logging/slf4j/JulParameterizedArguments.java index 0b76242d..624c2624 100644 --- a/src/main/java/org/openrewrite/java/logging/slf4j/JulParameterizedArguments.java +++ b/src/main/java/org/openrewrite/java/logging/slf4j/JulParameterizedArguments.java @@ -20,23 +20,21 @@ 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.JavaParser; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.logging.ArgumentArrayToVarargs; import org.openrewrite.java.search.UsesMethod; import org.openrewrite.java.tree.*; -import org.openrewrite.marker.Markers; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; +import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; import static java.util.Collections.emptyList; -import static java.util.Collections.singletonList; -import static org.openrewrite.Tree.randomId; +import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.toList; public class JulParameterizedArguments extends Recipe { 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,10 +83,6 @@ public static boolean isStringLiteral(Expression expression) { return null; } - private static J.Literal buildStringLiteral(String string) { - return new J.Literal(randomId(), Space.EMPTY, Markers.EMPTY, string, String.format("\"%s\"", string), null, JavaType.Primitive.String); - } - @Override public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { if (METHOD_MATCHER_ARRAY.matches(method) || METHOD_MATCHER_PARAM.matches(method)) { @@ -96,30 +90,49 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu Expression levelArgument = originalArguments.get(0); Expression messageArgument = originalArguments.get(1); + Expression stringFormatArgument = originalArguments.get(2); if (!(levelArgument instanceof J.FieldAccess || levelArgument instanceof J.Identifier) || - !isStringLiteral(messageArgument)) { + !isStringLiteral(messageArgument)) { return method; } String newName = getMethodIdentifier(levelArgument); - if(newName == null) { + if (newName == null) { return method; } maybeRemoveImport("java.util.logging.Level"); - String originalFormatString = Objects.requireNonNull((String) ((J.Literal) messageArgument).getValue()); + String originalFormatString = requireNonNull((String) ((J.Literal) messageArgument).getValue()); List originalIndices = originalLoggedArgumentIndices(originalFormatString); - List originalParameters = originalParameters(originalArguments.get(2)); - - List targetArguments = new ArrayList<>(2); - targetArguments.add(buildStringLiteral(originalFormatString.replaceAll("\\{\\d*}", "{}"))); - originalIndices.forEach(i -> targetArguments.add(originalParameters.get(i))); - return JavaTemplate.builder(createTemplateString(newName, targetArguments)) - .contextSensitive() - .javaParser(JavaParser.fromJavaVersion() - .classpathFromResources(ctx, "slf4j-api-2.1.+")) + + Expression updatedStringFormatArgument = stringFormatArgument; + if (stringFormatArgument instanceof J.NewArray) { + J.NewArray newArray = (J.NewArray) stringFormatArgument; + List arrayContent = newArray.getInitializer() == null ? emptyList() : newArray.getInitializer(); + updatedStringFormatArgument = newArray + .withInitializer(originalIndices.stream().map(arrayContent::get).collect(toList())) + // Also unpack `new String[]{ ... }`, as `ArgumentArrayToVarargs` requires `Object[]` + .withType(((JavaType.Array) requireNonNull(newArray.getType())).withElemType(JavaType.ShallowClass.build("java.lang.Object"))); + } + + J.MethodInvocation updatedMi = JavaTemplate.builder(newName + "(\"#{}\",#{anyArray(Object)})") .build() - .apply(getCursor(), method.getCoordinates().replaceMethod(), targetArguments.toArray()); + .apply( + getCursor(), + method.getCoordinates().replaceMethod(), + originalFormatString.replaceAll("\\{\\d*}", "{}"), + updatedStringFormatArgument + ); + + // In case of logger.log(Level.INFO, "Hello {0}, {0}", "foo") + if (!(stringFormatArgument instanceof J.NewArray) && originalIndices.size() > 1) { + return updatedMi.withArguments(ListUtils.concatAll(updatedMi.getArguments(), Collections.nCopies(originalIndices.size() - 1, updatedStringFormatArgument))); + } + // Delegate to ArgumentArrayToVarargs to convert the array argument to varargs + doAfterVisit(new ArgumentArrayToVarargs().getVisitor()); + Set flags = new HashSet<>(requireNonNull(updatedMi.getMethodType()).getFlags()); + flags.add(Flag.Varargs); + return updatedMi.withMethodType(updatedMi.getMethodType().withFlags(flags)); } return super.visitMethodInvocation(method, ctx); } @@ -133,22 +146,5 @@ private List originalLoggedArgumentIndices(String strFormat) { } return loggedArgumentIndices; } - - private static List originalParameters(Expression logParameters) { - if (logParameters instanceof J.NewArray) { - final List initializer = ((J.NewArray) logParameters).getInitializer(); - if (initializer == null || initializer.isEmpty()) { - return emptyList(); - } - return initializer; - } - return singletonList(logParameters); - } - - private static String createTemplateString(String newName, List targetArguments) { - List targetArgumentsStrings = new ArrayList<>(); - targetArguments.forEach(targetArgument -> targetArgumentsStrings.add("#{any()}")); - return newName + '(' + String.join(",", targetArgumentsStrings) + ')'; - } } } diff --git a/src/main/resources/META-INF/rewrite/jboss.yml b/src/main/resources/META-INF/rewrite/jboss.yml new file mode 100644 index 00000000..e3f0c929 --- /dev/null +++ b/src/main/resources/META-INF/rewrite/jboss.yml @@ -0,0 +1,28 @@ +# +# 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. +# +--- +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 diff --git a/src/main/resources/META-INF/rewrite/slf4j.yml b/src/main/resources/META-INF/rewrite/slf4j.yml index 86907ff2..8f5ca64d 100644 --- a/src/main/resources/META-INF/rewrite/slf4j.yml +++ b/src/main/resources/META-INF/rewrite/slf4j.yml @@ -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 diff --git a/src/test/java/org/openrewrite/java/logging/ArgumentArrayToVarargsTest.java b/src/test/java/org/openrewrite/java/logging/ArgumentArrayToVarargsTest.java new file mode 100644 index 00000000..eed88448 --- /dev/null +++ b/src/test/java/org/openrewrite/java/logging/ArgumentArrayToVarargsTest.java @@ -0,0 +1,193 @@ +/* + * 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; + +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.+")); + } + + @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() { + 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}); + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/java/logging/jboss/JBossLoggingBestPracticesTest.java b/src/test/java/org/openrewrite/java/logging/jboss/JBossLoggingBestPracticesTest.java new file mode 100644 index 00000000..b74710e4 --- /dev/null +++ b/src/test/java/org/openrewrite/java/logging/jboss/JBossLoggingBestPracticesTest.java @@ -0,0 +1,61 @@ +/* + * 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.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); + } + } + """ + ) + ); + } +}