diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java index b99abf17ff..4eb61ffcdd 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/logging/InvalidSlf4jMessageFormatRule.java +++ b/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 argumentList = parentNode.getFirstChildOfType(ASTPrimarySuffix.class) .getFirstDescendantOfType(ASTArgumentList.class).findChildrenOfType(ASTExpression.class); - final List params = new ArrayList(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 params) { + private void removeThrowableParam(final List 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 params, final int expectedArguments) { + private String getExpectedMessage(final List params, final int expectedArguments) { return " expected " + expectedArguments + (expectedArguments > 1 ? " arguments " : " argument ") + "but have " + params.size(); } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml index ac0f174986..38f74f2666 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/loggingjava/xml/InvalidSlf4jMessageFormat.xml @@ -198,6 +198,26 @@ public class TestBug1551 { return "message"; } +} + ]]> + + + + #365 [java] InvalidSlf4jMessageFormat: false positive with pre-incremented variable + 0 + diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index 98359d46f9..ebbddd4ca5 100644 --- a/src/site/markdown/overview/changelog.md +++ b/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