diff --git a/src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java b/src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java index 1a679e2..8f14f60 100644 --- a/src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java +++ b/src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java @@ -110,6 +110,15 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } } + // Check if any of the concatenation arguments is a throwable + // If so, skip parameterization to preserve exception handling behavior + boolean hasThrowableInConcatenation = concatenationArgs.stream() + .anyMatch(arg -> TypeUtils.isAssignableTo("java.lang.Throwable", arg.getType())); + + if (hasThrowableInConcatenation) { + return m; // Skip parameterization when throwables are concatenated + } + // Build the message template ListUtils.map(m.getArguments(), (index, message) -> { if (index > 0) { @@ -120,11 +129,7 @@ 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 - literalAndArgs.arguments.forEach(arg -> messageBuilder.append( - TypeUtils.isAssignableTo("java.lang.Throwable", arg.getType()) ? - ", (Object) #{any()}" : - ", #{any()}")); + literalAndArgs.arguments.forEach(arg -> messageBuilder.append(", #{any()}")); } else { messageBuilder.append("#{any()}"); } diff --git a/src/test/java/org/openrewrite/java/logging/ParameterizedLoggingTest.java b/src/test/java/org/openrewrite/java/logging/ParameterizedLoggingTest.java index 240a755..4ebba3c 100644 --- a/src/test/java/org/openrewrite/java/logging/ParameterizedLoggingTest.java +++ b/src/test/java/org/openrewrite/java/logging/ParameterizedLoggingTest.java @@ -242,16 +242,49 @@ static void asInteger(Logger logger, String numberString) { } } } - """, + """ + ) + ); + } + + @Test + void exceptionArgumentsWithMultiCatch() { + 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) { + static void parseValue(Logger logger, String numberString) { + try { + Integer i = Integer.valueOf(numberString); + } catch (NumberFormatException | IllegalArgumentException ex) { + logger.debug("parsing error: " + ex); + } + } + } + """ + ) + ); + } + + @Test + void exceptionArgumentsWithOtherParametersNoChange() { + 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("some big error: {}", (Object) ex); + logger.debug("Error in context " + context + ": " + ex); } } } diff --git a/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java b/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java index 1f4b055..5e299b4 100644 --- a/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java +++ b/src/test/java/org/openrewrite/java/logging/slf4j/Slf4jBestPracticesTest.java @@ -110,7 +110,7 @@ void test() { try { throw new IllegalStateException("oops"); } catch (Exception e) { - logger.error("aaa: {}", (Object) e); + logger.error("aaa: " + e); logger.error("bbb: {}", String.valueOf(e)); logger.error("ccc: {}", e.toString()); }