From cc4294b39925163ca0ac45ae3d4c7c3bf67641af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:04:41 -0300 Subject: [PATCH 1/2] [java] Simplify handling of SLF4J log arguments - Fixes #365 --- .../InvalidSlf4jMessageFormatRule.java | 35 ++++++------------- .../xml/InvalidSlf4jMessageFormat.xml | 20 +++++++++++ 2 files changed, 30 insertions(+), 25 deletions(-) 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 + From 60359ad1cc74bc43e0169ae0b6ae0d380ed67815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 21 Jun 2017 17:07:26 -0300 Subject: [PATCH 2/2] Update changelog, refs #365 --- src/site/markdown/overview/changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md index e54f761ecf..4598374faf 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