Merge branch 'master' into pr/3153

This commit is contained in:
Clément Fournier committed 2021-03-10 12:45:03 +01:00
commit ea41b1d556
10 files changed
+243 -6

No files matched your search

+7
View File
@@ -16,6 +16,13 @@ This is a {{ site.pmd.release_type }} release.
### Fixed Issues
* java
* [#3117](https://github.com/pmd/pmd/issues/3117): \[java] Infinite loop when parsing invalid code nested in lambdas
* [#3145](https://github.com/pmd/pmd/issues/3145): \[java] Parse exception when using "record" as variable name
* java-bestpractices
* [#3160](https://github.com/pmd/pmd/issues/3160): \[java] MethodReturnsInternalArray does not consider static final fields and fields initialized with empty array
* java-errorprone
* [#3146](https://github.com/pmd/pmd/issues/3146): \[java] InvalidLogMessageFormat detection failing when String.format used
* java-performance
* [#2427](https://github.com/pmd/pmd/issues/2427): \[java] ConsecutiveLiteralAppend false-positive with builder inside lambda
+15 -4
View File
@@ -1,4 +1,10 @@
/**
* Fix #3117 - infinite loop when parsing invalid code nested in lambdas
* Andreas Dangel 03/2021
*====================================================================
* Fix #3145 - parse exception with local records
* Clément Fournier 03/2021
*====================================================================
* Remove support for Java 14 preview language features
* JEP 397: Sealed Classes (Second Preview) for Java16 Preview
* JEP 395: Records for Java16
@@ -237,6 +243,11 @@ options {
TRACK_TOKENS = true;
NODE_PACKAGE="net.sourceforge.pmd.lang.java.ast";
// disable the calculation of expected tokens when a parse error occurs
// depending on the possible allowed next tokens, this
// could be expensive (see https://github.com/pmd/pmd/issues/3117)
//ERROR_REPORTING = false;
//DEBUG_PARSER = true;
//DEBUG_LOOKAHEAD = true;
//DEBUG_TOKEN_MANAGER = true;
@@ -594,7 +605,7 @@ public class JavaParser {
return next.kind == CLASS
|| isRecordTypeSupported() && next.kind == INTERFACE
|| isRecordTypeSupported() && next.kind == IDENTIFIER && next.image.equals("enum")
|| isRecordTypeSupported() && next.kind == IDENTIFIER && next.image.equals("record");
|| isRecordTypeSupported() && next.kind == IDENTIFIER && next.image.equals("record") && isToken(2, IDENTIFIER);
}
/**
@@ -2131,9 +2142,9 @@ void StatementExpression() :
|
PreDecrementExpression()
|
LOOKAHEAD( PrimaryExpression() AssignmentOperator() ) PrimaryExpression() AssignmentOperator() Expression()
|
PostfixExpression()
// using PostfixExpression here allows us to skip the part of the production tree
// between Expression() and PostfixExpression()
PostfixExpression() [ AssignmentOperator() Expression() ]
}
void SwitchStatement():
@@ -116,7 +116,7 @@ public class MethodReturnsInternalArrayRule extends AbstractSunSecureRule {
if (fds != null) {
for (ASTFieldDeclaration fd : fds) {
final ASTVariableDeclaratorId vid = fd.getFirstDescendantOfType(ASTVariableDeclaratorId.class);
if (vid != null && vid.hasImageEqualTo(varName)) {
if (fd.isFinal() && vid != null && vid.hasImageEqualTo(varName)) {
ASTVariableInitializer initializer = fd.getFirstDescendantOfType(ASTVariableInitializer.class);
if (initializer != null && initializer.getNumChildren() == 1) {
Node child = initializer.getChild(0);
@@ -15,6 +15,7 @@ import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
import net.sourceforge.pmd.lang.java.ast.ASTArrayInitializer;
@@ -23,6 +24,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTEnumBody;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
@@ -63,10 +65,31 @@ public class InvalidLogMessageFormatRule extends AbstractJavaRule {
LOGGERS = loggersMap;
}
private boolean formatIsStringFormat;
public InvalidLogMessageFormatRule() {
addRuleChainVisit(ASTImportDeclaration.class);
addRuleChainVisit(ASTName.class);
}
@Override
public void start(RuleContext ctx) {
formatIsStringFormat = false;
}
@Override
public Object visit(ASTImportDeclaration node, Object data) {
if (node.isStatic()) {
if ("java.lang.String.format".equals(node.getImportedName())) {
formatIsStringFormat = true;
}
if ("java.lang.String".equals(node.getImportedName()) && node.isImportOnDemand()) {
formatIsStringFormat = true;
}
}
return data;
}
@Override
public Object visit(final ASTName node, final Object data) {
final NameDeclaration nameDeclaration = node.getNameDeclaration();
@@ -111,8 +134,13 @@ public class InvalidLogMessageFormatRule extends AbstractJavaRule {
// remove the message parameter
final ASTExpression messageParam = argumentList.remove(0);
final int expectedArguments = expectedArguments(messageParam);
// ignore if String.format
if (isStringFormatCall(messageParam)) {
return data;
}
final int expectedArguments = expectedArguments(messageParam);
if (expectedArguments == -1) {
// ignore if we couldn't analyze the message parameter
return data;
@@ -213,6 +241,17 @@ public class InvalidLogMessageFormatRule extends AbstractJavaRule {
+ params.size();
}
private boolean isStringFormatCall(ASTExpression node) {
if (node.getNumChildren() > 0 && node.getChild(0) instanceof ASTPrimaryExpression
&& node.getChild(0).getNumChildren() > 0 && node.getChild(0).getChild(0) instanceof ASTPrimaryPrefix
&& node.getChild(0).getChild(0).getNumChildren() > 0 && node.getChild(0).getChild(0).getChild(0) instanceof ASTName) {
String name = node.getChild(0).getChild(0).getChild(0).getImage();
return "String.format".equals(name) || formatIsStringFormat && "format".equals(name);
}
return false;
}
private int expectedArguments(final ASTExpression node) {
int count = -1;
// look if the logger has a literal message
@@ -226,6 +226,13 @@ public class ParserCornersTest {
java8.parseResource("GitHubBug309.java");
}
@Test(timeout = 30000)
public void testInfiniteLoopInLookahead() {
expect.expect(ParseException.class);
// https://github.com/pmd/pmd/issues/3117
java8.parseResource("InfiniteLoopInLookahead.java");
}
/**
* This triggered bug #1484 UnusedLocalVariable - false positive -
* parenthesis
@@ -0,0 +1,28 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
import java.util.*;
public class InfiniteLoopInLookahead {
public void exam1(List resList) {
resList.forEach(a -> {
resList.forEach(b -> {
resList.forEach(c -> {
resList.forEach(d -> {
resList.forEach(e -> {
resList.forEach(f -> {
resList.forEach(g -> {
resList.forEach(h -> {
resList // note: missing semicolon -> parse error here...
});
});
});
});
});
});
});
});
}
}
@@ -31,6 +31,12 @@ public class LocalRecords {
final @Deprecated static record MyRecord4(String a) {}
}
void statementThatStartsWithRecordAsRegularIdent() {
// https://github.com/pmd/pmd/issues/3145
final Map<String, String> record = new HashMap<>();
record.put("key", "value");
}
void methodWithLocalClass() {
class MyLocalClass {}
}
@@ -215,6 +215,51 @@
| | +- VariableDeclaratorId[@Array = false, @ArrayDepth = 0, @ArrayType = false, @ExceptionBlockParameter = false, @ExplicitReceiverParameter = false, @Field = false, @Final = true, @FormalParameter = false, @Image = "a", @LambdaParameter = false, @LocalVariable = false, @Name = "a", @PatternBinding = false, @ResourceDeclaration = false, @TypeInferred = false, @VariableName = "a"]
| +- RecordBody[]
+- ClassOrInterfaceBodyDeclaration[@AnonymousInnerClass = false, @EnumChild = false, @Kind = DeclarationKind.METHOD]
| +- MethodDeclaration[@Abstract = false, @Arity = 0, @Default = false, @Final = false, @InterfaceMember = false, @Kind = MethodLikeKind.METHOD, @MethodName = "statementThatStartsWithRecordAsRegularIdent", @Modifiers = 0, @Name = "statementThatStartsWithRecordAsRegularIdent", @Native = false, @PackagePrivate = true, @Private = false, @Protected = false, @Public = false, @Static = false, @Strictfp = false, @Synchronized = false, @SyntacticallyAbstract = false, @SyntacticallyPublic = false, @Transient = false, @Void = true, @Volatile = false]
| +- ResultType[@Void = true, @returnsArray = false]
| +- MethodDeclarator[@Image = "statementThatStartsWithRecordAsRegularIdent", @ParameterCount = 0]
| | +- FormalParameters[@ParameterCount = 0, @Size = 0]
| +- Block[@containsComment = false]
| +- BlockStatement[@Allocation = true]
| | +- LocalVariableDeclaration[@Abstract = false, @Array = false, @ArrayDepth = 0, @Default = false, @Final = true, @Modifiers = 32, @Native = false, @PackagePrivate = true, @Private = false, @Protected = false, @Public = false, @Static = false, @Strictfp = false, @Synchronized = false, @Transient = false, @TypeInferred = false, @VariableName = "record", @Volatile = false]
| | +- Type[@Array = false, @ArrayDepth = 0, @ArrayType = false, @TypeImage = "Map"]
| | | +- ReferenceType[@Array = false, @ArrayDepth = 0]
| | | +- ClassOrInterfaceType[@AnonymousClass = false, @Array = false, @ArrayDepth = 0, @Image = "Map", @ReferenceToClassSameCompilationUnit = false]
| | | +- TypeArguments[@Diamond = false]
| | | +- TypeArgument[@Wildcard = false]
| | | | +- ReferenceType[@Array = false, @ArrayDepth = 0]
| | | | +- ClassOrInterfaceType[@AnonymousClass = false, @Array = false, @ArrayDepth = 0, @Image = "String", @ReferenceToClassSameCompilationUnit = false]
| | | +- TypeArgument[@Wildcard = false]
| | | +- ReferenceType[@Array = false, @ArrayDepth = 0]
| | | +- ClassOrInterfaceType[@AnonymousClass = false, @Array = false, @ArrayDepth = 0, @Image = "String", @ReferenceToClassSameCompilationUnit = false]
| | +- VariableDeclarator[@Initializer = true, @Name = "record"]
| | +- VariableDeclaratorId[@Array = false, @ArrayDepth = 0, @ArrayType = false, @ExceptionBlockParameter = false, @ExplicitReceiverParameter = false, @Field = false, @Final = true, @FormalParameter = false, @Image = "record", @LambdaParameter = false, @LocalVariable = true, @Name = "record", @PatternBinding = false, @ResourceDeclaration = false, @TypeInferred = false, @VariableName = "record"]
| | +- VariableInitializer[]
| | +- Expression[@StandAlonePrimitive = false]
| | +- PrimaryExpression[]
| | +- PrimaryPrefix[@SuperModifier = false, @ThisModifier = false]
| | +- AllocationExpression[@AnonymousClass = false]
| | +- ClassOrInterfaceType[@AnonymousClass = false, @Array = false, @ArrayDepth = 0, @Image = "HashMap", @ReferenceToClassSameCompilationUnit = false]
| | | +- TypeArguments[@Diamond = true]
| | +- Arguments[@ArgumentCount = 0, @Size = 0]
| +- BlockStatement[@Allocation = false]
| +- Statement[]
| +- StatementExpression[]
| +- PrimaryExpression[]
| +- PrimaryPrefix[@SuperModifier = false, @ThisModifier = false]
| | +- Name[@Image = "record.put"]
| +- PrimarySuffix[@ArgumentCount = 2, @Arguments = true, @ArrayDereference = false]
| +- Arguments[@ArgumentCount = 2, @Size = 2]
| +- ArgumentList[@Size = 2]
| +- Expression[@StandAlonePrimitive = false]
| | +- PrimaryExpression[]
| | +- PrimaryPrefix[@SuperModifier = false, @ThisModifier = false]
| | +- Literal[@CharLiteral = false, @DoubleLiteral = false, @EscapedStringLiteral = ""key"", @FloatLiteral = false, @Image = ""key"", @IntLiteral = false, @LongLiteral = false, @SingleCharacterStringLiteral = false, @StringLiteral = true, @TextBlock = false, @TextBlockContent = ""key"", @ValueAsDouble = NaN, @ValueAsFloat = NaN, @ValueAsInt = 0, @ValueAsLong = 0]
| +- Expression[@StandAlonePrimitive = false]
| +- PrimaryExpression[]
| +- PrimaryPrefix[@SuperModifier = false, @ThisModifier = false]
| +- Literal[@CharLiteral = false, @DoubleLiteral = false, @EscapedStringLiteral = ""value"", @FloatLiteral = false, @Image = ""value"", @IntLiteral = false, @LongLiteral = false, @SingleCharacterStringLiteral = false, @StringLiteral = true, @TextBlock = false, @TextBlockContent = ""value"", @ValueAsDouble = NaN, @ValueAsFloat = NaN, @ValueAsInt = 0, @ValueAsLong = 0]
+- ClassOrInterfaceBodyDeclaration[@AnonymousInnerClass = false, @EnumChild = false, @Kind = DeclarationKind.METHOD]
| +- MethodDeclaration[@Abstract = false, @Arity = 0, @Default = false, @Final = false, @InterfaceMember = false, @Kind = MethodLikeKind.METHOD, @MethodName = "methodWithLocalClass", @Modifiers = 0, @Name = "methodWithLocalClass", @Native = false, @PackagePrivate = true, @Private = false, @Protected = false, @Public = false, @Static = false, @Strictfp = false, @Synchronized = false, @SyntacticallyAbstract = false, @SyntacticallyPublic = false, @Transient = false, @Void = true, @Volatile = false]
| +- ResultType[@Void = true, @returnsArray = false]
| +- MethodDeclarator[@Image = "methodWithLocalClass", @ParameterCount = 0]
@@ -213,6 +213,34 @@ public class MethodReturnsInternalArrayCase {
]]></code>
</test-code>
<test-code>
<description>#3160 nonempty static final fields</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class MyClass {
private static final String[] FOO_BAR = new String[] { "foo", "bar" };
public final String[] call() { return FOO_BAR; }
}
]]></code>
</test-code>
<test-code>
<description>#3160 empty non-final fields</description>
<expected-problems>4</expected-problems>
<code><![CDATA[
public class MyClass {
private String[] foobar1 = new String[0];
private String[] foobar2 = {};
private static String[] FOO_BAR_3 = new String[0];
private static String[] FOO_BAR_4 = {};
public final String[] call1() { return foobar1; }
public final String[] call2() { return foobar2; }
public final String[] call3() { return FOO_BAR_3; }
public final String[] call4() { return FOO_BAR_4; }
}
]]></code>
</test-code>
<test-code>
<description> #1738 MethodReturnsInternalArray in inner classes</description>
<expected-problems>1</expected-problems>
@@ -366,6 +394,52 @@ public class OuterClass {
return Arrays.copyOf(titles, titles.length);
}
}
}
]]></code>
</test-code>
<test-code>
<description>Nested methods and local class</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class OuterClass {
public int[] arrayReturningMethod() {
class LocalClass {
private String s;
public String getString() {
return this.s;
}
}
LocalClass c = new LocalClass();
return new int[0];
}
}
]]></code>
</test-code>
<test-code>
<description>Detect non-final static arrays returned</description>
<expected-problems>2</expected-problems>
<expected-linenumbers>8,16</expected-linenumbers>
<code><![CDATA[
import java.util.concurrent.Callable;
public class MyClass {
private static final String[] FOO_BAR = new String[] { "foo", "bar" };
private final Callable<String[]> returnsFooBar = new Callable<String[]>() {
@Override
public String[] call() {
return FOO_BAR;
}
};
private static String[] fooBarNonFinal = new String[] { "foo", "bar" };
private final Callable<String[]> returnsFooBarNonFinal = new Callable<String[]>() {
@Override
public String[] call() {
return fooBarNonFinal;
}
};
}
]]></code>
</test-code>
@@ -925,6 +925,26 @@ class TestInvalidLogMessageFormat {
}
private String getBriefDescription() { return ""; }
}
]]></code>
</test-code>
<test-code>
<description>[java] InvalidLogMessageFormat detection failing when String.format used #3146</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.lang.String.format;
class TestInvalidLogMessageFormat {
private static final Logger LOGGER = LoggerFactory.getLogger(TestInvalidLogMessageFormat.class);
public void testPMD() {
LOGGER.info(String.format("Skipping file %s because no parser could be found", getName()));
LOGGER.info(format("Skipping file %s", getName()));
}
private String getName() { return "the-name"; }
}
]]></code>
</test-code>