Skip to content

Commit 6da55e6

Browse files
authored
Fix Slf4jLogShouldBeConstant to preserve format specifiers with width, alignment, and precision. (#239)
1 parent cc1dd4f commit 6da55e6

File tree

2 files changed

+71
-11
lines changed

2 files changed

+71
-11
lines changed

src/main/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstant.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,15 @@
3131
import org.openrewrite.java.tree.TypeUtils;
3232

3333
import java.util.*;
34+
import java.util.regex.Matcher;
3435
import java.util.regex.Pattern;
3536

3637
public class Slf4jLogShouldBeConstant extends Recipe {
3738

3839
private static final String SLF4J_FORMAT_SPECIFIER = "{}";
3940
private static final Pattern SLF4J_FORMAT_SPECIFIER_PATTERN = Pattern.compile("\\{}");
40-
private static final Pattern FORMAT_SPECIFIER_PATTERN = Pattern.compile("%[\\d.]*[dfscbBhHn%]");
41+
private static final Pattern FORMAT_SPECIFIER_PATTERN = Pattern.compile("%[-#+ 0,(\\d+]*[\\d.]*[dfscbBhHn%]");
42+
private static final Pattern SIMPLE_FORMAT_SPECIFIER_PATTERN = Pattern.compile("%[dfscbBhHn%]");
4143

4244
// A regular expression that matches index specifiers like '%2$s', '%1$s', etc.
4345
private static final Pattern INDEXED_FORMAT_SPECIFIER_PATTERN = Pattern.compile("%(\\d+\\$)[a-zA-Z]");
@@ -78,7 +80,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
7880
}
7981

8082
String strFormat = Objects.requireNonNull(((J.Literal) stringFormat.getArguments().get(0)).getValue()).toString();
81-
if (containsIndexFormatSpecifier(strFormat) || containsCombinedFormatSpecifiers(strFormat)) {
83+
if (containsIndexFormatSpecifier(strFormat) || containsCombinedFormatSpecifiers(strFormat) || containsComplexFormatSpecifiers(strFormat)) {
8284
return method;
8385
}
8486
String updatedStrFormat = replaceFormatSpecifier(strFormat, SLF4J_FORMAT_SPECIFIER);
@@ -133,4 +135,18 @@ private static boolean containsIndexFormatSpecifier(String str) {
133135
}
134136
return INDEXED_FORMAT_SPECIFIER_PATTERN.matcher(str).find();
135137
}
138+
139+
private static boolean containsComplexFormatSpecifiers(String str) {
140+
if (StringUtils.isNullOrEmpty(str)) {
141+
return false;
142+
}
143+
Matcher matcher = FORMAT_SPECIFIER_PATTERN.matcher(str);
144+
while (matcher.find()) {
145+
String specifier = matcher.group();
146+
if (!SIMPLE_FORMAT_SPECIFIER_PATTERN.matcher(specifier).matches()) {
147+
return true;
148+
}
149+
}
150+
return false;
151+
}
136152
}

src/test/java/org/openrewrite/java/logging/slf4j/Slf4jLogShouldBeConstantTest.java

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,6 @@ void method() {
4949
log.info(String.format("The first argument is '%d', and the second argument is '%.2f'.", 1, 2.3333));
5050
}
5151
}
52-
""",
53-
"""
54-
import org.slf4j.Logger;
55-
class A {
56-
Logger log;
57-
void method() {
58-
log.info("The first argument is '{}', and the second argument is '{}'.", 1, 2.3333);
59-
}
60-
}
6152
"""
6253
)
6354
);
@@ -372,4 +363,57 @@ void test() {
372363
);
373364
}
374365

366+
@Test
367+
void noChangeWithWidth() {
368+
//language=java
369+
rewriteRun(
370+
java(
371+
"""
372+
import org.slf4j.Logger;
373+
class A {
374+
Logger log;
375+
void method() {
376+
log.info(String.format("%10s", "test"));
377+
}
378+
}
379+
"""
380+
)
381+
);
382+
}
383+
384+
@Test
385+
void noChangeWithLeftAlignment() {
386+
//language=java
387+
rewriteRun(
388+
java(
389+
"""
390+
import org.slf4j.Logger;
391+
class A {
392+
Logger log;
393+
void method() {
394+
log.info(String.format("%-10s", "test"));
395+
}
396+
}
397+
"""
398+
)
399+
);
400+
}
401+
402+
@Test
403+
void noChangeWithPrecision() {
404+
//language=java
405+
rewriteRun(
406+
java(
407+
"""
408+
import org.slf4j.Logger;
409+
class A {
410+
Logger log;
411+
void method() {
412+
log.info(String.format("%.2f", 1.2345));
413+
}
414+
}
415+
"""
416+
)
417+
);
418+
}
375419
}

0 commit comments

Comments
 (0)