diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 65deb54c44..7e421bc5d0 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -45,6 +45,7 @@ This is a minor release. * java-errorprone * [#1078](https://github.com/pmd/pmd/issues/1078): \[java] MissingSerialVersionUID rule does not seem to catch inherited classes * java-performance + * [#1291](https://github.com/pmd/pmd/issues/1291): \[java] InvalidSlf4jMessageFormat false positive: too many arguments with string concatenation operator * [#1298](https://github.com/pmd/pmd/issues/1298): \[java] RedundantFieldInitializer - NumberFormatException with Long * jsp * [#1274](https://github.com/pmd/pmd/issues/1274): \[jsp] Support EL in tag attributes diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidSlf4jMessageFormatRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidSlf4jMessageFormatRule.java index 86ab80b76c..a3a56cf22e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidSlf4jMessageFormatRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidSlf4jMessageFormatRule.java @@ -42,18 +42,22 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { .unmodifiableSet(new HashSet(Arrays.asList("trace", "debug", "info", "warn", "error"))); } + public InvalidSlf4jMessageFormatRule() { + addRuleChainVisit(ASTName.class); + } + @Override public Object visit(final ASTName node, final Object data) { final NameDeclaration nameDeclaration = node.getNameDeclaration(); // ignore imports or methods if (!(nameDeclaration instanceof VariableNameDeclaration)) { - return super.visit(node, data); + return data; } // ignore non slf4j logger Class type = ((VariableNameDeclaration) nameDeclaration).getType(); if (type == null || !type.getName().equals(LOGGER_CLASS)) { - return super.visit(node, data); + return data; } // get the node that contains the logger @@ -65,7 +69,7 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { // ignore if not a log level if (!LOGGER_LEVELS.contains(method)) { - return super.visit(node, data); + return data; } // find the arguments @@ -73,13 +77,13 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { .getFirstDescendantOfType(ASTArgumentList.class).findChildrenOfType(ASTExpression.class); // remove the message parameter - final ASTPrimaryExpression messageParam = argumentList.remove(0).getFirstDescendantOfType(ASTPrimaryExpression.class); + final ASTExpression messageParam = argumentList.remove(0); final int expectedArguments = expectedArguments(messageParam); if (expectedArguments == 0) { // ignore if we are not expecting arguments to format the message // or if we couldn't analyze the message parameter - return super.visit(node, data); + return data; } // Remove throwable param, since it is shown separately. @@ -89,12 +93,14 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { } if (argumentList.size() < expectedArguments) { - addViolationWithMessage(data, node, "Missing arguments," + getExpectedMessage(argumentList, expectedArguments)); + addViolationWithMessage(data, node, + "Missing arguments," + getExpectedMessage(argumentList, expectedArguments)); } else if (argumentList.size() > expectedArguments) { - addViolationWithMessage(data, node, "Too many arguments," + getExpectedMessage(argumentList, expectedArguments)); + addViolationWithMessage(data, node, + "Too many arguments," + getExpectedMessage(argumentList, expectedArguments)); } - return super.visit(node, data); + return data; } private boolean isNewThrowable(ASTPrimaryExpression last) { @@ -145,7 +151,7 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { + params.size(); } - private int expectedArguments(final ASTPrimaryExpression node) { + private int expectedArguments(final ASTExpression node) { int count = 0; // look if the logger have a literal message if (node.getFirstDescendantOfType(ASTLiteral.class) != null) { @@ -153,7 +159,8 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { } else if (node.getFirstDescendantOfType(ASTName.class) != null) { final String variableName = node.getFirstDescendantOfType(ASTName.class).getImage(); // look if the message is defined locally - final List localVariables = node.getFirstParentOfType(ASTMethodOrConstructorDeclaration.class) + final List localVariables = node + .getFirstParentOfType(ASTMethodOrConstructorDeclaration.class) .findDescendantsOfType(ASTVariableDeclarator.class); count = getAmountOfExpectedArguments(variableName, localVariables); @@ -183,10 +190,13 @@ public class InvalidSlf4jMessageFormatRule extends AbstractJavaRule { } private int countPlaceholders(final AbstractJavaTypeNode node) { - int result = 0; // zero means, no placeholders, or we could not analyze the message parameter - ASTLiteral stringLiteral = node.getFirstDescendantOfType(ASTLiteral.class); - if (stringLiteral != null) { - result = StringUtils.countMatches(stringLiteral.getImage(), "{}"); + // zero means, no placeholders, or we could not analyze the message parameter + int result = 0; + List literals = node.findDescendantsOfType(ASTLiteral.class); + // if there are multiple literals, we just assume, they are concatenated + // together... + for (ASTLiteral stringLiteral : literals) { + result += StringUtils.countMatches(stringLiteral.getImage(), "{}"); } return result; } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidSlf4jMessageFormat.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidSlf4jMessageFormat.xml index 4163fdb66c..7dd73838b5 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidSlf4jMessageFormat.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidSlf4jMessageFormat.xml @@ -240,5 +240,28 @@ public class Foo ]]> - + + #1291 [java] InvalidSlf4jMessageFormat false positive: too many arguments with string concatenation operator + 0 + + \ No newline at end of file