findDescendantsOfType defaults to not crossing boundaries

- A few places actually need to do so, and some other were simply wrong
 - We can now cross the boundary if searching dowm from it
 - Anonymous inner classes are still somewhat inconsistent
This commit is contained in:
Juan Martín Sotuyo Dodero
2018-03-05 02:13:14 -03:00
parent cdc50f7667
commit 8deaf20f4d
13 changed files with 62 additions and 72 deletions

View File

@ -237,15 +237,13 @@ public abstract class AbstractNode implements Node {
return parents;
}
@Override
public <T> List<T> findDescendantsOfType(Class<T> targetType) {
List<T> list = new ArrayList<>();
findDescendantsOfType(this, targetType, list, true);
findDescendantsOfType(this, targetType, list, false);
return list;
}
@Override
public <T> void findDescendantsOfType(Class<T> targetType, List<T> results, boolean crossBoundaries) {
findDescendantsOfType(this, targetType, results, crossBoundaries);
@ -254,17 +252,15 @@ public abstract class AbstractNode implements Node {
private static <T> void findDescendantsOfType(Node node, Class<T> targetType, List<T> results,
boolean crossFindBoundaries) {
if (!crossFindBoundaries && node.isFindBoundary()) {
return;
}
for (int i = 0; i < node.jjtGetNumChildren(); i++) {
Node child = node.jjtGetChild(i);
if (child.getClass() == targetType) {
results.add(targetType.cast(child));
}
findDescendantsOfType(child, targetType, results, crossFindBoundaries);
if (crossFindBoundaries || !child.isFindBoundary()) {
findDescendantsOfType(child, targetType, results, crossFindBoundaries);
}
}
}
@ -341,19 +337,17 @@ public abstract class AbstractNode implements Node {
private static <T> T getFirstDescendantOfType(final Class<T> descendantType, final Node node) {
if (node.isFindBoundary()) {
return null;
}
final int n = node.jjtGetNumChildren();
for (int i = 0; i < n; i++) {
Node n1 = node.jjtGetChild(i);
if (descendantType.isAssignableFrom(n1.getClass())) {
return descendantType.cast(n1);
}
final T n2 = getFirstDescendantOfType(descendantType, n1);
if (n2 != null) {
return n2;
if (!n1.isFindBoundary()) {
final T n2 = getFirstDescendantOfType(descendantType, n1);
if (n2 != null) {
return n2;
}
}
}
return null;

View File

@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.java.ast;
import java.util.ArrayList;
import java.util.List;
import net.sourceforge.pmd.lang.ast.Node;
@ -35,13 +36,15 @@ public class ASTClassOrInterfaceType extends AbstractJavaTypeNode {
*/
public boolean isReferenceToClassSameCompilationUnit() {
ASTCompilationUnit root = getFirstParentOfType(ASTCompilationUnit.class);
List<ASTClassOrInterfaceDeclaration> classes = root.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class);
List<ASTClassOrInterfaceDeclaration> classes = new ArrayList<>();
root.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class, classes, true);
for (ASTClassOrInterfaceDeclaration c : classes) {
if (c.hasImageEqualTo(getImage())) {
return true;
}
}
List<ASTEnumDeclaration> enums = root.findDescendantsOfType(ASTEnumDeclaration.class);
List<ASTEnumDeclaration> enums = new ArrayList<>();
root.findDescendantsOfType(ASTEnumDeclaration.class, enums, true);
for (ASTEnumDeclaration e : enums) {
if (e.hasImageEqualTo(getImage())) {
return true;

View File

@ -103,7 +103,7 @@ public class AtfdBaseVisitor extends JavaParserVisitorAdapter {
private boolean isAttributeAccess(ASTPrimaryExpression node) {
return node.findDescendantsOfType(ASTPrimarySuffix.class).isEmpty();
return !node.hasDescendantOfType(ASTPrimarySuffix.class);
}
}

View File

@ -4,6 +4,7 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@ -139,7 +140,8 @@ public class PreserveStackTraceRule extends AbstractJavaRule {
private boolean checkForTargetUsage(String target, Node baseNode) {
boolean match = false;
if (target != null && baseNode != null) {
List<ASTName> nameNodes = baseNode.findDescendantsOfType(ASTName.class);
List<ASTName> nameNodes = new ArrayList<>();
baseNode.findDescendantsOfType(ASTName.class, nameNodes, true);
for (ASTName nameNode : nameNodes) {
if (target.equals(nameNode.getImage())) {
boolean isPartOfStringConcatenation = isStringConcat(nameNode, baseNode);

View File

@ -100,14 +100,16 @@ public class UnusedPrivateFieldRule extends AbstractLombokAwareRule {
nodes.addAll(enumConstants);
for (JavaNode node : nodes) {
List<ASTPrimarySuffix> primarySuffixes = node.findDescendantsOfType(ASTPrimarySuffix.class);
List<ASTPrimarySuffix> primarySuffixes = new ArrayList<>();
node.findDescendantsOfType(ASTPrimarySuffix.class, primarySuffixes, true);
for (ASTPrimarySuffix primarySuffix : primarySuffixes) {
if (decl.getImage().equals(primarySuffix.getImage())) {
return true; // No violation
}
}
List<ASTPrimaryPrefix> primaryPrefixes = node.findDescendantsOfType(ASTPrimaryPrefix.class);
List<ASTPrimaryPrefix> primaryPrefixes = new ArrayList<>();
node.findDescendantsOfType(ASTPrimaryPrefix.class, primaryPrefixes, true);
for (ASTPrimaryPrefix primaryPrefix : primaryPrefixes) {
ASTName name = primaryPrefix.getFirstDescendantOfType(ASTName.class);

View File

@ -117,14 +117,9 @@ public class CommentDefaultAccessModifierRule extends AbstractCommentRule {
if (parent != null) {
List<ASTAnnotation> annotations = parent.findChildrenOfType(ASTAnnotation.class);
for (ASTAnnotation annotation : annotations) {
List<ASTName> names = annotation.findDescendantsOfType(ASTName.class);
for (ASTName name : names) {
if (name.hasImageEqualTo("VisibleForTesting")) {
result = false;
break;
}
}
if (result == false) {
final ASTName name = annotation.getFirstDescendantOfType(ASTName.class);
if (name.hasImageEqualTo("VisibleForTesting")) {
result = false;
break;
}
}

View File

@ -4,50 +4,37 @@
package net.sourceforge.pmd.lang.java.rule.design;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.properties.IntegerProperty;
import net.sourceforge.pmd.util.NumericConstants;
public class TooManyFieldsRule extends AbstractJavaRule {
private static final int DEFAULT_MAXFIELDS = 15;
private Map<String, Integer> stats;
private Map<String, ASTClassOrInterfaceDeclaration> nodes;
private static final IntegerProperty MAX_FIELDS_DESCRIPTOR = new IntegerProperty("maxfields",
"Max allowable fields", 1, 300, DEFAULT_MAXFIELDS, 1.0f);
public TooManyFieldsRule() {
definePropertyDescriptor(MAX_FIELDS_DESCRIPTOR);
addRuleChainVisit(ASTClassOrInterfaceDeclaration.class);
}
@Override
public Object visit(ASTCompilationUnit node, Object data) {
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
final int maxFields = getProperty(MAX_FIELDS_DESCRIPTOR);
int counter = 0;
int maxFields = getProperty(MAX_FIELDS_DESCRIPTOR);
stats = new HashMap<>(5);
nodes = new HashMap<>(5);
List<ASTFieldDeclaration> l = node.findDescendantsOfType(ASTFieldDeclaration.class);
final List<ASTFieldDeclaration> l = node.findDescendantsOfType(ASTFieldDeclaration.class);
for (ASTFieldDeclaration fd : l) {
if (fd.isFinal() && fd.isStatic()) {
continue;
}
ASTClassOrInterfaceDeclaration clazz = fd.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class);
if (clazz != null && !clazz.isInterface()) {
bumpCounterFor(clazz);
}
counter++;
}
for (Map.Entry<String, Integer> entry : stats.entrySet()) {
int val = entry.getValue();
@ -56,16 +43,7 @@ public class TooManyFieldsRule extends AbstractJavaRule {
addViolation(data, n);
}
}
return data;
}
private void bumpCounterFor(ASTClassOrInterfaceDeclaration clazz) {
String key = clazz.getImage();
if (!stats.containsKey(key)) {
stats.put(key, NumericConstants.ZERO);
nodes.put(key, clazz);
}
Integer i = Integer.valueOf(stats.get(key) + 1);
stats.put(key, i);
}
}

View File

@ -199,22 +199,26 @@ public abstract class AbstractCommentRule extends AbstractJavaRule {
SortedMap<Integer, Node> itemsByLineNumber = new TreeMap<>();
List<ASTClassOrInterfaceDeclaration> packageDecl = cUnit
.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class);
addDeclarations(itemsByLineNumber, packageDecl);
List<ASTClassOrInterfaceDeclaration> classDecl = new ArrayList<>();
cUnit.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class, classDecl, true);
addDeclarations(itemsByLineNumber, classDecl);
addDeclarations(itemsByLineNumber, cUnit.getComments());
List<ASTFieldDeclaration> fields = cUnit.findDescendantsOfType(ASTFieldDeclaration.class);
List<ASTFieldDeclaration> fields = new ArrayList<>();
cUnit.findDescendantsOfType(ASTFieldDeclaration.class, fields, true);
addDeclarations(itemsByLineNumber, fields);
List<ASTMethodDeclaration> methods = cUnit.findDescendantsOfType(ASTMethodDeclaration.class);
List<ASTMethodDeclaration> methods = new ArrayList<>();
cUnit.findDescendantsOfType(ASTMethodDeclaration.class, methods, true);
addDeclarations(itemsByLineNumber, methods);
List<ASTConstructorDeclaration> constructors = cUnit.findDescendantsOfType(ASTConstructorDeclaration.class);
List<ASTConstructorDeclaration> constructors = new ArrayList<>();
cUnit.findDescendantsOfType(ASTConstructorDeclaration.class, constructors, true);
addDeclarations(itemsByLineNumber, constructors);
List<ASTEnumDeclaration> enumDecl = cUnit.findDescendantsOfType(ASTEnumDeclaration.class);
List<ASTEnumDeclaration> enumDecl = new ArrayList<>();
cUnit.findDescendantsOfType(ASTEnumDeclaration.class, enumDecl, true);
addDeclarations(itemsByLineNumber, enumDecl);
return itemsByLineNumber;

View File

@ -104,13 +104,13 @@ public class CloseResourceRule extends AbstractJavaRule {
@Override
public Object visit(ASTConstructorDeclaration node, Object data) {
checkForResources(node, data);
return data;
return super.visit(node, data);
}
@Override
public Object visit(ASTMethodDeclaration node, Object data) {
checkForResources(node, data);
return data;
return super.visit(node, data);
}
private void checkForResources(Node node, Object data) {

View File

@ -47,7 +47,8 @@ public class ASTVariableDeclaratorIdTest {
@Test
public void testLambdaWithType() throws Exception {
ASTCompilationUnit acu = parseJava18(TEST_LAMBDA_WITH_TYPE);
ASTVariableDeclaratorId f = acu.findDescendantsOfType(ASTVariableDeclaratorId.class).get(1);
ASTLambdaExpression lambda = acu.getFirstDescendantOfType(ASTLambdaExpression.class);
ASTVariableDeclaratorId f = lambda.getFirstDescendantOfType(ASTVariableDeclaratorId.class);
assertEquals("File", f.getTypeNode().getTypeImage());
assertEquals("File", f.getTypeNameNode().jjtGetChild(0).getImage());
}
@ -55,7 +56,8 @@ public class ASTVariableDeclaratorIdTest {
@Test
public void testLambdaWithoutType() throws Exception {
ASTCompilationUnit acu = parseJava18(TEST_LAMBDA_WITHOUT_TYPE);
ASTVariableDeclaratorId f = acu.findDescendantsOfType(ASTVariableDeclaratorId.class).get(1);
ASTLambdaExpression lambda = acu.getFirstDescendantOfType(ASTLambdaExpression.class);
ASTVariableDeclaratorId f = lambda.getFirstDescendantOfType(ASTVariableDeclaratorId.class);
assertNull(f.getTypeNode());
assertNull(f.getTypeNameNode());
}

View File

@ -17,6 +17,7 @@ import static org.junit.Assert.fail;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import org.apache.commons.io.IOUtils;
@ -276,7 +277,8 @@ public class JDKVersionTest {
@Test
public final void jdk7PrivateMethodInnerClassInterface1() {
ASTCompilationUnit acu = parseJava17(loadSource("private_method_in_inner_class_interface1.java"));
List<ASTMethodDeclaration> methods = acu.findDescendantsOfType(ASTMethodDeclaration.class);
List<ASTMethodDeclaration> methods = new ArrayList<>();
acu.findDescendantsOfType(ASTMethodDeclaration.class, methods, true);
assertEquals(3, methods.size());
for (ASTMethodDeclaration method : methods) {
assertFalse(method.isInterfaceMember());

View File

@ -18,6 +18,7 @@ import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
import net.sourceforge.pmd.lang.java.ast.ASTCatchStatement;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
@ -129,7 +130,8 @@ public class AcceptanceTest extends STBBaseTst {
@Test
public void testInnerOuterClass() {
parseCode(TEST_INNER_CLASS);
ASTVariableDeclaratorId vdi = acu.findDescendantsOfType(ASTVariableDeclaratorId.class).get(0);
ASTVariableDeclaratorId vdi = acu.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class).get(1) // get inner class
.findDescendantsOfType(ASTVariableDeclaratorId.class).get(0); // get first declaration
List<NameOccurrence> usages = vdi.getUsages();
assertEquals(2, usages.size());
assertEquals(5, usages.get(0).getLocation().getBeginLine());

View File

@ -4,6 +4,7 @@
package net.sourceforge.pmd.lang.java.symboltable;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
@ -13,6 +14,8 @@ import org.junit.Test;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.java.JavaLanguageModule;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
@ -65,7 +68,10 @@ public class ScopeAndDeclarationFinderTest extends STBBaseTst {
ClassScope cs = (ClassScope) acu.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class).getScope();
Assert.assertEquals(1, cs.getClassDeclarations().size()); // There should be 1 anonymous class
List<ASTMethodDeclarator> methods = acu.findDescendantsOfType(ASTMethodDeclarator.class);
List<ASTMethodDeclarator> methods = new ArrayList<>();
acu.getFirstDescendantOfType(ASTClassOrInterfaceBody.class) // outer class
.getFirstDescendantOfType(ASTClassOrInterfaceBody.class) // inner class
.findDescendantsOfType(ASTMethodDeclarator.class, methods, true); // inner class methods
Assert.assertEquals(2, methods.size());
ClassScope scope1 = methods.get(0).getScope().getEnclosingScope(ClassScope.class);
ClassScope scope2 = methods.get(1).getScope().getEnclosingScope(ClassScope.class);