Merge branch 'pr-453'

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java

@@ -70,23 +70,9 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule {
         // find the arguments
         final List<ASTExpression> argumentList = parentNode.getFirstChildOfType(ASTPrimarySuffix.class)
                 .getFirstDescendantOfType(ASTArgumentList.class).findChildrenOfType(ASTExpression.class);
-        final List<ASTPrimaryExpression> params = new ArrayList<ASTPrimaryExpression>(argumentList.size());
 
-        for (final ASTExpression astExpression : argumentList) {
-            ASTPrimaryExpression primaryExpression = astExpression.getFirstChildOfType(ASTPrimaryExpression.class);
-            if (primaryExpression != null) {
-                params.add(primaryExpression);
-            }
-        }
-
-        if (params.isEmpty()) {
-            // no params we could analyze
-            return super.visit(node, data);
-        }
-
-        final ASTPrimaryExpression messageParam = params.get(0);
         // remove the message parameter
-        params.remove(0);
+        final ASTPrimaryExpression messageParam = argumentList.remove(0).getFirstDescendantOfType(ASTPrimaryExpression.class);
         final int expectedArguments = expectedArguments(messageParam);
 
         if (expectedArguments == 0) {
@@ -97,14 +83,14 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule {
 
         // Remove throwable param, since it is shown separately.
         // But only, if it is not used as a placeholder argument
-        if (params.size() > expectedArguments) {
-            removeThrowableParam(params);
+        if (argumentList.size() > expectedArguments) {
+            removeThrowableParam(argumentList);
         }
 
-        if (params.size() < expectedArguments) {
-            addViolationWithMessage(data, node, "Missing arguments," + getExpectedMessage(params, expectedArguments));
-        } else if (params.size() > expectedArguments) {
-            addViolationWithMessage(data, node, "Too many arguments," + getExpectedMessage(params, expectedArguments));
+        if (argumentList.size() < expectedArguments) {
+            addViolationWithMessage(data, node, "Missing arguments," + getExpectedMessage(argumentList, expectedArguments));
+        } else if (argumentList.size() > expectedArguments) {
+            addViolationWithMessage(data, node, "Too many arguments," + getExpectedMessage(argumentList, expectedArguments));
         }
 
         return super.visit(node, data);
@@ -146,21 +132,20 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule {
         return false;
     }
 
-    private void removeThrowableParam(final List<ASTPrimaryExpression> params) {
+    private void removeThrowableParam(final List<ASTExpression> params) {
         // Throwable parameters are the last one in the list, if any.
         if (params.isEmpty()) {
             return;
         }
         int lastIndex = params.size() - 1;
-        ASTPrimaryExpression last = params.get(lastIndex);
+        ASTPrimaryExpression last = params.get(lastIndex).getFirstDescendantOfType(ASTPrimaryExpression.class);
 
         if (isNewThrowable(last) || hasTypeThrowable(last) || isReferencingThrowable(last)) {
             params.remove(lastIndex);
-            return;
         }
     }
 
-    private String getExpectedMessage(final List<ASTPrimaryExpression> params, final int expectedArguments) {
+    private String getExpectedMessage(final List<ASTExpression> params, final int expectedArguments) {
         return " expected " + expectedArguments + (expectedArguments > 1 ? " arguments " : " argument ") + "but have "
                 + params.size();
     }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml

@@ -198,6 +198,26 @@ public class TestBug1551
     {
         return "message";
     }
+}
+        ]]></code>
+    </test-code>
+    
+    <test-code>
+        <description>#365 [java] InvalidSlf4jMessageFormat: false positive with pre-incremented variable</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class Foo
+{
+    private static final Logger LOGGER = LoggerFactory.getLogger(Foo.class);
+
+    public void test()
+    {
+        int attempt = 0;
+        LOGGER.info("test (attempt #{})", ++attempt);
+    }
 }
         ]]></code>
     </test-code>

src/site/markdown/overview/changelog.md

@@ -61,6 +61,8 @@ Fields using generics are still Work in Progress, but we expect to fully support
     *   [#348]((https://github.com/pmd/pmd/issues/348): \[java] imports/UnusedImport rule not considering static inner classes of imports
 *   java-junit
     *   [#428](https://github.com/pmd/pmd/issues/428): \[java] PMD requires public modifier on JUnit 5 test
+*   java-logging:
+    *   [#365](https://github.com/pmd/pmd/issues/365): \[java] InvalidSlf4jMessageFormat does not handle inline incrementation of arguments
 *   java-unnecessary
     *   [#421](https://github.com/pmd/pmd/issues/421): \[java] UnnecessaryFinalModifier final in private method
 *   jsp