From 60cb037f8081ac431f03b1305f98b8927c307a84 Mon Sep 17 00:00:00 2001
From: kris-scheibe <k.scheibe@posteo.de>
Date: Sat, 25 Jan 2020 10:17:59 +0100
Subject: [PATCH] FP for InvalidLogMessageFormat when using slf4j-Markers

---
 .../InvalidLogMessageFormatRule.java          |  5 ++++
 .../xml/InvalidLogMessageFormat.xml           | 27 +++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidLogMessageFormatRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidLogMessageFormatRule.java
index 37d5236176..46101bdff2 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidLogMessageFormatRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidLogMessageFormatRule.java
@@ -89,6 +89,11 @@ public class InvalidLogMessageFormatRule extends AbstractJavaRule {
         final List<ASTExpression> argumentList = parentNode.getFirstChildOfType(ASTPrimarySuffix.class)
                 .getFirstDescendantOfType(ASTArgumentList.class).findChildrenOfType(ASTExpression.class);
 
+        // ignore the first argument if it is a known non-string value, e.g. a slf4j-Marker
+        if (argumentList.get(0).getType() != null && !argumentList.get(0).getType().equals(String.class)) {
+            argumentList.remove(0);
+        }
+
         // remove the message parameter
         final ASTExpression messageParam = argumentList.remove(0);
         final int expectedArguments = expectedArguments(messageParam);
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml
index 5b44373acc..8f0db82778 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml
@@ -780,6 +780,33 @@ public class InvalidLogMessageFormatTest {
         logger.warn("foo {}", "flibble", "moo", "blah", "blah"); // PMD flags this
         logger.warn("foo", "flibble", "moo", "blah", "blah"); // PMD doesn't flag this
     }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>ignore slf4j-Markers when detecting the number of arguments</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>11,17</expected-linenumbers>
+        <code><![CDATA[
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.Marker;
+import org.slf4j.helpers.BasicMarkerFactory;
+
+public class InvalidLogMessageFormatTest {
+    private static final Logger logger = LoggerFactory.getLogger("MyLogger");
+    private static final Marker marker = BasicMarkerFactory.getMarker("MARKER");
+
+    public static void main(String[] args) {
+        logger.warn(marker, "foo {}", "flibble", "moo", "blah", "blah"); // wrong number of arguments
+        logger.warn(marker, "foo"); // correct: marker and no arguments
+        logger.warn(marker, "foo", new Exception()); // correct: marker and one exception parameter
+        logger.warn(marker, "foo {}", "bar"); // correct: marker and one argument
+
+        final var otherMarker = MarkerFactory.getMarker("OTHER_MARKER");
+        logger.warn(otherMarker, "foo"); // gets flagged as we can't statically determine the type of the "otherMarker" variable
+    }
 }
         ]]></code>
     </test-code>