Skip to content

Commit

Permalink
ParameterizedLogging recipe fixed (#162)
Browse files Browse the repository at this point in the history 
* ParameterizedLogging recipe fixed

* Fix compilation error

* Fix argument handling

* Also handle Log4J markers

---------

Co-authored-by: Riyazul555 <riyazulislam2003@gmail.com>
Co-authored-by: Tim te Beek <tim@moderne.io>
  • Loading branch information
@Riyazul555 @MdRiyazulIslam @timtebeek
3 people committed Jul 1, 2024
1 parent b46bf0b commit ec68564
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 11 deletions.
29 changes: 21 additions & 8 deletions src/main/java/org/openrewrite/java/logging/ParameterizedLogging.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,27 +76,33 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
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) {
Expression logMsg = m.getArguments().get(0);
final int logMsgIndex = isMarker(m.getArguments().get(0)) ? 1 : 0;
Expression logMsg = m.getArguments().get(logMsgIndex);
if (logMsg instanceof J.Binary) {
StringBuilder messageBuilder = new StringBuilder("\"");
StringBuilder messageBuilder = new StringBuilder();
List<Expression> newArgList = new ArrayList<>();
ListUtils.map(m.getArguments(), (index, message) -> {
if (index == 0 && message instanceof J.Binary) {
if (index > 0) {
messageBuilder.append(", ");
}
if (index == logMsgIndex && message instanceof J.Binary) {
messageBuilder.append("\"");
MessageAndArguments literalAndArgs = concatenationToLiteral(message, new MessageAndArguments("", new ArrayList<>()));
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;
});
messageBuilder.append("\"");
newArgList.forEach(arg -> messageBuilder.append(", #{any()}"));
m = JavaTemplate.builder(escapeDollarSign(messageBuilder.toString()))
.contextSensitive()
.build()
.apply(new Cursor(getCursor().getParent(), m), m.getCoordinates().replaceArguments(), newArgList.toArray());
} else if (logMsg instanceof J.Identifier && TypeUtils.isAssignableTo("java.lang.Throwable", logMsg.getType()) ) {
} else if (logMsg instanceof J.Identifier && TypeUtils.isAssignableTo("java.lang.Throwable", logMsg.getType())) {
return m;
} else if (!TypeUtils.isString(logMsg.getType()) && logMsg.getType() instanceof JavaType.Class) {
StringBuilder messageBuilder = new StringBuilder("\"{}\"");
Expand All @@ -112,15 +118,22 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
}

// Avoid changing reference if the templating didn't actually change the contents of the method
if(m != method && m.print(getCursor()).equals(method.print(getCursor()))) {
if (m != method && m.print(getCursor()).equals(method.print(getCursor()))) {
return method;
}
return m;
}

private boolean isMarker(Expression expression) {
JavaType expressionType = expression.getType();
return TypeUtils.isAssignableTo("org.slf4j.Marker", expressionType) ||
TypeUtils.isAssignableTo("org.apache.logging.log4j.Marker", expressionType);
}

class RemoveToStringVisitor extends JavaVisitor<ExecutionContext> {
private final JavaTemplate t = JavaTemplate.builder("#{any(java.lang.String)}").build();
private final MethodMatcher TO_STRING = new MethodMatcher("java.lang.Object toString()");

@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
if (getCursor().getNearestMessage("DO_NOT_REMOVE", Boolean.FALSE)) {
Expand Down Expand Up @@ -173,7 +186,7 @@ private static MessageAndArguments concatenationToLiteral(Expression message, Me
} else if (concat.getRight() instanceof J.Literal) {
J.Literal right = (J.Literal) concat.getRight();
boolean rightIsStringLiteral = right.getType() == JavaType.Primitive.String;
if(result.previousMessageWasStringLiteral && rightIsStringLiteral) {
if (result.previousMessageWasStringLiteral && rightIsStringLiteral) {
result.message += "\" +" + right.getPrefix().getWhitespace() + "\"" + getLiteralValue(right);
} else {
result.message += getLiteralValue(right);
Expand Down
37 changes: 34 additions & 3 deletions src/test/java/org/openrewrite/java/logging/ParameterizedLoggingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ParameterizedLoggingTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.parser(JavaParser.fromJavaVersion()
.classpathFromResources(new InMemoryExecutionContext(), "slf4j-api-2.1", "log4j-api-2.23", "log4j-core-2.23"));;
.classpathFromResources(new InMemoryExecutionContext(), "slf4j-api-2.1", "log4j-api-2.23", "log4j-core-2.23"));
}

@DocumentExample
Expand Down Expand Up @@ -569,7 +569,7 @@ void multilineStringLiteralLeftAlone() {
java(
"""
import org.slf4j.Logger;
class Test {
static void method(Logger logger) {
String one = "one";
Expand All @@ -581,7 +581,7 @@ static void method(Logger logger) {
""",
"""
import org.slf4j.Logger;
class Test {
static void method(Logger logger) {
String one = "one";
Expand All @@ -594,4 +594,35 @@ static void method(Logger logger) {
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/159")
void concatenationWithMarker() {
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 name) {
logger.info(marker, "Hello " + name + ", nice to meet you " + name);
}
}
""",
"""
import org.slf4j.Logger;
import org.slf4j.Marker;
class Test {
static void method(Logger logger, Marker marker, String name) {
logger.info(marker, "Hello {}, nice to meet you {}", name, name);
}
}
"""
)
);
}
}

0 comments on commit ec68564

Please sign in to comment.