Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -120,11 +129,8 @@ 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()}"));
// Add arguments without casting throwables
Comment thread
timtebeek marked this conversation as resolved.
Outdated
literalAndArgs.arguments.forEach(arg -> messageBuilder.append(", #{any()}"));
} else {
messageBuilder.append("#{any()}");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 exceptionArgumentsWithOtherParametersNoCast() {
Comment thread
timtebeek marked this conversation as resolved.
Outdated
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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down