From 341d66b694b26790a3e071dab67a8575c4b97a54 Mon Sep 17 00:00:00 2001 From: oowekyala Date: Tue, 11 Jul 2017 14:41:54 +0200 Subject: [PATCH] Abstracted Metrics to use MetricKeys --- .../pmd/lang/java/oom/AbstractMetric.java | 1 - .../pmd/lang/java/oom/ClassStats.java | 79 ++++++++++--------- .../pmd/lang/java/oom/Metrics.java | 24 +++--- .../pmd/lang/java/oom/OperationStats.java | 6 +- .../pmd/lang/java/oom/PackageStats.java | 13 +-- .../lang/java/oom/ParameterizedMetricKey.java | 2 +- .../pmd/lang/java/oom/api/ClassMetricKey.java | 7 +- .../lang/java/oom/api/OperationMetricKey.java | 5 +- .../oom/rule/CyclomaticComplexityRule.java | 8 +- .../pmd/lang/java/oom/signature/SigMask.java | 2 +- .../oom/metrics/AbstractMetricTestRule.java | 34 +++++--- 11 files changed, 101 insertions(+), 80 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/AbstractMetric.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/AbstractMetric.java index 848228f583..0ee7ca931a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/AbstractMetric.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/AbstractMetric.java @@ -19,7 +19,6 @@ import net.sourceforge.pmd.lang.java.oom.api.Metric; * Base class for metrics. Metric objects encapsulate the computational logic required to compute a metric from a * PackageStats and node. They're stateless. * - * * @author Clément Fournier */ public abstract class AbstractMetric implements Metric { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ClassStats.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ClassStats.java index 61267b1bc2..d8e1edf035 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ClassStats.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ClassStats.java @@ -16,9 +16,9 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.QualifiedName; import net.sourceforge.pmd.lang.java.oom.api.ClassMetric; -import net.sourceforge.pmd.lang.java.oom.api.ClassMetricKey; +import net.sourceforge.pmd.lang.java.oom.api.MetricKey; import net.sourceforge.pmd.lang.java.oom.api.MetricVersion; -import net.sourceforge.pmd.lang.java.oom.api.OperationMetricKey; +import net.sourceforge.pmd.lang.java.oom.api.OperationMetric; import net.sourceforge.pmd.lang.java.oom.api.ResultOption; import net.sourceforge.pmd.lang.java.oom.signature.FieldSigMask; import net.sourceforge.pmd.lang.java.oom.signature.FieldSignature; @@ -159,14 +159,14 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature; * * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed */ - /* default */ double computeWithResultOption(OperationMetricKey key, ASTAnyTypeDeclaration node, + /* default */ double computeWithResultOption(MetricKey key, ASTAnyTypeDeclaration node, boolean force, MetricVersion version, ResultOption option) { List ops = findOperations(node, false); List values = new ArrayList<>(); for (ASTMethodOrConstructorDeclaration op : ops) { - if (key.supports(op)) { + if (key.getCalculator().supports(op)) { double val = this.compute(key, op, op.getQualifiedName().getOperation(), force, version); if (val != Double.NaN) { values.add(val); @@ -188,6 +188,38 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature; } + /** + * Finds the declaration nodes of all methods or constructors that are declared inside a class. + * + * @param node The class in which to look for. + * @param includeNested Include operations found in nested classes? + * + * @return The list of all operations declared inside the specified class. + * + * TODO:cf this one is computed every time + */ + private static List findOperations(ASTAnyTypeDeclaration node, + boolean includeNested) { + + if (includeNested) { + return node.findDescendantsOfType(ASTMethodOrConstructorDeclaration.class); + } + + List outerDecls + = node.jjtGetChild(0).findChildrenOfType(ASTClassOrInterfaceBodyDeclaration.class); + + + List operations = new ArrayList<>(); + + for (ASTClassOrInterfaceBodyDeclaration decl : outerDecls) { + if (decl.jjtGetChild(0) instanceof ASTMethodOrConstructorDeclaration) { + operations.add((ASTMethodOrConstructorDeclaration) decl.jjtGetChild(0)); + } + } + return operations; + } + + /** * Computes the value of a metric for an operation. * @@ -199,8 +231,8 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature; * * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed */ - /* default */ double compute(OperationMetricKey key, ASTMethodOrConstructorDeclaration node, String name, - boolean force, MetricVersion version) { + /* default */ double compute(MetricKey key, ASTMethodOrConstructorDeclaration node, + String name, boolean force, MetricVersion version) { // TODO:cf the operation signature might be built many times, consider storing it in the node Map sigMap = operations.get(OperationSignature.buildFor(node)); @@ -249,7 +281,8 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature; * * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed */ - /* default */ double compute(ClassMetricKey key, ASTAnyTypeDeclaration node, boolean force, MetricVersion version) { + /* default */ double compute(MetricKey key, ASTAnyTypeDeclaration node, boolean force, + MetricVersion version) { ParameterizedMetricKey paramKey = ParameterizedMetricKey.getInstance(key, version); // if memo.get(key) == null then the metric has never been computed. NaN is a valid value. @@ -265,36 +298,4 @@ import net.sourceforge.pmd.lang.java.oom.signature.OperationSignature; return val; } - - /** - * Finds the declaration nodes of all methods or constructors that are declared inside a class. - * - * @param node The class in which to look for. - * @param includeNested Include operations found in nested classes? - * - * @return The list of all operations declared inside the specified class. - * - * TODO:cf this one is computed every time - */ - private static List findOperations(ASTAnyTypeDeclaration node, - boolean includeNested) { - - if (includeNested) { - return node.findDescendantsOfType(ASTMethodOrConstructorDeclaration.class); - } - - List outerDecls - = node.jjtGetChild(0).findChildrenOfType(ASTClassOrInterfaceBodyDeclaration.class); - - - List operations = new ArrayList<>(); - - for (ASTClassOrInterfaceBodyDeclaration decl : outerDecls) { - if (decl.jjtGetChild(0) instanceof ASTMethodOrConstructorDeclaration) { - operations.add((ASTMethodOrConstructorDeclaration) decl.jjtGetChild(0)); - } - } - return operations; - } - } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/Metrics.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/Metrics.java index 6bebb0aec3..47d8e5cac2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/Metrics.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/Metrics.java @@ -7,10 +7,11 @@ package net.sourceforge.pmd.lang.java.oom; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; -import net.sourceforge.pmd.lang.java.oom.api.ClassMetricKey; +import net.sourceforge.pmd.lang.java.oom.api.ClassMetric; import net.sourceforge.pmd.lang.java.oom.api.Metric.Version; +import net.sourceforge.pmd.lang.java.oom.api.MetricKey; import net.sourceforge.pmd.lang.java.oom.api.MetricVersion; -import net.sourceforge.pmd.lang.java.oom.api.OperationMetricKey; +import net.sourceforge.pmd.lang.java.oom.api.OperationMetric; import net.sourceforge.pmd.lang.java.oom.api.ResultOption; @@ -34,13 +35,15 @@ public final class Metrics { * * @return The top level package stats */ - /* default */ static PackageStats getTopLevelPackageStats() { + /* default */ + static PackageStats getTopLevelPackageStats() { return TOP_LEVEL_PACKAGE; } /** Sets whether computations are forced or not. Used for tests. */ - /* default */ static void reset() { + /* default */ + static void reset() { TOP_LEVEL_PACKAGE.reset(); } @@ -53,7 +56,7 @@ public final class Metrics { * * @return The value of the metric, or {@code Double.NaN} if the value couln't be computed */ - public static double get(ClassMetricKey key, ASTAnyTypeDeclaration node) { + public static double get(MetricKey key, ASTAnyTypeDeclaration node) { return get(key, node, Version.STANDARD); } @@ -68,7 +71,7 @@ public final class Metrics { * * @return The value of the metric, or {@code Double.NaN} if the value couln't be computed */ - public static double get(ClassMetricKey key, ASTAnyTypeDeclaration node, MetricVersion version) { + public static double get(MetricKey key, ASTAnyTypeDeclaration node, MetricVersion version) { if (!key.getCalculator().supports(node)) { return Double.NaN; } @@ -87,7 +90,7 @@ public final class Metrics { * * @return The value of the metric, or {@code Double.NaN} if the value couln't be computed */ - public static double get(OperationMetricKey key, ASTMethodOrConstructorDeclaration node) { + public static double get(MetricKey key, ASTMethodOrConstructorDeclaration node) { return get(key, node, Version.STANDARD); } @@ -101,7 +104,7 @@ public final class Metrics { * * @return The value of the metric, or {@code Double.NaN} if the value couln't be computed */ - public static double get(OperationMetricKey key, ASTMethodOrConstructorDeclaration node, MetricVersion version) { + public static double get(MetricKey key, ASTMethodOrConstructorDeclaration node, MetricVersion version) { if (!key.getCalculator().supports(node)) { return Double.NaN; } @@ -123,7 +126,7 @@ public final class Metrics { * @return The value of the metric, or {@code Double.NaN} if the value couln't be computed or {@code option} is * {@code null} */ - public static double get(OperationMetricKey key, ASTAnyTypeDeclaration node, ResultOption option) { + public static double get(MetricKey key, ASTAnyTypeDeclaration node, ResultOption option) { return get(key, node, Version.STANDARD, option); } @@ -140,7 +143,8 @@ public final class Metrics { * @return The value of the metric, or {@code Double.NaN} if the value couln't be computed or {@code option} is * {@code null} */ - public static double get(OperationMetricKey key, ASTAnyTypeDeclaration node, MetricVersion version, ResultOption option) { + public static double get(MetricKey key, ASTAnyTypeDeclaration node, MetricVersion version, + ResultOption option) { MetricVersion safeVersion = (version == null) ? Version.STANDARD : version; return option == null ? Double.NaN diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/OperationStats.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/OperationStats.java index 30e7b83931..fabdfa9ca0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/OperationStats.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/OperationStats.java @@ -8,9 +8,9 @@ import java.util.HashMap; import java.util.Map; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; +import net.sourceforge.pmd.lang.java.oom.api.MetricKey; import net.sourceforge.pmd.lang.java.oom.api.MetricVersion; import net.sourceforge.pmd.lang.java.oom.api.OperationMetric; -import net.sourceforge.pmd.lang.java.oom.api.OperationMetricKey; /** @@ -43,8 +43,8 @@ import net.sourceforge.pmd.lang.java.oom.api.OperationMetricKey; * * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed */ - /* default */ double compute(OperationMetricKey key, ASTMethodOrConstructorDeclaration node, boolean force, - MetricVersion version) { + /* default */ double compute(MetricKey key, ASTMethodOrConstructorDeclaration node, + boolean force, MetricVersion version) { ParameterizedMetricKey paramKey = ParameterizedMetricKey.getInstance(key, version); Double prev = memo.get(paramKey); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/PackageStats.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/PackageStats.java index 9cccd5418a..ea2ea58196 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/PackageStats.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/PackageStats.java @@ -10,9 +10,10 @@ import java.util.Map; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.QualifiedName; -import net.sourceforge.pmd.lang.java.oom.api.ClassMetricKey; +import net.sourceforge.pmd.lang.java.oom.api.ClassMetric; +import net.sourceforge.pmd.lang.java.oom.api.MetricKey; import net.sourceforge.pmd.lang.java.oom.api.MetricVersion; -import net.sourceforge.pmd.lang.java.oom.api.OperationMetricKey; +import net.sourceforge.pmd.lang.java.oom.api.OperationMetric; import net.sourceforge.pmd.lang.java.oom.api.ResultOption; import net.sourceforge.pmd.lang.java.oom.signature.FieldSigMask; import net.sourceforge.pmd.lang.java.oom.signature.OperationSigMask; @@ -158,7 +159,7 @@ public final class PackageStats { * * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed */ - /* default */ double compute(ClassMetricKey key, ASTAnyTypeDeclaration node, boolean force, + /* default */ double compute(MetricKey key, ASTAnyTypeDeclaration node, boolean force, MetricVersion version) { ClassStats container = getClassStats(node.getQualifiedName(), false); @@ -177,8 +178,8 @@ public final class PackageStats { * * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed */ - /* default */ double compute(OperationMetricKey key, ASTMethodOrConstructorDeclaration node, boolean force, - MetricVersion version) { + /* default */ double compute(MetricKey key, ASTMethodOrConstructorDeclaration node, + boolean force, MetricVersion version) { QualifiedName qname = node.getQualifiedName(); ClassStats container = getClassStats(qname, false); @@ -198,7 +199,7 @@ public final class PackageStats { * * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed */ - /* default */ double computeWithResultOption(OperationMetricKey key, ASTAnyTypeDeclaration node, + /* default */ double computeWithResultOption(MetricKey key, ASTAnyTypeDeclaration node, boolean force, MetricVersion version, ResultOption option) { ClassStats container = getClassStats(node.getQualifiedName(), false); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ParameterizedMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ParameterizedMetricKey.java index 8032ca221f..5de7b50d77 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ParameterizedMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/ParameterizedMetricKey.java @@ -16,7 +16,7 @@ import net.sourceforge.pmd.lang.java.oom.api.MetricVersion; * * @author Clément Fournier */ -public final class ParameterizedMetricKey { +/* default */ final class ParameterizedMetricKey { private static final Map POOL = new HashMap<>(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/ClassMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/ClassMetricKey.java index 6794627a6f..ebcc96a695 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/ClassMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/ClassMetricKey.java @@ -12,17 +12,22 @@ import net.sourceforge.pmd.lang.java.oom.metrics.NcssMetric; import net.sourceforge.pmd.lang.java.oom.metrics.WmcMetric; /** - * Keys identifying class metrics. + * Keys identifying standard class metrics. */ public enum ClassMetricKey implements MetricKey { + /** Access to Foreign Data. */ ATFD(new AtfdMetric()), + /** Weighed Method Count. */ WMC(new WmcMetric()), + /** Cyclomatic complexity. */ CYCLO(new CycloMetric()), + /** Non Commenting Source Statements. */ NCSS(new NcssMetric()), + /** Lines of Code. */ LOC(new LocMetric()); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/OperationMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/OperationMetricKey.java index c9d0a3e307..3c200e5525 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/OperationMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/api/OperationMetricKey.java @@ -11,16 +11,19 @@ import net.sourceforge.pmd.lang.java.oom.metrics.LocMetric; import net.sourceforge.pmd.lang.java.oom.metrics.NcssMetric; /** - * Keys identifying operation metrics. + * Keys identifying standard operation metrics. */ public enum OperationMetricKey implements MetricKey { /** Access to Foreign Data. */ ATFD(new AtfdMetric()), + /** Cyclomatic complexity. */ CYCLO(new CycloMetric()), + /** Non Commenting Source Statements. */ NCSS(new NcssMetric()), + /** Lines of Code. */ LOC(new LocMetric()); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/rule/CyclomaticComplexityRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/rule/CyclomaticComplexityRule.java index f118f34b90..95f323475e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/rule/CyclomaticComplexityRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/rule/CyclomaticComplexityRule.java @@ -28,13 +28,13 @@ import net.sourceforge.pmd.lang.rule.properties.IntegerProperty; */ public class CyclomaticComplexityRule extends AbstractJavaMetricsRule { - public static final IntegerProperty REPORT_LEVEL_DESCRIPTOR = new IntegerProperty( + private static final IntegerProperty REPORT_LEVEL_DESCRIPTOR = new IntegerProperty( "reportLevel", "Cyclomatic Complexity reporting threshold", 1, 30, 10, 1.0f); - public static final BooleanProperty REPORT_CLASSES_DESCRIPTOR = new BooleanProperty( + private static final BooleanProperty REPORT_CLASSES_DESCRIPTOR = new BooleanProperty( "reportClasses", "Add class average violations to the report", true, 2.0f); - public static final BooleanProperty REPORT_METHODS_DESCRIPTOR = new BooleanProperty( + private static final BooleanProperty REPORT_METHODS_DESCRIPTOR = new BooleanProperty( "reportMethods", "Add method average violations to the report", true, 3.0f); @@ -42,7 +42,7 @@ public class CyclomaticComplexityRule extends AbstractJavaMetricsRule { private static final MetricVersion[] CYCLO_VERSIONS = {Metric.Version.STANDARD, CycloMetric.Version.IGNORE_BOOLEAN_PATHS}; - public static final EnumeratedProperty CYCLO_VERSION_DESCRIPTOR = new EnumeratedProperty<>( + private static final EnumeratedProperty CYCLO_VERSION_DESCRIPTOR = new EnumeratedProperty<>( "cycloVersion", "Choose a variant of Cyclo or the standard", VERSION_LABELS, CYCLO_VERSIONS, 0, 3.0f); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/signature/SigMask.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/signature/SigMask.java index 4d783add35..9e3fb86595 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/signature/SigMask.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/oom/signature/SigMask.java @@ -18,7 +18,7 @@ import java.util.Set; public abstract class SigMask { /** Visibility mask. */ - protected Set visMask = new HashSet<>(); + private Set visMask = new HashSet<>(); public SigMask() { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/oom/metrics/AbstractMetricTestRule.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/oom/metrics/AbstractMetricTestRule.java index c19f057830..983e3e1da9 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/oom/metrics/AbstractMetricTestRule.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/oom/metrics/AbstractMetricTestRule.java @@ -39,6 +39,7 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule { private ClassMetricKey classKey; private OperationMetricKey opKey; + public AbstractMetricTestRule() { classKey = getClassKey(); opKey = getOpKey(); @@ -48,6 +49,23 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule { definePropertyDescriptor(reportLevelDescriptor); } + + /** + * Returns the class metric key to test, or null if we shouldn't test classes. + * + * @return The class metric key to test. + */ + protected abstract ClassMetricKey getClassKey(); + + + /** + * Returns the class metric key to test, or null if we shouldn't test classes. + * + * @return The class metric key to test. + */ + protected abstract OperationMetricKey getOpKey(); + + public Object visit(ASTCompilationUnit node, Object data) { reportClasses = getProperty(reportClassesDescriptor); reportMethods = getProperty(reportMethodsDescriptor); @@ -55,6 +73,7 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule { return super.visit(node, data); } + /** * Sets the default for reportClasses descriptor. * @@ -64,6 +83,7 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule { return true; } + /** * Sets the default for reportMethods descriptor. * @@ -73,6 +93,7 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule { return true; } + /** * Sets the version to test * @@ -82,19 +103,6 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule { return Version.STANDARD; } - /** - * Returns the class metric key to test, or null if we shouldn't test classes. - * - * @return The class metric key to test. - */ - protected abstract ClassMetricKey getClassKey(); - - /** - * Returns the class metric key to test, or null if we shouldn't test classes. - * - * @return The class metric key to test. - */ - protected abstract OperationMetricKey getOpKey(); /** * Default report level, which is 0.