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 9eace66f6..60435a63b 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 @@ -102,6 +102,9 @@ public Void visitMethodInvocation(MethodInvocationTree tree, Void _unused) { if (sym.equals(ASTHelpers.getSymbol(receiver))) { if (!isSafeSlf4jInteraction(tree, state)) { foundUnknownUsage.set(true); + } else { + // Scan arguments for findings that may not compile + scan(tree.getArguments(), null); } return null; } 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 9eb213d3a..1c24b8f86 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 @@ -210,4 +210,48 @@ void testPassedLoggerUnmodified() { .expectUnchanged() .doTest(); } + + @Test + void testGetNameInArgUnmodified() { + // getName is not supported by SafeLogger, so we shouldn't make changes that won't compile. + fix().addInputLines( + "Test.java", + "import com.palantir.logsafe.*;", + "import org.slf4j.*;", + "class Test {", + " private static final Logger log = LoggerFactory.getLogger(Test.class);", + " void action() {", + " log.info(\"foo\", SafeArg.of(\"name\", log.getName()));", + " }", + "}") + .expectUnchanged() + .doTest(); + } + + @Test + void testIsTraceEnabledInArg() { + fix().addInputLines( + "Test.java", + "import com.palantir.logsafe.*;", + "import org.slf4j.*;", + "class Test {", + " private static final Logger log = LoggerFactory.getLogger(Test.class);", + " void action() {", + " log.info(\"foo\", SafeArg.of(\"name\", log.isTraceEnabled()));", + " }", + "}") + .addOutputLines( + "Test.java", + "import com.palantir.logsafe.*;", + "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() {", + " log.info(\"foo\", SafeArg.of(\"name\", log.isTraceEnabled()));", + " }", + "}") + .doTest(); + } } diff --git a/changelog/@unreleased/pr-1851.v2.yml b/changelog/@unreleased/pr-1851.v2.yml new file mode 100644 index 000000000..5a94374de --- /dev/null +++ b/changelog/@unreleased/pr-1851.v2.yml @@ -0,0 +1,6 @@ +type: fix +fix: + description: Fix PreferSafeLogger edge case that produced suggested fixes that didn't + compile without human interaction. + links: + - https://github.com/palantir/gradle-baseline/pull/1851