From adc6b3595cf181f428e48c2ff04989302c481c76 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 22 Jul 2021 13:19:00 -0400 Subject: [PATCH] Allow `PreferSafeLogger` to migrate uses with level-checks (#1842) Allow `PreferSafeLogger` to migrate logger uses which include level-checks --- .../baseline/errorprone/PreferSafeLogger.java | 3 +- .../errorprone/PreferSafeLoggerTest.java | 29 +++++++++++++++++++ changelog/@unreleased/pr-1842.v2.yml | 5 ++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 changelog/@unreleased/pr-1842.v2.yml diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferSafeLogger.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferSafeLogger.java index a218c1bbe..9eace66f6 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferSafeLogger.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/PreferSafeLogger.java @@ -128,7 +128,8 @@ private static boolean isSafeSlf4jInteraction(MethodInvocationTree tree, Visitor } List arguments = tree.getArguments(); if (arguments.isEmpty()) { - return false; + // isEnabled + return true; } if (!state.getTypes() .isSameType(ASTHelpers.getType(arguments.get(0)), state.getTypeFromString(String.class.getName()))) { diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferSafeLoggerTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferSafeLoggerTest.java index 44a4e227f..9eb213d3a 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferSafeLoggerTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferSafeLoggerTest.java @@ -130,6 +130,35 @@ void testSimpleFixWithArgAndThrowable() { .doTest(); } + @Test + void testFixWithLevelCheck() { + fix().addInputLines( + "Test.java", + "import org.slf4j.*;", + "class Test {", + " private static final Logger log = LoggerFactory.getLogger(Test.class);", + " void action(Throwable t) {", + " if (log.isInfoEnabled()) {", + " log.info(\"foo\", t);", + " }", + " }", + "}") + .addOutputLines( + "Test.java", + "import com.palantir.logsafe.logger.SafeLogger;", + "import com.palantir.logsafe.logger.SafeLoggerFactory;", + "import org.slf4j.*;", + "class Test {", + " private static final SafeLogger log = SafeLoggerFactory.get(Test.class);", + " void action(Throwable t) {", + " if (log.isInfoEnabled()) {", + " log.info(\"foo\", t);", + " }", + " }", + "}") + .doTest(); + } + @Test void testIgnoresIncorrectlyOrderedThrowables() { fix().addInputLines( diff --git a/changelog/@unreleased/pr-1842.v2.yml b/changelog/@unreleased/pr-1842.v2.yml new file mode 100644 index 000000000..44ae6ac25 --- /dev/null +++ b/changelog/@unreleased/pr-1842.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Allow `PreferSafeLogger` to migrate logger uses which include level-checks + links: + - https://github.com/palantir/gradle-baseline/pull/1842