Allow PreferSafeLogger to migrate uses with level-checks (#1842)

Allow `PreferSafeLogger` to migrate logger uses which include level-checks
palantir · Jul 22, 2021 · adc6b35 · adc6b35
1 parent 779c6db
commit adc6b35
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 1 deletion.
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
@@ -128,7 +128,8 @@ private static boolean isSafeSlf4jInteraction(MethodInvocationTree tree, Visitor
         }
         List<? extends Tree> arguments = tree.getArguments();
         if (arguments.isEmpty()) {
-            return false;
+            // is<level>Enabled
+            return true;
         }
         if (!state.getTypes()
                 .isSameType(ASTHelpers.getType(arguments.get(0)), state.getTypeFromString(String.class.getName()))) {

diff --git a/...line-error-prone/src/test/java/com/palantir/baseline/errorprone/PreferSafeLoggerTest.java b/...line-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
@@ -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