Skip to content

Commit d7fb6d0

Browse files
Laurens-Wtimtebeek
Laurens-W
and
timtebeek
authored
Support switch cases in SystemErrToLogging and ParameterizedLogging
* Add more test cases surrounding switch statements * Drop `firstEnclosing instanceof J.Block` to fix tests again * Further improve case handling * Revert lambda check to how it was * Revert unnecessary cursor change --------- Co-authored-by: Tim te Beek <[email protected]>
1 parent 799c19f commit d7fb6d0

File tree

3 files changed

+166
-8
lines changed

3 files changed

+166
-8
lines changed

‎src/main/java/org/openrewrite/java/logging/SystemErrToLogging.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.openrewrite.marker.Markers;
2929

3030
import java.util.Collections;
31+
import java.util.List;
3132
import java.util.Set;
3233
import java.util.concurrent.atomic.AtomicBoolean;
3334
import java.util.concurrent.atomic.AtomicInteger;
@@ -80,9 +81,23 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
8081
Cursor blockCursor = new Cursor(getCursor().getParent(), b);
8182

8283
AtomicBoolean addedLogger = new AtomicBoolean(false);
83-
AtomicInteger skip = new AtomicInteger(-1);
84+
b = b.withStatements(collapseNextThrowablePrintStackTrace(b.getStatements(), ctx, blockCursor, addedLogger));
85+
return addedLogger.get() ? block : b;
86+
}
8487

85-
b = b.withStatements(ListUtils.map(b.getStatements(), (i, stat) -> {
88+
@Override
89+
public J.Case visitCase(J.Case _case, ExecutionContext ctx) {
90+
J.Case c = super.visitCase(_case, ctx);
91+
Cursor caseCursor = new Cursor(getCursor().getParent(), c);
92+
93+
AtomicBoolean addedLogger = new AtomicBoolean(false);
94+
c = c.withStatements(collapseNextThrowablePrintStackTrace(c.getStatements(), ctx, caseCursor, addedLogger));
95+
return addedLogger.get() ? _case : c;
96+
}
97+
98+
private List<Statement> collapseNextThrowablePrintStackTrace(List<Statement> statements, ExecutionContext ctx, Cursor cursor, AtomicBoolean addedLogger) {
99+
AtomicInteger skip = new AtomicInteger(-1);
100+
return ListUtils.map(statements, (i, stat) -> {
86101
if (skip.get() == i) {
87102
return null;
88103
} else if (stat instanceof J.MethodInvocation) {
@@ -92,15 +107,15 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
92107
JavaType.Variable field = ((J.FieldAccess) m.getSelect()).getName().getFieldType();
93108
if (field != null && "err".equals(field.getName()) && TypeUtils.isOfClassType(field.getOwner(), "java.lang.System")) {
94109
Expression exceptionPrintStackTrace = null;
95-
if (block.getStatements().size() > i + 1) {
96-
J next = block.getStatements().get(i + 1);
110+
if (statements.size() > i + 1) {
111+
J next = statements.get(i + 1);
97112
if (next instanceof J.MethodInvocation && printStackTrace.matches((Expression) next)) {
98113
exceptionPrintStackTrace = ((J.MethodInvocation) next).getSelect();
99114
skip.set(i + 1);
100115
}
101116
}
102117

103-
Cursor printCursor = new Cursor(blockCursor, m);
118+
Cursor printCursor = new Cursor(cursor, m);
104119
J.MethodInvocation unchangedIfAddedLogger = logInsteadOfPrint(printCursor, ctx, exceptionPrintStackTrace);
105120
addedLogger.set(unchangedIfAddedLogger == m);
106121
return unchangedIfAddedLogger;
@@ -109,9 +124,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
109124
}
110125
}
111126
return stat;
112-
}));
113-
114-
return addedLogger.get() ? block : b;
127+
});
115128
}
116129

117130
@Override

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,4 +647,54 @@ fun main(logger: Logger, name: String) {
647647
)
648648
);
649649
}
650+
651+
@Test
652+
void logMethodInSwitchInTry() {
653+
rewriteRun(
654+
spec -> spec.recipe(new ParameterizedLogging("org.slf4j.Logger info(..)", false)),
655+
//language=java
656+
java(
657+
"""
658+
class Test {
659+
org.slf4j.Logger logger = null;
660+
661+
void method(int cnt, String name) {
662+
switch (cnt) {
663+
case 1:
664+
try {
665+
yield "bla";
666+
} catch (Exception e) {
667+
logger.info("This is a message for " + name + " with a $ dollar sign");
668+
yield "bla";
669+
}
670+
break;
671+
default:
672+
break;
673+
}
674+
}
675+
}
676+
""",
677+
"""
678+
class Test {
679+
org.slf4j.Logger logger = null;
680+
681+
void method(int cnt, String name) {
682+
switch (cnt) {
683+
case 1:
684+
try {
685+
yield "bla";
686+
} catch (Exception e) {
687+
logger.info("This is a message for {} with a $ dollar sign", name);
688+
yield "bla";
689+
}
690+
break;
691+
default:
692+
break;
693+
}
694+
}
695+
}"""
696+
)
697+
);
698+
}
699+
650700
}

‎src/test/java/org/openrewrite/java/logging/SystemErrToLoggingTest.java‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,4 +286,99 @@ void m(int cnt) {
286286
);
287287
}
288288

289+
@Issue("https://github.com/openrewrite/rewrite-logging-frameworks/issues/192")
290+
@Test
291+
void switchCaseStatementsWithAdditionalMethods() {
292+
rewriteRun(
293+
spec -> spec.recipe(new SystemErrToLogging(false, "logger", "SLF4J")),
294+
//language=java
295+
java(
296+
"""
297+
class A {
298+
org.slf4j.Logger logger = null;
299+
300+
void m(int cnt) {
301+
switch (cnt) {
302+
case 1:
303+
System.err.println("Oh no");
304+
break;
305+
case 2:
306+
default:
307+
break;
308+
}
309+
}
310+
311+
String m2() {
312+
return null;
313+
}
314+
}
315+
""",
316+
"""
317+
class A {
318+
org.slf4j.Logger logger = null;
319+
320+
void m(int cnt) {
321+
switch (cnt) {
322+
case 1:
323+
logger.error("Oh no");
324+
break;
325+
case 2:
326+
default:
327+
break;
328+
}
329+
}
330+
331+
String m2() {
332+
return null;
333+
}
334+
}
335+
"""
336+
),
337+
//language=java
338+
java(
339+
"""
340+
class B {
341+
org.slf4j.Logger logger = null;
342+
343+
void m(int cnt, Throwable t) {
344+
switch (cnt) {
345+
case 1:
346+
System.err.println("Oh " + cnt + " no");
347+
t.printStackTrace();
348+
break;
349+
case 2:
350+
default:
351+
break;
352+
}
353+
}
354+
355+
String m2() {
356+
return null;
357+
}
358+
}
359+
""",
360+
"""
361+
class B {
362+
org.slf4j.Logger logger = null;
363+
364+
void m(int cnt, Throwable t) {
365+
switch (cnt) {
366+
case 1:
367+
logger.error("Oh {} no", cnt, t);
368+
break;
369+
case 2:
370+
default:
371+
break;
372+
}
373+
}
374+
375+
String m2() {
376+
return null;
377+
}
378+
}
379+
"""
380+
)
381+
);
382+
}
383+
289384
}

0 commit comments

Comments
 (0)