From ed79281fff9b62deeed7b359c7fceedb6adb46c3 Mon Sep 17 00:00:00 2001 From: rnveach Date: Tue, 17 Nov 2015 07:26:56 -0500 Subject: [PATCH] Issue #2451: removed excess hierarchy from CyclomaticComplexityCheck --- pom.xml | 1 + .../metrics/CyclomaticComplexityCheck.java | 118 ++++++++++++++++-- .../CyclomaticComplexityCheckTest.java | 11 ++ 3 files changed, 118 insertions(+), 12 deletions(-) diff --git a/pom.xml b/pom.xml index dd469321f..40a0b04ae 100644 --- a/pom.xml +++ b/pom.xml @@ -1719,6 +1719,7 @@ com/puppycrawl/tools/checkstyle/checks/coding/AbstractIllegalCheck.class com/puppycrawl/tools/checkstyle/checks/coding/AbstractIllegalMethodCheck.class com/puppycrawl/tools/checkstyle/checks/coding/AbstractNestedDepthCheck.class + com/puppycrawl/tools/checkstyle/checks/metrics/AbstractComplexityCheck.class com/puppycrawl/tools/checkstyle/checks/naming/AbstractTypeParameterNameCheck.class diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheck.java index c63e9b117..dfee755db 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheck.java @@ -20,7 +20,10 @@ package com.puppycrawl.tools.checkstyle.checks.metrics; import java.math.BigInteger; +import java.util.ArrayDeque; +import java.util.Deque; +import com.puppycrawl.tools.checkstyle.api.Check; import com.puppycrawl.tools.checkstyle.api.DetailAST; import com.puppycrawl.tools.checkstyle.api.TokenTypes; @@ -43,7 +46,7 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; * @author Andrei Selkin */ public class CyclomaticComplexityCheck - extends AbstractComplexityCheck { + extends Check { /** * A key is pointing to the warning message text in "messages.properties" @@ -51,16 +54,23 @@ public class CyclomaticComplexityCheck */ public static final String MSG_KEY = "cyclomaticComplexity"; + /** The initial current value. */ + private static final BigInteger INITIAL_VALUE = BigInteger.ONE; + /** Default allowed complexity. */ - private static final int DEFAULT_VALUE = 10; + private static final int DEFAULT_COMPLEXITY_VALUE = 10; /** Whether to treat the whole switch block as a single decision point.*/ private boolean switchBlockAsSingleDecisionPoint; - /** Create an instance. */ - public CyclomaticComplexityCheck() { - super(DEFAULT_VALUE); - } + /** Stack of values - all but the current value. */ + private final Deque valueStack = new ArrayDeque<>(); + + /** The current value. */ + private BigInteger currentValue = INITIAL_VALUE; + + /** Threshold to report error for. */ + private int max = DEFAULT_COMPLEXITY_VALUE; /** * Sets whether to treat the whole switch block as a single decision point. @@ -71,6 +81,15 @@ public class CyclomaticComplexityCheck this.switchBlockAsSingleDecisionPoint = switchBlockAsSingleDecisionPoint; } + /** + * Set the maximum threshold allowed. + * + * @param max the maximum threshold + */ + public final void setMax(int max) { + this.max = max; + } + @Override public int[] getDefaultTokens() { return new int[] { @@ -112,6 +131,49 @@ public class CyclomaticComplexityCheck } @Override + public final int[] getRequiredTokens() { + return new int[] { + TokenTypes.CTOR_DEF, + TokenTypes.METHOD_DEF, + TokenTypes.INSTANCE_INIT, + TokenTypes.STATIC_INIT, + }; + } + + @Override + public void visitToken(DetailAST ast) { + switch (ast.getType()) { + case TokenTypes.CTOR_DEF: + case TokenTypes.METHOD_DEF: + case TokenTypes.INSTANCE_INIT: + case TokenTypes.STATIC_INIT: + visitMethodDef(); + break; + default: + visitTokenHook(ast); + } + } + + @Override + public void leaveToken(DetailAST ast) { + switch (ast.getType()) { + case TokenTypes.CTOR_DEF: + case TokenTypes.METHOD_DEF: + case TokenTypes.INSTANCE_INIT: + case TokenTypes.STATIC_INIT: + leaveMethodDef(ast); + break; + default: + break; + } + } + + /** + * Hook called when visiting a token. Will not be called the method + * definition tokens. + * + * @param ast the token being visited + */ protected final void visitTokenHook(DetailAST ast) { if (switchBlockAsSingleDecisionPoint) { if (ast.getType() != TokenTypes.LITERAL_CASE) { @@ -123,13 +185,45 @@ public class CyclomaticComplexityCheck } } - @Override - protected final String getMessageID() { - return MSG_KEY; + /** + * Process the end of a method definition. + * + * @param ast the token representing the method definition + */ + private void leaveMethodDef(DetailAST ast) { + final BigInteger bigIntegerMax = BigInteger.valueOf(max); + if (currentValue.compareTo(bigIntegerMax) > 0) { + log(ast, MSG_KEY, currentValue, bigIntegerMax); + } + popValue(); } - @Override - protected void leaveTokenHook(DetailAST ast) { - // no code + /** + * Increments the current value by a specified amount. + * + * @param by the amount to increment by + */ + protected final void incrementCurrentValue(BigInteger by) { + currentValue = currentValue.add(by); + } + + /** Push the current value on the stack. */ + protected final void pushValue() { + valueStack.push(currentValue); + currentValue = INITIAL_VALUE; + } + + /** + * Pops a value off the stack and makes it the current value. + * @return pop a value off the stack and make it the current value + */ + protected final BigInteger popValue() { + currentValue = valueStack.pop(); + return currentValue; + } + + /** Process the start of the method definition. */ + private void visitMethodDef() { + pushValue(); } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheckTest.java index 05daac583..7bbbe7c33 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/CyclomaticComplexityCheckTest.java @@ -24,6 +24,7 @@ import static com.puppycrawl.tools.checkstyle.checks.metrics.CyclomaticComplexit import java.io.File; import java.io.IOException; +import org.apache.commons.lang3.ArrayUtils; import org.junit.Assert; import org.junit.Test; @@ -113,4 +114,14 @@ public class CyclomaticComplexityCheckTest }; Assert.assertArrayEquals(expected, actual); } + + @Test + public void testHighMax() throws Exception { + final DefaultConfiguration checkConfig = + createCheckConfig(CyclomaticComplexityCheck.class); + checkConfig.addAttribute("max", "100"); + final String[] expected = ArrayUtils.EMPTY_STRING_ARRAY; + + verify(checkConfig, getPath("InputComplexitySwitchBlocks.java"), expected); + } }