Apply do no harm to ParameterizedLogging by not changing statements with exceptions

src/main/java/org/openrewrite/java/logging/ConvertLoggingExceptionCastToToString.java

@@ -0,0 +1,136 @@
+/*
+ * Copyright 2024 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 lombok.EqualsAndHashCode;
+import lombok.Value;
+import org.openrewrite.Cursor;
+import org.openrewrite.ExecutionContext;
+import org.openrewrite.Option;
+import org.openrewrite.Recipe;
+import org.openrewrite.TreeVisitor;
+import org.openrewrite.java.JavaIsoVisitor;
+import org.openrewrite.java.JavaTemplate;
+import org.openrewrite.java.MethodMatcher;
+import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.TypeUtils;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+@EqualsAndHashCode(callSuper = false)
+@Value
+public class ConvertLoggingExceptionCastToToString extends Recipe {
+
+    @Option(displayName = "Method pattern",
+            description = "A method pattern to find matching logging statements to update.",
+            example = "org.slf4j.Logger debug(..)")
+    String methodPattern;
+
+    @Override
+    public String getDisplayName() {
+        return "Convert Logging exception cast to toString() call";
+    }
+
+    @Override
+    public String getDescription() {
+        //language=markdown
+        return "Converts `(Object) exception` casts in logging statements to `exception.toString()` calls. " +
+               "This is more explicit about the intent to log the string representation of the exception " +
+               "rather than relying on implicit toString() conversion through Object casting.";
+    }
+
+    @Override
+    public Set<String> getTags() {
+        return new HashSet<>(Arrays.asList("Logging", "RSPEC-S1905"));
+    }
+
+    @Override
+    public TreeVisitor<?, ExecutionContext> getVisitor() {
+        return new JavaIsoVisitor<ExecutionContext>() {
+            private final MethodMatcher methodMatcher = new MethodMatcher(methodPattern, true);
+
+            @Override
+            public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
+                J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
+
+                // Check if this matches our target method pattern
+                if (!methodMatcher.matches(m)) {
+                    return m;
+                }
+
+                // Check if there are any Object casts of Throwables to replace
+                boolean hasObjectCastsToReplace = m.getArguments().stream().anyMatch(arg -> {
+                    if (arg instanceof J.TypeCast) {
+                        J.TypeCast cast = (J.TypeCast) arg;
+                        return cast.getType() != null &&
+                               TypeUtils.isOfClassType(cast.getType(), "java.lang.Object") &&
+                               cast.getExpression() != null &&
+                               TypeUtils.isAssignableTo("java.lang.Throwable", cast.getExpression().getType());
+                    }
+                    return false;
+                });
+
+                // If there are no casts to replace, return the method as-is
+                if (!hasObjectCastsToReplace) {
+                    return m;
+                }
+
+                // Use a JavaTemplate to properly replace the arguments
+                JavaTemplate template = JavaTemplate.builder(m.getArguments().stream()
+                        .map(arg -> {
+                            if (arg instanceof J.TypeCast) {
+                                J.TypeCast cast = (J.TypeCast) arg;
+                                if (cast.getType() != null &&
+                                    TypeUtils.isOfClassType(cast.getType(), "java.lang.Object") &&
+                                    cast.getExpression() != null &&
+                                    TypeUtils.isAssignableTo("java.lang.Throwable", cast.getExpression().getType())) {
+                                    return "#{any(java.lang.Throwable)}.toString()";
+                                }
+                            }
+                            return "#{any()}";
+                        })
+                        .reduce((a, b) -> a + ", " + b)
+                        .orElse(""))
+                        .build();
+
+                // Prepare the arguments for the template
+                Object[] templateArgs = m.getArguments().stream()
+                        .map(arg -> {
+                            if (arg instanceof J.TypeCast) {
+                                J.TypeCast cast = (J.TypeCast) arg;
+                                if (cast.getType() != null &&
+                                    TypeUtils.isOfClassType(cast.getType(), "java.lang.Object") &&
+                                    cast.getExpression() != null &&
+                                    TypeUtils.isAssignableTo("java.lang.Throwable", cast.getExpression().getType())) {
+                                    return cast.getExpression();
+                                }
+                            }
+                            return arg;
+                        })
+                        .toArray();
+
+                // Apply the template
+                return (J.MethodInvocation) template.apply(
+                                new Cursor(getCursor().getParent(), m),
+                                m.getCoordinates().replaceArguments(),
+                                templateArgs);
+            }
+        };
+    }
+}

src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java

@@ -110,6 +110,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
                             }
                         }
 
+                        // Determine if we should cast throwable to Object (only when it's the sole argument)
+                        final boolean shouldCastThrowable = regularArgs.isEmpty() && possibleThrowable == null;
+
                         // Build the message template
                         ListUtils.map(m.getArguments(), (index, message) -> {
                             if (index > 0) {
@@ -120,9 +123,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
                                 MessageAndArguments literalAndArgs = concatenationToLiteral(message, new MessageAndArguments("", new ArrayList<>()));
                                 messageBuilder.append(literalAndArgs.message);
                                 messageBuilder.append("\"");
-                                // Cast Throwables to Object to preserve toString() behavior
+                                // Cast Throwables to Object to preserve toString() behavior, but only when it's the sole argument
                                 literalAndArgs.arguments.forEach(arg -> messageBuilder.append(
-                                        TypeUtils.isAssignableTo("java.lang.Throwable", arg.getType()) ?
+                                        TypeUtils.isAssignableTo("java.lang.Throwable", arg.getType()) &&
+                                        literalAndArgs.arguments.size() == 1 && shouldCastThrowable ?
                                                 ", (Object) #{any()}" :
                                                 ", #{any()}"));
                             } else {

src/test/java/org/openrewrite/java/logging/ConvertLoggingExceptionCastToToStringTest.java

@@ -0,0 +1,195 @@
+/*
+ * Copyright 2024 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;
+
+class ConvertLoggingExceptionCastToToStringTest implements RewriteTest {
+
+    @Override
+    public void defaults(RecipeSpec spec) {
+        spec.parser(JavaParser.fromJavaVersion()
+          .classpathFromResources(new InMemoryExecutionContext(), "slf4j-api-2.1.+", "log4j-api-2.+", "log4j-core-2.+"));
+    }
+
+    @DocumentExample
+    @Test
+    void convertThrowableCastToToString() {
+        rewriteRun(
+          spec -> spec.recipe(new ConvertLoggingExceptionCastToToString("org.slf4j.Logger debug(..)")),
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void asInteger(Logger logger, String numberString) {
+                      try {
+                          Integer i = Integer.valueOf(numberString);
+                      } catch (NumberFormatException ex) {
+                          logger.debug("some big error: {}", (Object) ex);
+                      }
+                  }
+              }
+              """,
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void asInteger(Logger logger, String numberString) {
+                      try {
+                          Integer i = Integer.valueOf(numberString);
+                      } catch (NumberFormatException ex) {
+                          logger.debug("some big error: {}", ex.toString());
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void convertMultipleThrowableCasts() {
+        rewriteRun(
+          spec -> spec.recipe(new ConvertLoggingExceptionCastToToString("org.slf4j.Logger info(..)")),
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void method(Logger logger, Exception e1, RuntimeException e2) {
+                      logger.info("Errors: {} and {}", (Object) e1, (Object) e2);
+                  }
+              }
+              """,
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void method(Logger logger, Exception e1, RuntimeException e2) {
+                      logger.info("Errors: {} and {}", e1.toString(), e2.toString());
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void doNotChangeNonThrowableCasts() {
+        rewriteRun(
+          spec -> spec.recipe(new ConvertLoggingExceptionCastToToString("org.slf4j.Logger info(..)")),
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void method(Logger logger, String str) {
+                      logger.info("Value: {}", (Object) str);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void doNotChangeNonObjectCasts() {
+        rewriteRun(
+          spec -> spec.recipe(new ConvertLoggingExceptionCastToToString("org.slf4j.Logger info(..)")),
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void method(Logger logger, Exception ex) {
+                      logger.info("Error: {}", (Throwable) ex);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void workWithMarkers() {
+        rewriteRun(
+          spec -> spec.recipe(new ConvertLoggingExceptionCastToToString("org.slf4j.Logger info(..)")),
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+              import org.slf4j.Marker;
+
+              class Test {
+                  static void method(Logger logger, Marker marker, Exception ex) {
+                      logger.info(marker, "Error occurred: {}", (Object) ex);
+                  }
+              }
+              """,
+            """
+              import org.slf4j.Logger;
+              import org.slf4j.Marker;
+
+              class Test {
+                  static void method(Logger logger, Marker marker, Exception ex) {
+                      logger.info(marker, "Error occurred: {}", ex.toString());
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void workWithLog4j() {
+        rewriteRun(
+          spec -> spec.recipe(new ConvertLoggingExceptionCastToToString("org.apache.logging.log4j.Logger error(..)")),
+          //language=java
+          java(
+            """
+              import org.apache.logging.log4j.Logger;
+
+              class Test {
+                  static void method(Logger logger, Exception ex) {
+                      logger.error("Failed: {}", (Object) ex);
+                  }
+              }
+              """,
+            """
+              import org.apache.logging.log4j.Logger;
+
+              class Test {
+                  static void method(Logger logger, Exception ex) {
+                      logger.error("Failed: {}", ex.toString());
+                  }
+              }
+              """
+          )
+        );
+    }
+}

src/test/java/org/openrewrite/java/logging/ParameterizedLoggingTest.java

@@ -260,6 +260,42 @@ static void asInteger(Logger logger, String numberString) {
         );
     }
 
+    @Test
+    void exceptionArgumentsWithOtherParametersNoCast() {
+        rewriteRun(
+          spec -> spec.recipe(new ParameterizedLogging("org.slf4j.Logger debug(..)", false)),
+          //language=java
+          java(
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void asInteger(Logger logger, String numberString, String context) {
+                      try {
+                          Integer i = Integer.valueOf(numberString);
+                      } catch (NumberFormatException ex) {
+                          logger.debug("Error in context " + context + ": " + ex);
+                      }
+                  }
+              }
+              """,
+            """
+              import org.slf4j.Logger;
+
+              class Test {
+                  static void asInteger(Logger logger, String numberString, String context) {
+                      try {
+                          Integer i = Integer.valueOf(numberString);
+                      } catch (NumberFormatException ex) {
+                          logger.debug("Error in context {}: {}", context, ex);
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/38")
     @Test
     void indexOutOfBoundsExceptionOnParseMethodArguments() {