Merge branch 'pr-578'
This commit is contained in:
commit
d31d0dd7cd
22 files changed
+799
-398
No files matched your search
@@ -11,10 +11,18 @@ toc:
|
||||
|
||||
## Access to Foreign Data (ATFD)
|
||||
|
||||
*Operation metric, class metric.*
|
||||
*Operation metric, class metric.* Can be computed on classes, enums and
|
||||
concrete operations.
|
||||
|
||||
### Description
|
||||
|
||||
Number of usages of foreign attributes, both directly and through accessors.
|
||||
High values of ATFD (> 3 for an operation) may suggest that the class or operation
|
||||
breaks encapsulation by relying on the internal representation of the classes
|
||||
it uses instead of the services they provide.
|
||||
|
||||
ATFD can be used to detect God Classes and Feature Envy. \[[Lanza05](#Lanza05)\]
|
||||
|
||||
|
||||
## Cyclomatic Complexity (CYCLO)
|
||||
|
||||
@@ -226,11 +234,30 @@ between blocks 4 and 5 before jumping to block 6. The first `if` offers 2
|
||||
choices, the second offers 3, so the cyclomatic complexity of this method is
|
||||
2 + 3 = 5. NPath, however, sees 2 * 3 = 6 full paths from the beginning to the end.
|
||||
|
||||
|
||||
## Number Of Public Attributes (NOPA)
|
||||
*Class metric.* Can be computed on classes.
|
||||
|
||||
## Number Of Accessor Methods (NOAM)
|
||||
*Class metric.* Can be computed on classes.
|
||||
|
||||
## Tight Class Cohesion (TCC)
|
||||
|
||||
*Class metric.* Can be computed on classes and enums.
|
||||
|
||||
### Description
|
||||
|
||||
The relative number of method pairs of a class that access in common at
|
||||
least one attribute of the measured class. TCC only counts
|
||||
direct attribute accesses, that is, only those attributes that are accessed in
|
||||
the body of the method \[[BK95](#BK95)\].
|
||||
|
||||
TCC is taken to be a reliable cohesion metric for a class. High values (>70%)
|
||||
indicate a class with one basic function, which is hard to break into subcomponents.
|
||||
On the other hand, low values (<50%) may indicate that the class tries to do too much and
|
||||
defines several unrelated services, which is undesirable.
|
||||
|
||||
TCC can be used to detect God Classes and Brain Classes \[[Lanza05](#Lanza05)\].
|
||||
|
||||
## Weighted Method Count (WMC)
|
||||
|
||||
@@ -268,6 +295,10 @@ This metric is used to detect Data Classes, in conjunction with [WMC](#weighted-
|
||||
|
||||
# References
|
||||
|
||||
|
||||
<a name="BK95">BK95:</a> Bieman, Kang; Cohesion and reuse in an object-oriented system.
|
||||
In Proceedings ACM Symposium on Software Reusability, 1995.
|
||||
|
||||
<a name="Lanza05">Lanza05:</a> Lanza, Marinescu; Object-Oriented Metrics in Practice, 2005.
|
||||
|
||||
<a name="McCabe76">McCabe76:</a> McCabe, A Complexity Measure, in Proceedings of the 2nd ICSE (1976).
|
||||
|
||||
@@ -92,6 +92,8 @@ and include them to such reports.
|
||||
* The rule `AbstractClassWithoutAnyMethod` (ruleset `java-design`) will now ignore classes annotated with
|
||||
`com.google.auto.value.AutoValue`.
|
||||
|
||||
* The rule `GodClass` (ruleset `java-design`) has been revamped to use the new metrics framework.
|
||||
|
||||
#### Removed Rules
|
||||
|
||||
* The deprecated rule `UseSingleton` has been removed from the ruleset `java-design`. The rule has been renamed
|
||||
@@ -238,6 +240,6 @@ All existing rules have been updated to reflect these changes. If you have custo
|
||||
* [#567](https://github.com/pmd/pmd/pull/567): \[java] Last API change for metrics (metric options) - [Clément Fournier](https://github.com/oowekyala)
|
||||
* [#570](https://github.com/pmd/pmd/pull/570): \[java] Model lower, upper and intersection types - [Bendegúz Nagy](https://github.com/WinterGrascph)
|
||||
* [#573](https://github.com/pmd/pmd/pull/573): \[java] Data class rule - [Clément Fournier](https://github.com/oowekyala)
|
||||
* [#576](https://github.com/pmd/pmd/pull/576): \[doc][java] Add hint for Guava users in InefficientEmptyStringCheck - [mmoehring](https://github.com/mmoehring)
|
||||
* [#576](https://github.com/pmd/pmd/pull/576): \[doc]\[java] Add hint for Guava users in InefficientEmptyStringCheck - [mmoehring](https://github.com/mmoehring)
|
||||
* [#578](https://github.com/pmd/pmd/pull/578): \[java] Refactored god class rule - [Clément Fournier](https://github.com/oowekyala)
|
||||
* [#581](https://github.com/pmd/pmd/pull/581): \[java] Relax AbstractClassWithoutAnyMethod when class is annotated by @AutoValue - [Niklas Baudy](https://github.com/vanniktech)
|
||||
|
||||
@@ -8,6 +8,7 @@ import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
|
||||
@@ -24,6 +25,25 @@ public final class StringUtil {
|
||||
private StringUtil() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Formats a double to a percentage, keeping {@code numDecimal} decimal places.
|
||||
*
|
||||
* @param val a double value between 0 and 1
|
||||
* @param numDecimals The number of decimal places to keep
|
||||
*
|
||||
* @return A formatted string
|
||||
*
|
||||
* @throws IllegalArgumentException if the double to format is not between 0 and 1
|
||||
*/
|
||||
public static String percentageString(double val, int numDecimals) {
|
||||
if (val < 0 || val > 1) {
|
||||
throw new IllegalArgumentException("Expected a number between 0 and 1");
|
||||
}
|
||||
|
||||
return String.format(Locale.ROOT, "%." + numDecimals + "f%%", 100 * val);
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Return whether the non-null text arg starts with any of the prefix
|
||||
* values.
|
||||
|
||||
+5
-5
@@ -12,19 +12,19 @@ package net.sourceforge.pmd.lang.java.ast;
|
||||
public class JavaParserVisitorReducedAdapter extends JavaParserVisitorAdapter {
|
||||
|
||||
@Override
|
||||
public final Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
|
||||
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
|
||||
return visit((ASTAnyTypeDeclaration) node, data);
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public final Object visit(ASTAnnotationTypeDeclaration node, Object data) {
|
||||
public Object visit(ASTAnnotationTypeDeclaration node, Object data) {
|
||||
return visit((ASTAnyTypeDeclaration) node, data);
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public final Object visit(ASTEnumDeclaration node, Object data) {
|
||||
public Object visit(ASTEnumDeclaration node, Object data) {
|
||||
return visit((ASTAnyTypeDeclaration) node, data);
|
||||
}
|
||||
|
||||
@@ -35,13 +35,13 @@ public class JavaParserVisitorReducedAdapter extends JavaParserVisitorAdapter {
|
||||
|
||||
|
||||
@Override
|
||||
public final Object visit(ASTMethodDeclaration node, Object data) {
|
||||
public Object visit(ASTMethodDeclaration node, Object data) {
|
||||
return visit((ASTMethodOrConstructorDeclaration) node, data);
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public final Object visit(ASTConstructorDeclaration node, Object data) {
|
||||
public Object visit(ASTConstructorDeclaration node, Object data) {
|
||||
return visit((ASTMethodOrConstructorDeclaration) node, data);
|
||||
}
|
||||
|
||||
|
||||
+10
-2
@@ -10,6 +10,7 @@ import net.sourceforge.pmd.lang.java.metrics.impl.LocMetric.LocClassMetric;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.NcssMetric.NcssClassMetric;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.NoamMetric;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.NopaMetric;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.TccMetric;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.WmcMetric;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.WocMetric;
|
||||
import net.sourceforge.pmd.lang.metrics.MetricKey;
|
||||
@@ -60,13 +61,20 @@ public enum JavaClassMetricKey implements MetricKey<ASTAnyTypeDeclaration> {
|
||||
* @see NopaMetric
|
||||
*/
|
||||
NOAM(new NoamMetric()),
|
||||
|
||||
|
||||
/**
|
||||
* Weight of class.
|
||||
*
|
||||
* @see WocMetric
|
||||
*/
|
||||
WOC(new WocMetric());
|
||||
WOC(new WocMetric()),
|
||||
|
||||
/**
|
||||
* Tight Class Cohesion.
|
||||
*
|
||||
* @see TccMetric
|
||||
*/
|
||||
TCC(new TccMetric());
|
||||
|
||||
|
||||
private final JavaClassMetric calculator;
|
||||
|
||||
@@ -4,15 +4,16 @@
|
||||
|
||||
package net.sourceforge.pmd.lang.java.metrics.impl;
|
||||
|
||||
import java.util.List;
|
||||
import org.apache.commons.lang3.mutable.MutableInt;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.JavaQualifiedName;
|
||||
import net.sourceforge.pmd.lang.java.metrics.signature.JavaOperationSigMask;
|
||||
import net.sourceforge.pmd.lang.java.metrics.signature.JavaOperationSignature.Role;
|
||||
import net.sourceforge.pmd.lang.java.metrics.signature.JavaSignature.Visibility;
|
||||
import net.sourceforge.pmd.lang.java.metrics.JavaMetrics;
|
||||
import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.visitors.AtfdBaseVisitor;
|
||||
import net.sourceforge.pmd.lang.metrics.MetricOptions;
|
||||
import net.sourceforge.pmd.lang.metrics.ResultOption;
|
||||
|
||||
/**
|
||||
* Access to Foreign Data. Quantifies the number of foreign fields accessed directly or via accessors.
|
||||
@@ -24,31 +25,25 @@ public final class AtfdMetric {
|
||||
|
||||
public static final class AtfdOperationMetric extends AbstractJavaOperationMetric {
|
||||
|
||||
@Override // TODO:cf
|
||||
public double computeFor(ASTMethodOrConstructorDeclaration node, MetricOptions options) {
|
||||
|
||||
JavaOperationSigMask targetOps = new JavaOperationSigMask();
|
||||
targetOps.restrictVisibilitiesTo(Visibility.PUBLIC);
|
||||
targetOps.restrictRolesTo(Role.GETTER_OR_SETTER);
|
||||
|
||||
List<JavaQualifiedName> callQNames = findAllCalls(node);
|
||||
int foreignCalls = 0;
|
||||
for (JavaQualifiedName name : callQNames) {
|
||||
if (getSignatureMatcher().hasMatchingSig(name, targetOps)) {
|
||||
foreignCalls++;
|
||||
}
|
||||
}
|
||||
|
||||
return foreignCalls / callQNames.size();
|
||||
@Override
|
||||
public boolean supports(ASTMethodOrConstructorDeclaration node) {
|
||||
return node instanceof ASTMethodDeclaration && super.supports(node);
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public double computeFor(ASTMethodOrConstructorDeclaration node, MetricOptions options) {
|
||||
return ((MutableInt) node.jjtAccept(new AtfdBaseVisitor(), new MutableInt(0))).getValue();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
public static final class AtfdClassMetric extends AbstractJavaClassMetric {
|
||||
|
||||
@Override
|
||||
public double computeFor(ASTAnyTypeDeclaration node, MetricOptions options) {
|
||||
// TODO:cf
|
||||
return 0;
|
||||
// TODO maybe consider code outside methods
|
||||
return JavaMetrics.get(JavaOperationMetricKey.ATFD, node, options, ResultOption.SUM);
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.lang.java.metrics.impl;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.metrics.impl.visitors.TccMethodPairVisitor;
|
||||
import net.sourceforge.pmd.lang.metrics.MetricOptions;
|
||||
|
||||
/**
|
||||
* Tight class cohesion.
|
||||
*
|
||||
* @author Clément Fournier
|
||||
* @since 6.0.0
|
||||
*/
|
||||
public class TccMetric extends AbstractJavaClassMetric {
|
||||
|
||||
|
||||
@Override
|
||||
public double computeFor(ASTAnyTypeDeclaration node, MetricOptions options) {
|
||||
@SuppressWarnings("unchecked")
|
||||
Map<String, Set<String>> usagesByMethod = (Map<String, Set<String>>) node.jjtAccept(new TccMethodPairVisitor(), null);
|
||||
|
||||
int numPairs = numMethodsRelatedByAttributeAccess(usagesByMethod);
|
||||
int maxPairs = maxMethodPairs(usagesByMethod.size());
|
||||
|
||||
return numPairs / (double) maxPairs;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Gets the number of pairs of methods that use at least one attribute in common.
|
||||
*
|
||||
* @param usagesByMethod Map of method name to names of local attributes accessed
|
||||
*
|
||||
* @return The number of pairs
|
||||
*/
|
||||
private int numMethodsRelatedByAttributeAccess(Map<String, Set<String>> usagesByMethod) {
|
||||
List<String> methods = new ArrayList<>(usagesByMethod.keySet());
|
||||
int methodCount = methods.size();
|
||||
int pairs = 0;
|
||||
|
||||
if (methodCount > 1) {
|
||||
for (int i = 0; i < methodCount - 1; i++) {
|
||||
for (int j = i + 1; j < methodCount; j++) {
|
||||
String firstMethodName = methods.get(i);
|
||||
String secondMethodName = methods.get(j);
|
||||
|
||||
if (!Collections.disjoint(usagesByMethod.get(firstMethodName),
|
||||
usagesByMethod.get(secondMethodName))) {
|
||||
pairs++;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return pairs;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Calculates the number of possible method pairs of two methods.
|
||||
*
|
||||
* @param methods Number of methods in the class
|
||||
*
|
||||
* @return Number of possible method pairs
|
||||
*/
|
||||
private int maxMethodPairs(int methods) {
|
||||
return methods * (methods - 1) / 2;
|
||||
}
|
||||
|
||||
}
|
||||
+109
@@ -0,0 +1,109 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.lang.java.metrics.impl.visitors;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.commons.lang3.mutable.MutableInt;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
|
||||
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.JavaParserVisitorAdapter;
|
||||
import net.sourceforge.pmd.util.StringUtil;
|
||||
|
||||
/**
|
||||
* Computes Atfd.
|
||||
*
|
||||
* @author Clément Fournier
|
||||
* @since 6.0.0
|
||||
*/
|
||||
public class AtfdBaseVisitor extends JavaParserVisitorAdapter {
|
||||
|
||||
|
||||
@Override
|
||||
public Object visit(ASTPrimaryExpression node, Object data) {
|
||||
if (isForeignAttributeOrMethod(node) && (isAttributeAccess(node)
|
||||
|| isMethodCall(node) && isForeignGetterSetterCall(node))) {
|
||||
|
||||
((MutableInt) data).increment();
|
||||
}
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
|
||||
private boolean isForeignGetterSetterCall(ASTPrimaryExpression node) {
|
||||
|
||||
String methodOrAttributeName = getMethodOrAttributeName(node);
|
||||
|
||||
return methodOrAttributeName != null && StringUtil.startsWithAny(methodOrAttributeName, "get", "is", "set");
|
||||
}
|
||||
|
||||
|
||||
private boolean isMethodCall(ASTPrimaryExpression node) {
|
||||
boolean result = false;
|
||||
List<ASTPrimarySuffix> suffixes = node.findDescendantsOfType(ASTPrimarySuffix.class);
|
||||
if (suffixes.size() == 1) {
|
||||
result = suffixes.get(0).isArguments();
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
private boolean isForeignAttributeOrMethod(ASTPrimaryExpression node) {
|
||||
boolean result;
|
||||
String nameImage = getNameImage(node);
|
||||
|
||||
if (nameImage != null && (!nameImage.contains(".") || nameImage.startsWith("this."))) {
|
||||
result = false;
|
||||
} else if (nameImage == null && node.getFirstDescendantOfType(ASTPrimaryPrefix.class).usesThisModifier()) {
|
||||
result = false;
|
||||
} else if (nameImage == null && node.hasDecendantOfAnyType(ASTLiteral.class, ASTAllocationExpression.class)) {
|
||||
result = false;
|
||||
} else {
|
||||
result = true;
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
private String getNameImage(ASTPrimaryExpression node) {
|
||||
ASTPrimaryPrefix prefix = node.getFirstDescendantOfType(ASTPrimaryPrefix.class);
|
||||
ASTName name = prefix.getFirstDescendantOfType(ASTName.class);
|
||||
|
||||
String image = null;
|
||||
if (name != null) {
|
||||
image = name.getImage();
|
||||
}
|
||||
return image;
|
||||
}
|
||||
|
||||
|
||||
private String getMethodOrAttributeName(ASTPrimaryExpression node) {
|
||||
ASTPrimaryPrefix prefix = node.getFirstDescendantOfType(ASTPrimaryPrefix.class);
|
||||
ASTName name = prefix.getFirstDescendantOfType(ASTName.class);
|
||||
|
||||
String methodOrAttributeName = null;
|
||||
|
||||
if (name != null) {
|
||||
int dotIndex = name.getImage().indexOf(".");
|
||||
if (dotIndex > -1) {
|
||||
methodOrAttributeName = name.getImage().substring(dotIndex + 1);
|
||||
}
|
||||
}
|
||||
|
||||
return methodOrAttributeName;
|
||||
}
|
||||
|
||||
|
||||
private boolean isAttributeAccess(ASTPrimaryExpression node) {
|
||||
return node.findDescendantsOfType(ASTPrimarySuffix.class).isEmpty();
|
||||
}
|
||||
|
||||
}
|
||||
+121
@@ -0,0 +1,121 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.lang.java.metrics.impl.visitors;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.Stack;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
|
||||
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.JavaParserVisitorReducedAdapter;
|
||||
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
|
||||
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
|
||||
import net.sourceforge.pmd.lang.symboltable.Scope;
|
||||
|
||||
/**
|
||||
* Returns the map of method names to the set local attributes accessed when visiting a class.
|
||||
*
|
||||
* @author Clément Fournier
|
||||
* @since 6.0.0
|
||||
*/
|
||||
public class TccMethodPairVisitor extends JavaParserVisitorReducedAdapter {
|
||||
|
||||
/**
|
||||
* Collects for each method of the current class, which local attributes are accessed.
|
||||
*/
|
||||
Stack<Map<String, Set<String>>> methodAttributeAccess = new Stack<>();
|
||||
|
||||
/** The name of the current method. */
|
||||
private String currentMethodName;
|
||||
|
||||
|
||||
@Override
|
||||
public Object visit(ASTAnyTypeDeclaration node, Object data) {
|
||||
methodAttributeAccess.push(new HashMap<String, Set<String>>());
|
||||
super.visit(node, data);
|
||||
|
||||
methodAttributeAccess.peek().remove(null);
|
||||
return methodAttributeAccess.pop();
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public Object visit(ASTMethodDeclaration node, Object data) {
|
||||
|
||||
if (!node.isAbstract()) {
|
||||
currentMethodName = node.getQualifiedName().getOperation();
|
||||
methodAttributeAccess.peek().put(currentMethodName, new HashSet<String>());
|
||||
|
||||
super.visit(node, data);
|
||||
|
||||
currentMethodName = null;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* The primary expression node is used to detect access to attributes and method calls. If the access is not for a
|
||||
* foreign class, then the {@link #methodAttributeAccess} map is updated for the current method.
|
||||
*/
|
||||
@Override
|
||||
public Object visit(ASTPrimaryExpression node, Object data) {
|
||||
if (currentMethodName != null) {
|
||||
Set<String> methodAccess = methodAttributeAccess.peek().get(currentMethodName);
|
||||
String variableName = getVariableName(node);
|
||||
if (isLocalAttributeAccess(variableName, node.getScope())) {
|
||||
methodAccess.add(variableName);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
|
||||
private String getVariableName(ASTPrimaryExpression node) {
|
||||
ASTPrimaryPrefix prefix = node.getFirstDescendantOfType(ASTPrimaryPrefix.class);
|
||||
ASTName name = prefix.getFirstDescendantOfType(ASTName.class);
|
||||
|
||||
String variableName = null;
|
||||
|
||||
if (name != null) {
|
||||
int dotIndex = name.getImage().indexOf(".");
|
||||
if (dotIndex == -1) {
|
||||
variableName = name.getImage();
|
||||
} else {
|
||||
variableName = name.getImage().substring(0, dotIndex);
|
||||
}
|
||||
}
|
||||
|
||||
return variableName;
|
||||
}
|
||||
|
||||
|
||||
private boolean isLocalAttributeAccess(String varName, Scope scope) {
|
||||
Scope currentScope = scope;
|
||||
|
||||
while (currentScope != null) {
|
||||
for (VariableNameDeclaration decl : currentScope.getDeclarations(VariableNameDeclaration.class).keySet()) {
|
||||
if (decl.getImage().equals(varName)) {
|
||||
if (currentScope instanceof ClassScope) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
currentScope = currentScope.getParent(); // WARNING doesn't consider inherited fields or static imports
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
+9
-1
@@ -8,6 +8,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.metrics.JavaMetrics;
|
||||
import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey;
|
||||
import net.sourceforge.pmd.lang.java.rule.AbstractJavaMetricsRule;
|
||||
import net.sourceforge.pmd.util.StringUtil;
|
||||
|
||||
/**
|
||||
* @author Clément Fournier
|
||||
@@ -29,7 +30,14 @@ public class DataClassRule extends AbstractJavaMetricsRule {
|
||||
boolean isDataClass = interfaceRevealsData(node) && classRevealsDataAndLacksComplexity(node);
|
||||
|
||||
if (isDataClass) {
|
||||
addViolation(data, node, new String[] {node.getImage()});
|
||||
double woc = JavaMetrics.get(JavaClassMetricKey.WOC, node);
|
||||
int nopa = (int) JavaMetrics.get(JavaClassMetricKey.NOPA, node);
|
||||
int noam = (int) JavaMetrics.get(JavaClassMetricKey.NOAM, node);
|
||||
int wmc = (int) JavaMetrics.get(JavaClassMetricKey.WMC, node);
|
||||
|
||||
addViolation(data, node, new Object[] {node.getImage(),
|
||||
StringUtil.percentageString(woc, 3),
|
||||
nopa, noam, wmc, });
|
||||
}
|
||||
|
||||
return super.visit(node, data);
|
||||
|
||||
+26
-336
File diff suppressed because it is too large.
Load diff
@@ -1,9 +1,9 @@
|
||||
<?xml version="1.0"?>
|
||||
|
||||
<ruleset name="Design"
|
||||
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
|
||||
xmlns="http://pmd.sourceforge.net/ruleset/3.0.0"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
|
||||
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/3.0.0 http://pmd.sourceforge.net/ruleset_3_0_0.xsd">
|
||||
|
||||
<description>
|
||||
The Design ruleset contains rules that flag suboptimal code implementations. Alternate approaches
|
||||
@@ -1858,7 +1858,8 @@ public class HelloWorldBean {
|
||||
<rule name="GodClass"
|
||||
language="java"
|
||||
since="5.0"
|
||||
message="Possible God class"
|
||||
message="Possible God Class (WMC={0}, ATFD={2}, TCC={1})"
|
||||
metrics="true"
|
||||
class="net.sourceforge.pmd.lang.java.rule.design.GodClassRule"
|
||||
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#godclass">
|
||||
<description>
|
||||
|
||||
@@ -165,7 +165,7 @@ public class Foo {
|
||||
|
||||
<rule name="DataClass"
|
||||
since="6.0"
|
||||
message="The class ''{0}'' is suspected to be a Data Class"
|
||||
message="The class ''{0}'' is suspected to be a Data Class (WOC={1}, NOPA={2}, NOAM={3}, WMC={4})"
|
||||
class="net.sourceforge.pmd.lang.java.metrics.rule.DataClassRule"
|
||||
metrics="true"
|
||||
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_metrics.html#DataClass">
|
||||
|
||||
+8
-21
@@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.java.metrics.impl;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
|
||||
@@ -126,26 +127,12 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule {
|
||||
}
|
||||
|
||||
|
||||
/** Gets a string representation rounded to the nearest half. */
|
||||
private String presentableString(double val) {
|
||||
boolean isInt = Math.floor(val) == val;
|
||||
|
||||
if (!isInt && val >= 0 && val <= 1) { // percentage
|
||||
return roundedString(100 * val) + "%";
|
||||
} else if (!isInt) {
|
||||
return String.valueOf(roundedString(val));
|
||||
} else {
|
||||
/** Gets a nice string representation of a double. */
|
||||
private String niceDoubleString(double val) {
|
||||
if (val == (int) val) {
|
||||
return String.valueOf((int) val);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private String roundedString(double val) {
|
||||
double truncated = Math.floor(100 * val) / 100;
|
||||
if (truncated == Math.floor(truncated)) {
|
||||
return String.valueOf((int) truncated);
|
||||
} else {
|
||||
return String.valueOf(truncated);
|
||||
return String.format(Locale.ROOT, "%." + 4 + "f", val);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -155,11 +142,11 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule {
|
||||
if (classKey != null && reportClasses && classKey.supports(node)) {
|
||||
double classValue = JavaMetrics.get(classKey, node, metricOptions);
|
||||
|
||||
String valueReport = presentableString(classValue);
|
||||
String valueReport = niceDoubleString(classValue);
|
||||
|
||||
if (opKey != null) {
|
||||
double highest = JavaMetrics.get(opKey, node, metricOptions, ResultOption.HIGHEST);
|
||||
valueReport += " highest " + presentableString(highest);
|
||||
valueReport += " highest " + niceDoubleString(highest);
|
||||
}
|
||||
if (classValue >= reportLevel) {
|
||||
addViolation(data, node, new String[] {node.getQualifiedName().toString(), valueReport, });
|
||||
@@ -175,7 +162,7 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule {
|
||||
double methodValue = JavaMetrics.get(opKey, node, metricOptions);
|
||||
if (methodValue >= reportLevel) {
|
||||
addViolation(data, node, new String[] {node.getQualifiedName().toString(),
|
||||
"" + presentableString(methodValue), });
|
||||
"" + niceDoubleString(methodValue), });
|
||||
}
|
||||
}
|
||||
return data;
|
||||
|
||||
@@ -36,6 +36,8 @@ public class AllMetricsTest extends SimpleAggregatorTst {
|
||||
addRule(RULESET, "NopaTest");
|
||||
addRule(RULESET, "NoamTest");
|
||||
addRule(RULESET, "WocTest");
|
||||
addRule(RULESET, "TccTest");
|
||||
addRule(RULESET, "AtfdTest");
|
||||
}
|
||||
|
||||
}
|
||||
@@ -0,0 +1,26 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.lang.java.metrics.impl;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey;
|
||||
import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey;
|
||||
|
||||
/**
|
||||
* @author Clément Fournier
|
||||
* @since 6.0.0
|
||||
*/
|
||||
public class AtfdTestRule extends AbstractMetricTestRule {
|
||||
|
||||
@Override
|
||||
protected JavaClassMetricKey getClassKey() {
|
||||
return JavaClassMetricKey.ATFD;
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
protected JavaOperationMetricKey getOpKey() {
|
||||
return JavaOperationMetricKey.ATFD;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,26 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.lang.java.metrics.impl;
|
||||
|
||||
import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey;
|
||||
import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey;
|
||||
|
||||
/**
|
||||
* @author Clément Fournier
|
||||
* @since 6.0.0
|
||||
*/
|
||||
public class TccTestRule extends AbstractMetricTestRule {
|
||||
|
||||
@Override
|
||||
protected JavaClassMetricKey getClassKey() {
|
||||
return JavaClassMetricKey.TCC;
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
protected JavaOperationMetricKey getOpKey() {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
+188
@@ -0,0 +1,188 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<test-data>
|
||||
|
||||
<code-fragment id="full-example"><![CDATA[
|
||||
public class StatementAndBraceFinder extends JavaParserVisitorAdapter {
|
||||
private static final Logger LOGGER = Logger.getLogger(StatementAndBraceFinder.class.getName());
|
||||
|
||||
private final DataFlowHandler dataFlowHandler;
|
||||
private Structure dataFlow;
|
||||
|
||||
public StatementAndBraceFinder(DataFlowHandler dataFlowHandler) {
|
||||
this.dataFlowHandler = dataFlowHandler;
|
||||
}
|
||||
|
||||
public void buildDataFlowFor(JavaNode node) {
|
||||
if (!(node instanceof ASTMethodDeclaration) && !(node instanceof ASTConstructorDeclaration)) {
|
||||
throw new RuntimeException("Can't build a data flow for anything other than a method or a constructor");
|
||||
}
|
||||
|
||||
this.dataFlow = new Structure(dataFlowHandler);
|
||||
this.dataFlow.createStartNode(node.getBeginLine());
|
||||
this.dataFlow.createNewNode(node);
|
||||
|
||||
node.jjtAccept(this, dataFlow);
|
||||
|
||||
this.dataFlow.createEndNode(node.getEndLine());
|
||||
if (LOGGER.isLoggable(Level.FINE)) {
|
||||
// TODO SRT Remove after development
|
||||
LOGGER.fine("DataFlow is " + this.dataFlow.dump());
|
||||
}
|
||||
Linker linker = new Linker(dataFlowHandler, dataFlow.getBraceStack(), dataFlow.getContinueBreakReturnStack());
|
||||
try {
|
||||
linker.computePaths();
|
||||
} catch (SequenceException | LinkerException e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private void tryToLog(String tag, NodeType type, Node node) {
|
||||
if (LOGGER.isLoggable(Level.FINEST)) {
|
||||
LOGGER.finest("pushOnStack " + tag + " " + type + ": line " + node.getBeginLine()
|
||||
+ ", column " + node.getBeginColumn());
|
||||
}
|
||||
}
|
||||
|
||||
private void tryToLog(NodeType type, Node node) {
|
||||
tryToLog("", type, node);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTStatementExpression node, Object data) {
|
||||
if (!(data instanceof Structure)) {
|
||||
return data;
|
||||
}
|
||||
Structure dataFlow = (Structure) data;
|
||||
if (LOGGER.isLoggable(Level.FINEST)) {
|
||||
LOGGER.finest("createNewNode ASTStatementExpression: line " + node.getBeginLine() + ", column "
|
||||
+ node.getBeginColumn());
|
||||
}
|
||||
dataFlow.createNewNode(node);
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTVariableDeclarator node, Object data) {
|
||||
if (!(data instanceof Structure)) {
|
||||
return data;
|
||||
}
|
||||
Structure dataFlow = (Structure) data;
|
||||
if (LOGGER.isLoggable(Level.FINEST)) {
|
||||
LOGGER.finest("createNewNode ASTVariableDeclarator: line " + node.getBeginLine() + ", column "
|
||||
+ node.getBeginColumn());
|
||||
}
|
||||
dataFlow.createNewNode(node);
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTExpression node, Object data) {
|
||||
if (!(data instanceof Structure)) {
|
||||
return data;
|
||||
}
|
||||
Structure dataFlow = (Structure) data;
|
||||
|
||||
String loggerTag = "parent";
|
||||
|
||||
Node parent = node.jjtGetParent();
|
||||
|
||||
// TODO what about throw stmts?
|
||||
if (parent instanceof ASTIfStatement) {
|
||||
dataFlow.createNewNode(node); // START IF
|
||||
dataFlow.pushOnStack(NodeType.IF_EXPR, dataFlow.getLast());
|
||||
tryToLog(loggerTag, NodeType.IF_EXPR, node);
|
||||
} else if (parent instanceof ASTWhileStatement) {
|
||||
dataFlow.createNewNode(node); // START WHILE
|
||||
dataFlow.pushOnStack(NodeType.WHILE_EXPR, dataFlow.getLast());
|
||||
tryToLog(loggerTag, NodeType.WHILE_EXPR, node);
|
||||
} else if (parent instanceof ASTSwitchStatement) {
|
||||
dataFlow.createNewNode(node); // START SWITCH
|
||||
dataFlow.pushOnStack(NodeType.SWITCH_START, dataFlow.getLast());
|
||||
tryToLog(loggerTag, NodeType.SWITCH_START, node);
|
||||
} else if (parent instanceof ASTForStatement) {
|
||||
dataFlow.createNewNode(node); // FOR EXPR
|
||||
dataFlow.pushOnStack(NodeType.FOR_EXPR, dataFlow.getLast());
|
||||
tryToLog(loggerTag, NodeType.FOR_EXPR, node);
|
||||
} else if (parent instanceof ASTDoStatement) {
|
||||
dataFlow.createNewNode(node); // DO EXPR
|
||||
dataFlow.pushOnStack(NodeType.DO_EXPR, dataFlow.getLast());
|
||||
tryToLog(loggerTag, NodeType.DO_EXPR, node);
|
||||
} else if (parent instanceof ASTAssertStatement) {
|
||||
dataFlow.createNewNode(node);
|
||||
dataFlow.pushOnStack(NodeType.ASSERT_STATEMENT, dataFlow.getLast());
|
||||
tryToLog(loggerTag, NodeType.ASSERT_STATEMENT, node);
|
||||
}
|
||||
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
]]></code-fragment>
|
||||
|
||||
|
||||
<test-code>
|
||||
<description>Full example</description> <!-- TODO issues w/ that-->
|
||||
<expected-problems>7</expected-problems>
|
||||
<expected-messages>
|
||||
<message>'.StatementAndBraceFinder' has value 10 highest 6.</message>
|
||||
<message>'.StatementAndBraceFinder#buildDataFlowFor(JavaNode)' has value 6.</message>
|
||||
<message>'.StatementAndBraceFinder#tryToLog(String, NodeType, Node)' has value 4.</message>
|
||||
<message>'.StatementAndBraceFinder#tryToLog(NodeType, Node)' has value 0.</message>
|
||||
<message>'.StatementAndBraceFinder#visit(ASTStatementExpression, Object)' has value 4.</message>
|
||||
<message>'.StatementAndBraceFinder#visit(ASTVariableDeclarator, Object)' has value 4.</message>
|
||||
<message>'.StatementAndBraceFinder#visit(ASTExpression, Object)' has value 18.</message>
|
||||
</expected-messages>
|
||||
<code-ref id="full-example"/>
|
||||
</test-code>
|
||||
|
||||
<test-code regressionTest="false">
|
||||
<description>TODO: known limitation, should report 1</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<rule-property name="reportClasses">false</rule-property>
|
||||
<expected-messages>
|
||||
<message>'.Foo#bar()' has value 1.</message>
|
||||
</expected-messages>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
Type a;
|
||||
void bar() {
|
||||
a.b.foo();
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Test empty class</description>
|
||||
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-messages>
|
||||
<message>'.Foo' has value 0 highest 0.</message>
|
||||
</expected-messages>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
|
||||
|
||||
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>ATFD doesn't support interfaces or annotations</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public interface Foo {
|
||||
|
||||
public final int h;
|
||||
|
||||
@interface Tag {
|
||||
public static final int num = 0;
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
</test-data>
|
||||
+97
@@ -0,0 +1,97 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<test-data>
|
||||
|
||||
<code-fragment id="full-example"><![CDATA[
|
||||
|
||||
// Data class from ArgoUML
|
||||
/**
|
||||
* A property that can be displayed and edited within a PropertyTable.
|
||||
*
|
||||
* @author Jeremy Jones
|
||||
**/
|
||||
public class Property implements Comparable {
|
||||
|
||||
private String _name;
|
||||
private Class _valueType;
|
||||
private Object _initialValue;
|
||||
private Object _currentValue;
|
||||
private Object[] _availableValues;
|
||||
|
||||
public Property(String name, Class valueType, Object initialValue) {
|
||||
this(name, valueType, initialValue, null);
|
||||
}
|
||||
|
||||
|
||||
public Property(String name, Class valueType, Object initialValue, Object[] values) {
|
||||
_name = name;
|
||||
_valueType = valueType;
|
||||
_initialValue = initialValue;
|
||||
_availableValues = values;
|
||||
_currentValue = _initialValue;
|
||||
}
|
||||
|
||||
public String getName() {
|
||||
return _name;
|
||||
}
|
||||
|
||||
public Class getValueType() {
|
||||
return _valueType;
|
||||
}
|
||||
|
||||
public Object getInitialValue() {
|
||||
return _initialValue;
|
||||
}
|
||||
|
||||
public Object[] getAvailableValues() {
|
||||
return _availableValues;
|
||||
}
|
||||
|
||||
public Object getCurrentValue() {
|
||||
return _currentValue;
|
||||
}
|
||||
|
||||
public void setCurrentValue(Object value) {
|
||||
_currentValue = value;
|
||||
}
|
||||
|
||||
public int compareTo(Object o) {
|
||||
return _name.compareTo(((Property) o)._name);
|
||||
}
|
||||
}
|
||||
]]></code-fragment>
|
||||
|
||||
|
||||
<test-code>
|
||||
<description>Full example</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-messages>
|
||||
<message>'.Property' has value 0.0952.</message>
|
||||
</expected-messages>
|
||||
<code-ref id="full-example"/>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Test empty class</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>TCC doesn't support interfaces or annotations</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public interface Foo {
|
||||
|
||||
public final int h;
|
||||
|
||||
@interface Tag {
|
||||
public static final int num = 0;
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
</test-data>
|
||||
+1
-1
@@ -66,7 +66,7 @@ return _name.compareTo(((Property) o)._name);
|
||||
<description>Full example</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-messages>
|
||||
<message>'.Property' has value 11.11%.</message>
|
||||
<message>'.Property' has value 0.1111.</message>
|
||||
</expected-messages>
|
||||
<code-ref id="full-example"/>
|
||||
</test-code>
|
||||
|
||||
+2
-2
@@ -59,7 +59,7 @@ public class Property implements Comparable { // Woc = 10%, Noam = 6, Nopa = 0,
|
||||
<description>ArgoUML data class</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-messages>
|
||||
<message>The class 'Property' is suspected to be a Data Class</message>
|
||||
<message>The class 'Property' is suspected to be a Data Class (WOC=11.111%, NOPA=0, NOAM=6, WMC=9)</message>
|
||||
</expected-messages>
|
||||
<code-ref id="argoUML-dataclass"/>
|
||||
</test-code>
|
||||
@@ -206,7 +206,7 @@ public class TestDescriptor {
|
||||
<description>PMD data class</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<expected-messages>
|
||||
<message>The class 'TestDescriptor' is suspected to be a Data Class</message>
|
||||
<message>The class 'TestDescriptor' is suspected to be a Data Class (WOC=13.043%, NOPA=0, NOAM=17, WMC=25)</message>
|
||||
</expected-messages>
|
||||
<code-ref id="testDescriptor"/>
|
||||
</test-code>
|
||||
|
||||
@@ -57,4 +57,16 @@
|
||||
metrics="true">
|
||||
</rule>
|
||||
|
||||
<rule name="TccTest"
|
||||
message = "''{0}'' has value {1}."
|
||||
class="net.sourceforge.pmd.lang.java.metrics.impl.TccTestRule"
|
||||
metrics="true">
|
||||
</rule>
|
||||
|
||||
<rule name="AtfdTest"
|
||||
message = "''{0}'' has value {1}."
|
||||
class="net.sourceforge.pmd.lang.java.metrics.impl.AtfdTestRule"
|
||||
metrics="true">
|
||||
</rule>
|
||||
|
||||
</ruleset>
|
||||
Reference in new issue
Block a user