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

Merge branch 'pr-1068'

Browse Source
This commit is contained in:
Clément Fournier
2018-05-20 01:09:10 +02:00
parent a0ed10c0e7 d2dcbd4f1c
commit 07b8c4f9b2
18 changed files with 123 additions and 128 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pmd-java/etc/grammar/Java.jjt
+4 -5
View File
@@ -374,11 +374,11 @@ public class JavaParser {
// Note that this can't be replaced with a syntactic lookahead
// since "assert" isn't a string literal token
private boolean isNextTokenAnAssert() {
boolean res = getToken(1).image.equals("assert");
if (res && jdkVersion <= 3 && getToken(2).image.equals("(")) {
res = false;
if (jdkVersion <= 3) {
return false;
}
return res;
return getToken(1).image.equals("assert");
}
private boolean isPrecededByComment(Token tok) {
@@ -2379,7 +2379,6 @@ void IfStatement() :
{}
{
"if" "(" Expression() ")" Statement() [ LOOKAHEAD(1) "else" {jjtThis.setHasElse();} Statement() ]
{}
}
void WhileStatement() :
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java
+4
View File
@@ -63,6 +63,10 @@ public class ASTVariableDeclaratorId extends AbstractJavaTypeNode implements Dim
return jjtGetParent().jjtGetParent() instanceof ASTTryStatement;
}
public boolean isFormalParameter() {
return jjtGetParent() instanceof ASTFormalParameter;
}
public void setExplicitReceiverParameter() {
explicitReceiverParameter = true;
}
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java
+6
View File
@@ -28,6 +28,12 @@ public class AvoidReassigningParametersRule extends AbstractJavaRule {
for (Map.Entry<VariableNameDeclaration, List<NameOccurrence>> entry : params.entrySet()) {
VariableNameDeclaration decl = entry.getKey();
List<NameOccurrence> usages = entry.getValue();
// Only look for formal parameters
if (!decl.getDeclaratorId().isFormalParameter()) {
continue;
}
for (NameOccurrence occ : usages) {
JavaNameOccurrence jocc = (JavaNameOccurrence) occ;
if ((jocc.isOnLeftHandSide() || jocc.isSelfAssignment())
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java
+2 -2
View File
@@ -87,8 +87,8 @@ public class UnusedFormalParameterRule extends AbstractJavaRule {
for (Map.Entry<VariableNameDeclaration, List<NameOccurrence>> entry : vars.entrySet()) {
VariableNameDeclaration nameDecl = entry.getKey();
ASTVariableDeclaratorId declNode = (ASTVariableDeclaratorId) nameDecl.getNode();
if (declNode.isExplicitReceiverParameter()) {
ASTVariableDeclaratorId declNode = nameDecl.getDeclaratorId();
if (!declNode.isFormalParameter() || declNode.isExplicitReceiverParameter()) {
continue;
}
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java
+4
View File
@@ -63,6 +63,10 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule {
.getDeclarations(VariableNameDeclaration.class);
for (Map.Entry<VariableNameDeclaration, List<NameOccurrence>> entry : vars.entrySet()) {
VariableNameDeclaration variableDeclaration = entry.getKey();
if (variableDeclaration.getDeclaratorId().isFormalParameter()) {
continue;
}
List<NameOccurrence> usages = entry.getValue();
if (usages.size() == 1) { // If there is more than 1 usage, then it's not only returned
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/ConsecutiveAppendsShouldReuseRule.java
-28
View File
@@ -24,34 +24,6 @@ import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
/**
* Original rule was written with XPath, but didn't verify whether the two calls
* to append would have been done on the same variable.
*
* <pre>
//BlockStatement[./Statement/StatementExpression//PrimaryPrefix/Name[ends-with(@Image,'.append')]
[substring-before(@Image, '.') =
ancestor::Block//LocalVariableDeclaration[./Type//ClassOrInterfaceType[@Image='StringBuffer']]//VariableDeclaratorId/@Image
]
]/following-sibling::*[1][./Statement/StatementExpression//PrimaryPrefix/Name[ends-with(@Image,'.append')]
[substring-before(@Image, '.') =
ancestor::Block//LocalVariableDeclaration[./Type//ClassOrInterfaceType[@Image='StringBuffer']]//VariableDeclaratorId/@Image
]
]
|
//BlockStatement[./Statement/StatementExpression//PrimaryPrefix/Name[ends-with(@Image,'.append')]
[substring-before(@Image, '.') =
ancestor::Block//LocalVariableDeclaration[./Type//ClassOrInterfaceType[@Image='StringBuilder']]//VariableDeclaratorId/@Image
]
]/following-sibling::*[1][./Statement/StatementExpression//PrimaryPrefix/Name[ends-with(@Image,'.append')]
[substring-before(@Image, '.') =
ancestor::Block//LocalVariableDeclaration[./Type//ClassOrInterfaceType[@Image='StringBuilder']]//VariableDeclaratorId/@Image
]
]
*
* </pre>
*
*/
public class ConsecutiveAppendsShouldReuseRule extends AbstractJavaRule {
@Override
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/LocalScope.java
+4
View File
@@ -24,6 +24,7 @@ public class LocalScope extends AbstractJavaScope {
return getDeclarations(VariableNameDeclaration.class);
}
@Override
public Set<NameDeclaration> addNameOccurrence(NameOccurrence occurrence) {
JavaNameOccurrence javaOccurrence = (JavaNameOccurrence) occurrence;
Set<NameDeclaration> declarations = findVariableHere(javaOccurrence);
@@ -40,6 +41,7 @@ public class LocalScope extends AbstractJavaScope {
return declarations;
}
@Override
public void addDeclaration(NameDeclaration nameDecl) {
if (!(nameDecl instanceof VariableNameDeclaration || nameDecl instanceof ClassNameDeclaration)) {
throw new IllegalArgumentException(
@@ -49,6 +51,7 @@ public class LocalScope extends AbstractJavaScope {
super.addDeclaration(nameDecl);
}
@Override
public Set<NameDeclaration> findVariableHere(JavaNameOccurrence occurrence) {
if (occurrence.isThisOrSuper() || occurrence.isMethodOrConstructorInvocation()) {
return Collections.emptySet();
@@ -61,6 +64,7 @@ public class LocalScope extends AbstractJavaScope {
return Collections.emptySet();
}
@Override
public String toString() {
return "LocalScope:" + glomNames(getVariableDeclarations().keySet());
}
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/MethodScope.java
+5 -2
View File
@@ -13,7 +13,6 @@ import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.symboltable.Applier;
import net.sourceforge.pmd.lang.symboltable.ImageFinderFunction;
import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
@@ -33,6 +32,7 @@ public class MethodScope extends AbstractJavaScope {
return getDeclarations(VariableNameDeclaration.class);
}
@Override
public Set<NameDeclaration> addNameOccurrence(NameOccurrence occurrence) {
JavaNameOccurrence javaOccurrence = (JavaNameOccurrence) occurrence;
Set<NameDeclaration> declarations = findVariableHere(javaOccurrence);
@@ -48,6 +48,7 @@ public class MethodScope extends AbstractJavaScope {
return declarations;
}
@Override
public void addDeclaration(NameDeclaration variableDecl) {
if (!(variableDecl instanceof VariableNameDeclaration || variableDecl instanceof ClassNameDeclaration)) {
throw new IllegalArgumentException(
@@ -56,11 +57,12 @@ public class MethodScope extends AbstractJavaScope {
super.addDeclaration(variableDecl);
}
@Override
public Set<NameDeclaration> findVariableHere(JavaNameOccurrence occurrence) {
if (occurrence.isThisOrSuper() || occurrence.isMethodOrConstructorInvocation()) {
return Collections.emptySet();
}
ImageFinderFunction finder = new ImageFinderFunction(occurrence.getImage());
DeclarationFinderFunction finder = new DeclarationFinderFunction(occurrence);
Applier.apply(finder, getVariableDeclarations().keySet().iterator());
if (finder.getDecl() != null) {
return Collections.singleton(finder.getDecl());
@@ -75,6 +77,7 @@ public class MethodScope extends AbstractJavaScope {
return node.jjtGetChild(1).getImage();
}
@Override
public String toString() {
return "MethodScope:" + glomNames(getVariableDeclarations().keySet());
}
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ScopeAndDeclarationFinder.java
+13 -63
View File
@@ -7,20 +7,15 @@ package net.sourceforge.pmd.lang.java.symboltable;
import java.util.ArrayDeque;
import java.util.Deque;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
import net.sourceforge.pmd.lang.java.ast.ASTCatchStatement;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFinallyStatement;
import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
@@ -28,7 +23,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement;
import net.sourceforge.pmd.lang.java.ast.ASTTryStatement;
import net.sourceforge.pmd.lang.java.ast.ASTTypeParameters;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.AbstractJavaNode;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
@@ -207,8 +201,18 @@ public class ScopeAndDeclarationFinder extends JavaParserVisitorAdapter {
@Override
public Object visit(ASTBlock node, Object data) {
createLocalScope(node);
cont(node);
// top-level blocks for methods should have the same scope as parameters, just skip them
// same applies to catch statements defining exceptions + the catch block, and for-blocks
if (node.jjtGetParent() instanceof ASTMethodDeclaration
|| node.jjtGetParent() instanceof ASTConstructorDeclaration
|| node.jjtGetParent() instanceof ASTLambdaExpression
|| node.jjtGetParent() instanceof ASTCatchStatement
|| node.jjtGetParent() instanceof ASTForStatement) {
super.visit(node, null);
} else {
createLocalScope(node);
cont(node);
}
return data;
}
@@ -219,57 +223,10 @@ public class ScopeAndDeclarationFinder extends JavaParserVisitorAdapter {
return data;
}
@Override
public Object visit(ASTFinallyStatement node, Object data) {
createLocalScope(node);
cont(node);
return data;
}
@Override
public Object visit(ASTConstructorDeclaration node, Object data) {
/*
* Local variables declared inside the constructor need to be in a
* different scope so special handling is needed
*/
createMethodScope(node);
Scope methodScope = node.getScope();
Node formalParameters = node.jjtGetChild(0);
int i = 1;
int n = node.jjtGetNumChildren();
if (!(formalParameters instanceof ASTFormalParameters)) {
visit((ASTTypeParameters) formalParameters, data);
formalParameters = node.jjtGetChild(1);
i++;
}
visit((ASTFormalParameters) formalParameters, data);
Scope localScope = null;
for (; i < n; i++) {
JavaNode b = (JavaNode) node.jjtGetChild(i);
if (b instanceof ASTBlockStatement) {
if (localScope == null) {
createLocalScope(node);
localScope = node.getScope();
}
b.setScope(localScope);
visit(b, data);
} else {
visit(b, data);
}
}
if (localScope != null) {
// pop the local scope
scopes.pop();
// reset the correct scope for the constructor
node.setScope(methodScope);
}
// pop the method scope
scopes.pop();
cont(node);
return data;
}
@@ -304,13 +261,6 @@ public class ScopeAndDeclarationFinder extends JavaParserVisitorAdapter {
return data;
}
@Override
public Object visit(ASTIfStatement node, Object data) {
createLocalScope(node);
cont(node);
return data;
}
@Override
public Object visit(ASTVariableDeclaratorId node, Object data) {
VariableNameDeclaration decl = new VariableNameDeclaration(node);
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java
+13 -5
View File
@@ -54,15 +54,23 @@ public class VariableNameDeclaration extends AbstractNameDeclaration implements
}
public boolean isExceptionBlockParameter() {
return ((ASTVariableDeclaratorId) node).isExceptionBlockParameter();
return getDeclaratorId().isExceptionBlockParameter();
}
/**
* @deprecated use {@link #isTypeInferred()}
*/
@Deprecated
public boolean isLambdaTypelessParameter() {
return getAccessNodeParent() instanceof ASTLambdaExpression;
return isTypeInferred();
}
public boolean isTypeInferred() {
return getDeclaratorId().isTypeInferred();
}
public boolean isPrimitiveType() {
return !isLambdaTypelessParameter()
return !isTypeInferred()
&& getAccessNodeParent().getFirstChildOfType(ASTType.class).jjtGetChild(0) instanceof ASTPrimitiveType;
}
@@ -78,7 +86,7 @@ public class VariableNameDeclaration extends AbstractNameDeclaration implements
* Note that an array of primitive types (int[]) is a reference type.
*/
public boolean isReferenceType() {
return !isLambdaTypelessParameter()
return !isTypeInferred()
&& getAccessNodeParent().getFirstChildOfType(ASTType.class).jjtGetChild(0) instanceof ASTReferenceType;
}
@@ -97,7 +105,7 @@ public class VariableNameDeclaration extends AbstractNameDeclaration implements
if (isPrimitiveType()) {
return (TypeNode) getAccessNodeParent().getFirstChildOfType(ASTType.class).jjtGetChild(0);
}
if (!isLambdaTypelessParameter()) {
if (!isTypeInferred()) {
return (TypeNode) getAccessNodeParent().getFirstChildOfType(ASTType.class).jjtGetChild(0).jjtGetChild(0);
}
return null;
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/AcceptanceTest.java
+1 -1
View File
@@ -49,7 +49,7 @@ public class AcceptanceTest extends STBBaseTst {
ASTCatchStatement c = acu.findDescendantsOfType(ASTCatchStatement.class).get(0);
ASTBlock a = c.findDescendantsOfType(ASTBlock.class).get(0);
Scope s = a.getScope();
Map<NameDeclaration, List<NameOccurrence>> vars = s.getParent().getDeclarations();
Map<NameDeclaration, List<NameOccurrence>> vars = s.getDeclarations();
assertEquals(1, vars.size());
NameDeclaration v = vars.keySet().iterator().next();
assertEquals("e", v.getImage());
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/ClassScopeTest.java
+16 -11
View File
@@ -280,20 +280,25 @@ public class ClassScopeTest extends STBBaseTst {
public void testNestedClassFieldAndParameter() {
parseCode(NESTED_CLASS_FIELD_AND_PARAM);
ASTMethodDeclaration node = acu.getFirstDescendantOfType(ASTMethodDeclaration.class);
Map<NameDeclaration, List<NameOccurrence>> vd = node.getScope().getDeclarations();
assertEquals(1, vd.size());
Map<VariableNameDeclaration, List<NameOccurrence>> vd = node.getScope().getDeclarations(VariableNameDeclaration.class);
assertEquals(2, vd.size());
for (Map.Entry<NameDeclaration, List<NameOccurrence>> entry : vd.entrySet()) {
assertEquals("field", entry.getKey().getImage());
List<NameOccurrence> occurrences = entry.getValue();
assertEquals(2, occurrences.size());
NameOccurrence no1 = occurrences.get(0);
assertEquals(8, no1.getLocation().getBeginLine());
NameOccurrence no2 = occurrences.get(1);
assertEquals(9, no2.getLocation().getBeginLine());
int paramCount = 0;
for (Map.Entry<VariableNameDeclaration, List<NameOccurrence>> entry : vd.entrySet()) {
if (entry.getKey().getDeclaratorId().isFormalParameter()) {
assertEquals("field", entry.getKey().getImage());
List<NameOccurrence> occurrences = entry.getValue();
assertEquals(2, occurrences.size());
NameOccurrence no1 = occurrences.get(0);
assertEquals(8, no1.getLocation().getBeginLine());
NameOccurrence no2 = occurrences.get(1);
assertEquals(9, no2.getLocation().getBeginLine());
paramCount++;
}
}
assertEquals(1, paramCount);
}
@Test
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/LocalScopeTest.java
+1 -5
View File
@@ -119,12 +119,8 @@ public class LocalScopeTest extends STBBaseTst {
public static final String TEST3 = "public class Foo {" + PMD.EOL + " void foo() {" + PMD.EOL + " int x = 2;"
+ PMD.EOL + " x++;" + PMD.EOL + " }" + PMD.EOL + "}";
public static final String TEST4 = "public class Foo {" + PMD.EOL + " void foo(String x, String z) { int y; }"
public static final String TEST4 = "public class Foo {" + PMD.EOL + " void foo(String x, String z) { { int x; } }"
+ PMD.EOL + "}";
public static final String TEST5 = "public class Foo {" + PMD.EOL + " void foo(String x);" + PMD.EOL + "}";
public static junit.framework.Test suite() {
return new junit.framework.JUnit4TestAdapter(LocalScopeTest.class);
}
}
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/ScopeCreationVisitorTest.java
+3 -1
View File
@@ -9,6 +9,7 @@ import static org.junit.Assert.assertTrue;
import org.junit.Test;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
public class ScopeCreationVisitorTest extends STBBaseTst {
@@ -16,7 +17,8 @@ public class ScopeCreationVisitorTest extends STBBaseTst {
@Test
public void testScopesAreCreated() {
parseCode(TEST1);
ASTIfStatement n = acu.findDescendantsOfType(ASTIfStatement.class).get(0);
ASTBlock n = acu.getFirstDescendantOfType(ASTIfStatement.class)
.getFirstDescendantOfType(ASTBlock.class);
assertTrue(n.getScope() instanceof LocalScope);
}
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidAssertAsIdentifier.xml
+16 -2
View File
@@ -5,12 +5,26 @@
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description><![CDATA[
assert as identifier
assert as identifier - parameter
]]></description>
<expected-problems>2</expected-problems>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
void bar(String assert) {
assert = "hi";
}
}
]]></code>
<source-type>java 1.3</source-type>
</test-code>
<test-code>
<description><![CDATA[
assert as identifier - local var
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
void bar() {
String assert = "hi";
}
}
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidEnumAsIdentifier.xml
+17 -3
View File
@@ -5,14 +5,28 @@
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description><![CDATA[
variable and param named enum
variable named enum
]]></description>
<expected-problems>2</expected-problems>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
void bar(String enum) {
void bar() {
String enum = "hi";
}
}
]]></code>
<source-type>java 1.4</source-type>
</test-code>
<test-code>
<description><![CDATA[
param named enum
]]></description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
void bar(String enum) {
enum = "hi";
}
}
]]></code>
<source-type>java 1.4</source-type>
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/ConsecutiveAppendsShouldReuse.xml
+13
View File
@@ -250,6 +250,19 @@ public class Foo {
stringBuffer = new StringBuffer().append("agrego ").append("un ");
stringBuffer.append("string ");
}
}
]]></code>
</test-code>
<test-code>
<description>#1051 ConsecutiveAppendsShouldReuse not detect on parameter</description>
<expected-problems>2</expected-problems>
<code><![CDATA[
public class Foo {
public static void asd(StringBuilder builder, String b) {
builder.append("asd");
builder.append(b);
builder.append(123);
}
}
]]></code>
</test-code>