Skip to content

Handle existing arguments alongside concatenation in ParameterizedLogging #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timtebeek merged 2 commits into from
Jul 17, 2025
Merged

Handle existing arguments alongside concatenation in ParameterizedLogging #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b4da49
Handle existing arguments alongside concatenation in `ParameterizedLo…
timtebeek Jul 17, 2025
7f2482e
Trim empty lines
timtebeek Jul 17, 2025
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
35 changes: 32 additions & 3 deletions src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,35 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
if (matcher.matches(m) && !m.getArguments().isEmpty() && !(m.getArguments().get(0) instanceof J.Empty) && m.getArguments().size() <= 2) {
if (matcher.matches(m) && !m.getArguments().isEmpty() && !(m.getArguments().get(0) instanceof J.Empty)) {
final int logMsgIndex = isMarker(m.getArguments().get(0)) ? 1 : 0;
// Only process if we have at most 2 arguments after accounting for marker
if (m.getArguments().size() - logMsgIndex > 2) {
return m;
}
Expression logMsg = m.getArguments().get(logMsgIndex);
if (logMsg instanceof J.Binary) {
StringBuilder messageBuilder = new StringBuilder();
List<Expression> newArgList = new ArrayList<>();
List<Expression> concatenationArgs = new ArrayList<>();
List<Expression> regularArgs = new ArrayList<>();
Expression possibleThrowable = null;

// First, process all arguments
for (int index = 0; index < m.getArguments().size(); index++) {
Expression arg = m.getArguments().get(index);
if (index == logMsgIndex && arg instanceof J.Binary) {
MessageAndArguments literalAndArgs = concatenationToLiteral(arg, new MessageAndArguments("", new ArrayList<>()));
concatenationArgs.addAll(literalAndArgs.arguments);
} else if (index == m.getArguments().size() - 1 &&
TypeUtils.isAssignableTo("java.lang.Throwable", arg.getType())) {
possibleThrowable = arg;
} else {
regularArgs.add(arg);
}
}

// Build the message template
ListUtils.map(m.getArguments(), (index, message) -> {
if (index > 0) {
messageBuilder.append(", ");
Expand All @@ -97,13 +120,19 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
messageBuilder.append(literalAndArgs.message);
messageBuilder.append("\"");
literalAndArgs.arguments.forEach(arg -> messageBuilder.append(", #{any()}"));
newArgList.addAll(literalAndArgs.arguments);
} else {
messageBuilder.append("#{any()}");
newArgList.add(message);
}
return message;
});

// Assemble arguments in correct order: regular args, concatenation args, throwable (if any)
newArgList.addAll(regularArgs);
newArgList.addAll(concatenationArgs);
if (possibleThrowable != null) {
newArgList.add(possibleThrowable);
}

m = JavaTemplate.builder(escapeDollarSign(messageBuilder.toString()))
.build()
.apply(new Cursor(getCursor().getParent(), m), m.getCoordinates().replaceArguments(), newArgList.toArray());
Expand Down
89 changes: 89 additions & 0 deletions src/test/java/org/openrewrite/java/logging/ParameterizedLoggingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,95 @@ static void method(String name) {
);
}

@Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/236")
@Test
void preserveArgumentOrderWhenMixingConcatenationAndParameters() {
rewriteRun(
spec -> spec.recipe(new ParameterizedLogging("org.slf4j.Logger info(..)", false)),
//language=java
java(
"""
import org.slf4j.Logger;

class Test {
static void method(Logger logger, String esHost, String[] indexPrefix) {
logger.info("Some logging {} with string concatenation: " + String.join(",", indexPrefix), esHost);
}
}
""",
"""
import org.slf4j.Logger;

class Test {
static void method(Logger logger, String esHost, String[] indexPrefix) {
logger.info("Some logging {} with string concatenation: {}", esHost, String.join(",", indexPrefix));
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/236")
@Test
void preserveArgumentOrderWithMarkerAndMixedConcatenation() {
rewriteRun(
spec -> spec.recipe(new ParameterizedLogging("org.slf4j.Logger info(..)", false)),
//language=java
java(
"""
import org.slf4j.Logger;
import org.slf4j.Marker;

class Test {
static void method(Logger logger, Marker marker, String param1, String param2) {
logger.info(marker, "Message with {} and concat: " + param2, param1);
}
}
""",
"""
import org.slf4j.Logger;
import org.slf4j.Marker;

class Test {
static void method(Logger logger, Marker marker, String param1, String param2) {
logger.info(marker, "Message with {} and concat: {}", param1, param2);
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/236")
@Test
void preserveArgumentOrderWithMultipleConcatenations() {
rewriteRun(
spec -> spec.recipe(new ParameterizedLogging("org.slf4j.Logger info(..)", false)),
//language=java
java(
"""
import org.slf4j.Logger;

class Test {
static void method(Logger logger, String existing, String part1, String part2) {
logger.info("Existing param {} with concat: " + part1 + " and " + part2, existing);
}
}
""",
"""
import org.slf4j.Logger;

class Test {
static void method(Logger logger, String existing, String part1, String part2) {
logger.info("Existing param {} with concat: {} and {}", existing, part1, part2);
}
}
"""
)
);
}

@Test
void returnsAnonymousClassThatUsesLoggerInMethodImplementation() {
rewriteRun(
Expand Down