phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'pr-2263'

Browse Source 
[java] Fix InvalidLogMessageFormat false-positive with lambda arguments
This commit is contained in:
Andreas Dangel
2020-02-16 12:06:12 +01:00
parent cb50a7af58 dd87753f1a
commit 7fb09deb3d
3 changed files with 78 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/InvalidLogMessageFormatRule.java
View File

@ -158,11 +158,32 @@ public class InvalidLogMessageFormatRule extends AbstractJavaRule {
int lastIndex = params.size() - 1;
ASTPrimaryExpression last = params.get(lastIndex).getFirstDescendantOfType(ASTPrimaryExpression.class);
if (isNewThrowable(last) || hasTypeThrowable(last) || isReferencingThrowable(last)) {
if (isNewThrowable(last) || hasTypeThrowable(last) || isReferencingThrowable(last) || isLambdaParameter(last)) {
params.remove(lastIndex);
}
}
private boolean isLambdaParameter(ASTPrimaryExpression last) {
String varName = null;
ASTPrimaryPrefix prefix = last.getFirstChildOfType(ASTPrimaryPrefix.class);
if (prefix != null) {
ASTName name = prefix.getFirstChildOfType(ASTName.class);
if (name != null) {
varName = name.getImage();
}
}
for (NameDeclaration decl : prefix.getScope().getDeclarations().keySet()) {
if (decl.getName().equals(varName)) {
if (decl.getNode().getParent() instanceof ASTLambdaExpression) {
// If the last parameter is a lambda parameter, then we also ignore it - regardless of the type.
// This is actually a workaround, since type resolution doesn't resolve the types of lambda parameters.
return true;
}
}
}
return false;
}
private String getExpectedMessage(final List<ASTExpression> params, final int expectedArguments) {
return " expected " + expectedArguments + (expectedArguments > 1 ? " arguments " : " argument ") + "but have "
+ params.size();

56
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml
View File

@ -785,7 +785,7 @@ public class InvalidLogMessageFormatTest {
</test-code>
<test-code>
<description>ignore slf4j-Markers when detecting the number of arguments</description>
<description>ignore slf4j-Markers when detecting the number of arguments #2250</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>11,17</expected-linenumbers>
<code><![CDATA[
@ -807,6 +807,60 @@ public class InvalidLogMessageFormatTest {
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>
<test-code>
<description>[java] InvalidLogMessageFormat false-positive for a lambda argument #2255</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
class InvalidLogMessageFormatTest {
private static final Logger LOGGER = LoggerFactory.getLogger(InvalidLogMessageFormatTest.class);
private InvalidLogMessageFormatTest() {
}
public static void main(String[] args) {
foo(exception -> LOGGER.error("Foo", exception));
}
private static void foo(Consumer<Throwable> consumer) {
consumer.accept(new IllegalArgumentException());
}
}
]]></code>
</test-code>
<test-code regressionTest="false">
<description>[java] InvalidLogMessageFormat false-negative for a lambda argument #2255</description>
<expected-problems>3</expected-problems>
<expected-linenumbers>11,13,16</expected-linenumbers>
<code><![CDATA[
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
class InvalidLogMessageFormatTest {
private static final Logger LOGGER = LoggerFactory.getLogger(InvalidLogMessageFormatTest.class);
private InvalidLogMessageFormatTest() {
}
public static void main(String[] args) {
foo(arg -> LOGGER.error("Foo", arg)); // missing violation: extra argument, that is not a exception
// explicit cast helps type resolution
foo((String arg) -> LOGGER.error("Foo", arg)); // violation: extra argument, that is not a exception
foo((String arg) -> LOGGER.error("Foo {}", arg)); // no violation: correct number of arguments
foo(arg -> LOGGER.error("Foo {}", arg)); // no violation: correct number of arguments
foo(arg -> LOGGER.error("Foo {} {}", arg)); // violation: missing argument
}
private static void foo(Consumer<String> consumer) {
consumer.accept("bar");
}
}
]]></code>
</test-code>