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

Simplified getter and setter detection

Browse Source
This commit is contained in:
oowekyala
2017-06-27 07:01:32 +02:00
parent a4af2220ec
commit d5f69e5f81
4 changed files with 37 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ClassStats.java
View File

@ -128,8 +128,7 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature;
/**
* Checks whether the class declares an operation by the name given which is covered by the
* signature mask.
* Checks whether the class declares an operation by the name given which is covered by the signature mask.
*
* @param name The name of the operation to look for
* @param mask The mask covering accepted signatures
@ -151,8 +150,7 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature;
/**
* Checks whether the class declares a field by the name given which is covered by the
* signature mask.
* Checks whether the class declares a field by the name given which is covered by the signature mask.
*
* @param name The name of the field to look for
* @param mask The mask covering accepted signatures
@ -226,7 +224,7 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature;
/* default */ double compute(OperationMetricKey key, ASTMethodOrConstructorDeclaration node, String name,
boolean force, MetricVersion version) {
// TODO:cf the operation signature might be built many times, consider profiling
// TODO:cf the operation signature might be built many times, consider storing it in the node
Map<String, OperationStats> sigMap = operations.get(OperationSignature.buildFor(node));
if (sigMap == null) {

3
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/Metrics.java
View File

@ -33,8 +33,7 @@ public final class Metrics {
*
* @return The top level package stats
*/
/* default */
static PackageStats getTopLevelPackageStats() {
/* default */ static PackageStats getTopLevelPackageStats() {
return TOP_LEVEL_PACKAGE;
}

106
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/signature/OperationSignature.java
View File

@ -5,24 +5,16 @@
package net.sourceforge.pmd.lang.java.oom.signature;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
@ -35,8 +27,8 @@ import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
*/
public final class OperationSignature extends Signature {
private static final Map<Integer, OperationSignature> POOL = new HashMap<>();
private static final Map<Integer, OperationSignature> POOL = new HashMap<>();
public final Role role;
public final boolean isAbstract;
@ -47,6 +39,19 @@ public final class OperationSignature extends Signature {
this.isAbstract = isAbstract;
}
@Override
public boolean equals(Object o) {
return this == o;
}
@Override
public int hashCode() {
return (isAbstract ? 1 : 0) + super.hashCode() << 1 + role.hashCode() << 2;
}
/**
* Builds an operation signature from a method or constructor declaration.
*
@ -62,20 +67,12 @@ public final class OperationSignature extends Signature {
return POOL.get(code);
}
/** Used internally by the pooler. */
private static int code(Visibility visibility, Role role, boolean isAbstract) {
return visibility.hashCode() * 31 + role.hashCode() * 2 + (isAbstract ? 1 : 0);
}
@Override
public boolean equals(Object o) {
return this == o;
}
@Override
public int hashCode() {
return (isAbstract ? 1 : 0) + super.hashCode() << 1 + role.hashCode() << 2;
}
/**
* Role of an operation.
@ -83,7 +80,8 @@ public final class OperationSignature extends Signature {
public enum Role {
GETTER_OR_SETTER, CONSTRUCTOR, METHOD, STATIC;
private static final Pattern NAME_PATTERN = Pattern.compile("(?:get|set|is|increment|decrement)\\w*");
private static final Pattern GETTER_OR_SETTER_NAME_PATTERN = Pattern.compile("(?:get|set|is)\\w*");
public static Role get(ASTMethodOrConstructorDeclaration node) {
@ -101,25 +99,12 @@ public final class OperationSignature extends Signature {
}
}
// TODO:cf clean that up
private static boolean isGetterOrSetter(ASTMethodDeclaration node) {
String name = node.getName();
if (NAME_PATTERN.matcher(name).matches()) {
if (GETTER_OR_SETTER_NAME_PATTERN.matcher(name).matches()) {
return true;
}
if (node.isAbstract()) {
return false;
}
int length = node.getEndLine() - node.getBeginLine();
if (length > 6) {
return false;
} else if (length > 4 && node.getFirstDescendantOfType(ASTIfStatement.class) == null) {
return false;
}
ClassScope scope = node.getScope().getEnclosingScope(ClassScope.class);
// fields names mapped to their types
@ -137,62 +122,27 @@ public final class OperationSignature extends Signature {
return isGetter(node, fieldNames) || isSetter(node, fieldNames);
}
/** Attempts to determine if the method is a getter. */
private static boolean isGetter(ASTMethodDeclaration node, Map<String, String> fieldNames) {
List<ASTReturnStatement> returnStatements
= node.getBlock().findDescendantsOfType(ASTReturnStatement.class);
for (ASTReturnStatement st : returnStatements) {
ASTName name = st.getFirstDescendantOfType(ASTName.class);
if (name == null) {
continue;
}
if (fieldNames.containsKey(name.getImage().split("\\.")[0])) {
return true;
}
}
if (node.getFirstDescendantOfType(ASTFormalParameters.class).getParameterCount() != 0
|| node.getFirstDescendantOfType(ASTResultType.class).isVoid()) {
return false;
}
return fieldNames.containsKey(node.getName());
}
/** Attempts to determine if the method is a setter. */
private static boolean isSetter(ASTMethodDeclaration node, Map<String, String> fieldNames) {
if (node.getFirstDescendantOfType(ASTFormalParameters.class).jjtGetNumChildren() != 1) {
if (node.getFirstDescendantOfType(ASTFormalParameters.class).getParameterCount() != 1) {
return false;
}
List<ASTStatementExpression> statementExpressions
= node.getBlock().findDescendantsOfType(ASTStatementExpression.class);
Set<String> namesToCheck = new HashSet<>();
for (ASTStatementExpression st : statementExpressions) {
ASTName name = st.getFirstDescendantOfType(ASTName.class);
if (name == null) {
// not an assignment, check for method
ASTPrimaryExpression prim = st.getFirstChildOfType(ASTPrimaryExpression.class);
ASTPrimaryPrefix prefix = prim.getFirstChildOfType(ASTPrimaryPrefix.class);
if (prefix.usesThisModifier() || prefix.usesSuperModifier()) {
namesToCheck.add(prim.getFirstChildOfType(ASTPrimarySuffix.class).getImage());
} else {
namesToCheck.add(prefix.getImage().split("\\.")[0]);
}
} else {
// this is a direct assignment
namesToCheck.add(name.getImage().split("\\.")[0]);
}
}
for (String name : namesToCheck) {
if (fieldNames.containsKey(name)) {
return true;
}
}
return false;
return node.getFirstDescendantOfType(ASTResultType.class).isVoid();
}
}
}

8
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/oom/testdata/GetterDetection.java vendored
View File

@ -16,19 +16,23 @@ public class GetterDetection {
private MutableInt mutX;
private boolean bool;
public int getValue() {
return value;
}
public boolean isBooleanTrue() {
return bool;
}
public int value() {
return value;
}
public double speedModified() {
/* public double speedModified() {
return speed * 12 + 1;
}
@ -51,5 +55,5 @@ public class GetterDetection {
public int mutableIntConditional() {
return mutX == null ? 0 : mutX.getValue();
}
*/
}