From e810918205ff8e99a5cd692d6a99b899f5337eef Mon Sep 17 00:00:00 2001 From: kazachka Date: Wed, 1 Feb 2017 21:26:30 +0300 Subject: [PATCH] Issue #56: fix NPathComplexityCheck --- config/suppressions.xml | 1 + .../checks/metrics/NPathComplexityCheck.java | 367 +++++++++++++++--- .../CyclomaticComplexityCheckTest.java | 20 +- .../metrics/NPathComplexityCheckTest.java | 102 ++++- .../checks/metrics/InputComplexity.java | 34 +- .../metrics/InputComplexityOverflow.java | 84 ++++ .../checks/metrics/InputNPathComplexity.java | 178 +++++++++ src/xdocs/config_metrics.xml | 2 +- 8 files changed, 712 insertions(+), 76 deletions(-) create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputNPathComplexity.java diff --git a/config/suppressions.xml b/config/suppressions.xml index 35f715b09b9..ad8ed39646c 100644 --- a/config/suppressions.xml +++ b/config/suppressions.xml @@ -29,6 +29,7 @@ + diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheck.java index 57f8c600af0..b202ecd9588 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheck.java @@ -50,20 +50,36 @@ public final class NPathComplexityCheck extends AbstractCheck { private static final int DEFAULT_MAX = 200; /** The initial current value. */ - private static final BigInteger INITIAL_VALUE = BigInteger.ONE; + private static final BigInteger INITIAL_VALUE = BigInteger.ZERO; - /** Stack of values - all but the current value. */ - private final Deque valueStack = new ArrayDeque<>(); + /** + * Stack of NP values for ranges. + */ + private final Deque rangeValues = new ArrayDeque<>(); + + /** Stack of NP values for expressions. */ + private final Deque expressionValues = new ArrayDeque<>(); + + /** Stack of belongs to range values for question operator. */ + private final Deque isAfterValues = new ArrayDeque<>(); + + /** + * Range of the last processed expression. Used for checking that ternary operation + * which is a part of expression won't be processed for second time. + */ + private final TokenEnd processingTokenEnd = new TokenEnd(); - /** The current value. */ - private BigInteger currentValue = INITIAL_VALUE; + /** NP value for current range. */ + private BigInteger currentRangeValue = INITIAL_VALUE; /** Threshold to report error for. */ private int max = DEFAULT_MAX; + /** True, when branch is visited, but not leaved. */ + private boolean branchVisited; + /** * Set the maximum threshold allowed. - * * @param max the maximum threshold */ public void setMax(int max) { @@ -88,10 +104,12 @@ public int[] getAcceptableTokens() { TokenTypes.LITERAL_IF, TokenTypes.LITERAL_ELSE, TokenTypes.LITERAL_SWITCH, - TokenTypes.LITERAL_CASE, + TokenTypes.CASE_GROUP, TokenTypes.LITERAL_TRY, TokenTypes.LITERAL_CATCH, TokenTypes.QUESTION, + TokenTypes.LITERAL_RETURN, + TokenTypes.LITERAL_DEFAULT, }; } @@ -100,28 +118,54 @@ public int[] getRequiredTokens() { return getAcceptableTokens(); } + @Override + public void beginTree(DetailAST rootAST) { + rangeValues.clear(); + expressionValues.clear(); + isAfterValues.clear(); + processingTokenEnd.reset(); + currentRangeValue = INITIAL_VALUE; + branchVisited = false; + } + @Override public void visitToken(DetailAST ast) { switch (ast.getType()) { + case TokenTypes.LITERAL_IF: + case TokenTypes.LITERAL_SWITCH: case TokenTypes.LITERAL_WHILE: case TokenTypes.LITERAL_DO: case TokenTypes.LITERAL_FOR: - case TokenTypes.LITERAL_IF: + visitConditional(ast, 1); + break; case TokenTypes.QUESTION: - case TokenTypes.LITERAL_TRY: - case TokenTypes.LITERAL_SWITCH: - visitMultiplyingConditional(); + visitUnitaryOperator(ast, 2); + break; + case TokenTypes.LITERAL_RETURN: + visitUnitaryOperator(ast, 0); + break; + case TokenTypes.CASE_GROUP: + final int caseNumber = countCaseTokens(ast); + branchVisited = true; + pushValue(caseNumber); break; case TokenTypes.LITERAL_ELSE: + branchVisited = true; + if (currentRangeValue.equals(BigInteger.ZERO)) { + currentRangeValue = BigInteger.ONE; + } + pushValue(0); + break; + case TokenTypes.LITERAL_TRY: case TokenTypes.LITERAL_CATCH: - case TokenTypes.LITERAL_CASE: - visitAddingConditional(); + case TokenTypes.LITERAL_DEFAULT: + pushValue(1); break; case TokenTypes.CTOR_DEF: case TokenTypes.METHOD_DEF: case TokenTypes.INSTANCE_INIT: case TokenTypes.STATIC_INIT: - visitMethodDef(); + pushValue(0); break; default: break; @@ -135,16 +179,27 @@ public void leaveToken(DetailAST ast) { case TokenTypes.LITERAL_DO: case TokenTypes.LITERAL_FOR: case TokenTypes.LITERAL_IF: - case TokenTypes.QUESTION: - case TokenTypes.LITERAL_TRY: case TokenTypes.LITERAL_SWITCH: + leaveConditional(); + break; + case TokenTypes.LITERAL_TRY: leaveMultiplyingConditional(); break; - case TokenTypes.LITERAL_ELSE: + case TokenTypes.LITERAL_RETURN: + case TokenTypes.QUESTION: + leaveUnitaryOperator(); + break; case TokenTypes.LITERAL_CATCH: - case TokenTypes.LITERAL_CASE: leaveAddingConditional(); break; + case TokenTypes.LITERAL_DEFAULT: + leaveBranch(); + break; + case TokenTypes.LITERAL_ELSE: + case TokenTypes.CASE_GROUP: + leaveBranch(); + branchVisited = false; + break; case TokenTypes.CTOR_DEF: case TokenTypes.METHOD_DEF: case TokenTypes.INSTANCE_INIT: @@ -156,56 +211,272 @@ public void leaveToken(DetailAST ast) { } } - /** Visits else, catch or case. */ - private void visitAddingConditional() { - pushValue(); + /** + * Visits if, while, do-while, for and switch tokens - all of them have expression in + * parentheses which is used for calculation. + * @param ast visited token. + * @param basicBranchingFactor default number of branches added. + */ + private void visitConditional(DetailAST ast, int basicBranchingFactor) { + int expressionValue = basicBranchingFactor; + DetailAST bracketed; + for (bracketed = ast.findFirstToken(TokenTypes.LPAREN).getNextSibling(); + bracketed.getType() != TokenTypes.RPAREN; + bracketed = bracketed.getNextSibling()) { + expressionValue += countConditionalOperators(bracketed); + } + processingTokenEnd.setToken(bracketed); + pushValue(expressionValue); + } + + /** + * Visits ternary operator (?:) and return tokens. They differ from those processed by + * visitConditional method in that their expression isn't bracketed. + * @param ast visited token. + * @param basicBranchingFactor number of branches inherently added by this token. + */ + private void visitUnitaryOperator(DetailAST ast, int basicBranchingFactor) { + final boolean isAfter = processingTokenEnd.isAfter(ast); + isAfterValues.push(isAfter); + if (!isAfter) { + processingTokenEnd.setToken(getLastToken(ast)); + final int expressionValue = basicBranchingFactor + countConditionalOperators(ast); + pushValue(expressionValue); + } + } + + /** + * Leaves ternary operator (?:) and return tokens. + */ + private void leaveUnitaryOperator() { + if (!isAfterValues.pop()) { + final Values valuePair = popValue(); + BigInteger basicRangeValue = valuePair.getRangeValue(); + BigInteger expressionValue = valuePair.getExpressionValue(); + if (expressionValue.equals(BigInteger.ZERO)) { + expressionValue = BigInteger.ONE; + } + if (basicRangeValue.equals(BigInteger.ZERO)) { + basicRangeValue = BigInteger.ONE; + } + currentRangeValue = currentRangeValue.add(expressionValue).multiply(basicRangeValue); + } + } + + /** Leaves while, do, for, if, ternary (?::), return or switch. */ + private void leaveConditional() { + final Values valuePair = popValue(); + final BigInteger expressionValue = valuePair.getExpressionValue(); + BigInteger basicRangeValue = valuePair.getRangeValue(); + if (currentRangeValue.equals(BigInteger.ZERO)) { + currentRangeValue = BigInteger.ONE; + } + if (basicRangeValue.equals(BigInteger.ZERO)) { + basicRangeValue = BigInteger.ONE; + } + currentRangeValue = currentRangeValue.add(expressionValue).multiply(basicRangeValue); + } + + /** Leaves else, default or case group tokens. */ + private void leaveBranch() { + final Values valuePair = popValue(); + final BigInteger basicRangeValue = valuePair.getRangeValue(); + final BigInteger expressionValue = valuePair.getExpressionValue(); + if (branchVisited && currentRangeValue.equals(BigInteger.ZERO)) { + currentRangeValue = BigInteger.ONE; + } + currentRangeValue = currentRangeValue.subtract(BigInteger.ONE) + .add(basicRangeValue) + .add(expressionValue); } - /** Leaves else, catch or case. */ + /** + * Process the end of a method definition. + * @param ast the token type representing the method definition + */ + private void leaveMethodDef(DetailAST ast) { + final BigInteger bigIntegerMax = BigInteger.valueOf(max); + if (currentRangeValue.compareTo(bigIntegerMax) > 0) { + log(ast, MSG_KEY, currentRangeValue, bigIntegerMax); + } + popValue(); + currentRangeValue = INITIAL_VALUE; + } + + /** Leaves catch. */ private void leaveAddingConditional() { - currentValue = currentValue.subtract(BigInteger.ONE).add(popValue()); + currentRangeValue = currentRangeValue.add(popValue().getRangeValue().add(BigInteger.ONE)); } - /** Visits while, do, for, if, try, ? (in ?::) or switch. */ - private void visitMultiplyingConditional() { - pushValue(); + /** + * Pushes the current range value on the range value stack. Pushes this token expression value + * on the expression value stack. + * @param expressionValue value of expression calculated for current token. + */ + private void pushValue(Integer expressionValue) { + rangeValues.push(currentRangeValue); + expressionValues.push(expressionValue); + currentRangeValue = INITIAL_VALUE; } - /** Leaves while, do, for, if, try, ? (in ?::) or switch. */ + /** + * Pops values from both stack of expression values and stack of range values. + * @return pair of head values from both of the stacks. + */ + private Values popValue() { + final int expressionValue = expressionValues.pop(); + return new Values(rangeValues.pop(), BigInteger.valueOf(expressionValue)); + } + + /** Leaves try. */ private void leaveMultiplyingConditional() { - currentValue = currentValue.add(BigInteger.ONE).multiply(popValue()); + currentRangeValue = currentRangeValue.add(BigInteger.ONE) + .multiply(popValue().getRangeValue().add(BigInteger.ONE)); } - /** Push the current value on the stack. */ - private void pushValue() { - valueStack.push(currentValue); - currentValue = INITIAL_VALUE; + /** + * Calculates number of conditional operators, including inline ternary operatior, for a token. + * @param ast inspected token. + * @return number of conditional operators. + * @see + * Java Language Specification, §15.23 + * @see + * Java Language Specification, §15.24 + * @see + * Java Language Specification, §15.25 + */ + private static int countConditionalOperators(DetailAST ast) { + int number = 0; + for (DetailAST child = ast.getFirstChild(); child != null; + child = child.getNextSibling()) { + final int type = child.getType(); + if (type == TokenTypes.LOR || type == TokenTypes.LAND) { + number++; + } + else if (type == TokenTypes.QUESTION) { + number += 2; + } + number += countConditionalOperators(child); + } + return number; } /** - * 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 + * Finds a leaf, which is the most distant from the root. + * @param ast the root of tree. + * @return the leaf. */ - private BigInteger popValue() { - currentValue = valueStack.pop(); - return currentValue; + private static DetailAST getLastToken(DetailAST ast) { + final DetailAST lastChild = ast.getLastChild(); + final DetailAST result; + if (lastChild.getFirstChild() == null) { + result = lastChild; + } + else { + result = getLastToken(lastChild); + } + return result; } - /** Process the start of the method definition. */ - private void visitMethodDef() { - pushValue(); + /** + * Counts number of case tokens subject to a case group token. + * @param ast case group token. + * @return number of case tokens. + */ + private static int countCaseTokens(DetailAST ast) { + int counter = 0; + for (DetailAST iterator = ast.getFirstChild(); iterator != null; + iterator = iterator.getNextSibling()) { + if (iterator.getType() == TokenTypes.LITERAL_CASE) { + counter++; + } + } + return counter; } /** - * Process the end of a method definition. - * - * @param ast the token representing the method definition + * Coordinates of token end. Used to prevent inline ternary + * operator from being processed twice. */ - private void leaveMethodDef(DetailAST ast) { - final BigInteger bigIntegerMax = BigInteger.valueOf(max); - if (currentValue.compareTo(bigIntegerMax) > 0) { - log(ast, MSG_KEY, currentValue, bigIntegerMax); + private static class TokenEnd { + /** End line of token. */ + private int endLineNo; + + /** End column of token. */ + private int endColumnNo; + + /** + * Sets end coordinates from given token. + * @param endToken token. + */ + public void setToken(DetailAST endToken) { + if (!isAfter(endToken)) { + endLineNo = endToken.getLineNo(); + endColumnNo = endToken.getColumnNo(); + } } - popValue(); + + /** Sets end token coordinates to the start of the file. */ + public void reset() { + endLineNo = 0; + endColumnNo = 0; + } + + /** + * Checks if saved coordinates located after given token. + * @param ast given token. + * @return true, if saved coordinates located after given token. + */ + public boolean isAfter(DetailAST ast) { + final int lineNo = ast.getLineNo(); + final int columnNo = ast.getColumnNo(); + boolean isAfter = true; + if (lineNo > endLineNo + || lineNo == endLineNo + && columnNo > endColumnNo) { + isAfter = false; + } + return isAfter; + } + } + + /** + * Class that store range value and expression value. + */ + private static class Values { + + /** NP value for range. */ + private final BigInteger rangeValue; + + /** NP value for expression. */ + private final BigInteger expressionValue; + + /** + * Constructor that assigns all of class fields. + * @param valueOfRange NP value for range + * @param valueOfExpression NP value for expression + */ + Values(BigInteger valueOfRange, BigInteger valueOfExpression) { + rangeValue = valueOfRange; + expressionValue = valueOfExpression; + } + + /** + * Returns NP value for range. + * @return NP value for range + */ + public BigInteger getRangeValue() { + return rangeValue; + } + + /** + * Returns NP value for expression. + * @return NP value for expression + */ + public BigInteger getExpressionValue() { + return expressionValue; + } + } + } 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 c0c97822347..2d1131b81a6 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 @@ -76,16 +76,16 @@ public void test() throws Exception { checkConfig.addAttribute("max", "0"); final String[] expected = { - "4:5: " + getCheckMessage(MSG_KEY, 2, 0), - "7:17: " + getCheckMessage(MSG_KEY, 2, 0), - "17:5: " + getCheckMessage(MSG_KEY, 6, 0), - "27:5: " + getCheckMessage(MSG_KEY, 3, 0), - "34:5: " + getCheckMessage(MSG_KEY, 5, 0), - "48:5: " + getCheckMessage(MSG_KEY, 3, 0), - "58:5: " + getCheckMessage(MSG_KEY, 3, 0), - "67:5: " + getCheckMessage(MSG_KEY, 3, 0), - "76:5: " + getCheckMessage(MSG_KEY, 1, 0), - "79:13: " + getCheckMessage(MSG_KEY, 2, 0), + "5:5: " + getCheckMessage(MSG_KEY, 2, 0), + "10:17: " + getCheckMessage(MSG_KEY, 2, 0), + "22:5: " + getCheckMessage(MSG_KEY, 6, 0), + "35:5: " + getCheckMessage(MSG_KEY, 3, 0), + "45:5: " + getCheckMessage(MSG_KEY, 5, 0), + "63:5: " + getCheckMessage(MSG_KEY, 3, 0), + "76:5: " + getCheckMessage(MSG_KEY, 3, 0), + "88:5: " + getCheckMessage(MSG_KEY, 3, 0), + "100:5: " + getCheckMessage(MSG_KEY, 1, 0), + "104:13: " + getCheckMessage(MSG_KEY, 2, 0), }; verify(checkConfig, getPath("InputComplexity.java"), expected); diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheckTest.java index a7c8d3f8765..9171576531f 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/NPathComplexityCheckTest.java @@ -49,21 +49,46 @@ public void testCalculation() throws Exception { checkConfig.addAttribute("max", "0"); final String[] expected = { - "4:5: " + getCheckMessage(MSG_KEY, 2, 0), - "7:17: " + getCheckMessage(MSG_KEY, 2, 0), - "17:5: " + getCheckMessage(MSG_KEY, 5, 0), - "27:5: " + getCheckMessage(MSG_KEY, 3, 0), - "34:5: " + getCheckMessage(MSG_KEY, 7, 0), - "48:5: " + getCheckMessage(MSG_KEY, 3, 0), - "58:5: " + getCheckMessage(MSG_KEY, 3, 0), - "67:5: " + getCheckMessage(MSG_KEY, 3, 0), - "76:5: " + getCheckMessage(MSG_KEY, 1, 0), - "79:13: " + getCheckMessage(MSG_KEY, 2, 0), + "5:5: " + getCheckMessage(MSG_KEY, 2, 0), + "10:17: " + getCheckMessage(MSG_KEY, 2, 0), + "22:5: " + getCheckMessage(MSG_KEY, 10, 0), + "35:5: " + getCheckMessage(MSG_KEY, 3, 0), + "45:5: " + getCheckMessage(MSG_KEY, 7, 0), + "63:5: " + getCheckMessage(MSG_KEY, 3, 0), + "76:5: " + getCheckMessage(MSG_KEY, 3, 0), + "88:5: " + getCheckMessage(MSG_KEY, 3, 0), + "104:13: " + getCheckMessage(MSG_KEY, 2, 0), }; verify(checkConfig, getPath("InputComplexity.java"), expected); } + @Test + public void testCalculation2() throws Exception { + final DefaultConfiguration checkConfig = + createCheckConfig(NPathComplexityCheck.class); + + checkConfig.addAttribute("max", "0"); + final String[] expected = { + "5:5: " + getCheckMessage(MSG_KEY, 5, 0), + "11:5: " + getCheckMessage(MSG_KEY, 5, 0), + "18:5: " + getCheckMessage(MSG_KEY, 4, 0), + "33:5: " + getCheckMessage(MSG_KEY, 4, 0), + "49:5: " + getCheckMessage(MSG_KEY, 6, 0), + "65:5: " + getCheckMessage(MSG_KEY, 15, 0), + "90:5: " + getCheckMessage(MSG_KEY, 11, 0), + "100:5: " + getCheckMessage(MSG_KEY, 8, 0), + "113:5: " + getCheckMessage(MSG_KEY, 120, 0), + "125:5: " + getCheckMessage(MSG_KEY, 6, 0), + "135:5: " + getCheckMessage(MSG_KEY, 21, 0), + "148:5: " + getCheckMessage(MSG_KEY, 35, 0), + "156:5: " + getCheckMessage(MSG_KEY, 25, 0), + "171:5: " + getCheckMessage(MSG_KEY, 2, 0), + }; + + verify(checkConfig, getPath("InputNPathComplexity.java"), expected); + } + @Test public void testIntegerOverflow() throws Exception { final DefaultConfiguration checkConfig = @@ -74,7 +99,7 @@ public void testIntegerOverflow() throws Exception { final long largerThanMaxInt = 3_486_784_401L; final String[] expected = { - "9:5: " + getCheckMessage(MSG_KEY, largerThanMaxInt, 0), + "13:5: " + getCheckMessage(MSG_KEY, largerThanMaxInt, 0), }; verify(checkConfig, getPath("InputComplexityOverflow.java"), expected); @@ -105,10 +130,12 @@ public void testGetAcceptableTokens() { TokenTypes.LITERAL_IF, TokenTypes.LITERAL_ELSE, TokenTypes.LITERAL_SWITCH, - TokenTypes.LITERAL_CASE, + TokenTypes.CASE_GROUP, TokenTypes.LITERAL_TRY, TokenTypes.LITERAL_CATCH, TokenTypes.QUESTION, + TokenTypes.LITERAL_RETURN, + TokenTypes.LITERAL_DEFAULT, }; Assert.assertNotNull(actual); Assert.assertArrayEquals(expected, actual); @@ -129,10 +156,12 @@ public void testGetRequiredTokens() { TokenTypes.LITERAL_IF, TokenTypes.LITERAL_ELSE, TokenTypes.LITERAL_SWITCH, - TokenTypes.LITERAL_CASE, + TokenTypes.CASE_GROUP, TokenTypes.LITERAL_TRY, TokenTypes.LITERAL_CATCH, TokenTypes.QUESTION, + TokenTypes.LITERAL_RETURN, + TokenTypes.LITERAL_DEFAULT, }; Assert.assertNotNull(actual); Assert.assertArrayEquals(expected, actual); @@ -146,4 +175,51 @@ public void testDefaultHooks() { npathComplexityCheckObj.visitToken(ast); npathComplexityCheckObj.leaveToken(ast); } + + @Test + public void testVisitTokenBeforeExpressionRange() { + // Create first ast + final DetailAST astIf = mockAST(TokenTypes.LITERAL_IF, "if", "mockfile", 2, 2); + final DetailAST astIfLeftParen = mockAST(TokenTypes.LPAREN, "(", "mockfile", 3, 3); + astIf.addChild(astIfLeftParen); + final DetailAST astIfTrue = + mockAST(TokenTypes.LITERAL_TRUE, "true", "mockfile", 3, 3); + astIf.addChild(astIfTrue); + final DetailAST astIfRightParen = mockAST(TokenTypes.RPAREN, ")", "mockfile", 4, 4); + astIf.addChild(astIfRightParen); + // Create ternary ast + final DetailAST astTernary = mockAST(TokenTypes.QUESTION, "?", "mockfile", 1, 1); + final DetailAST astTernaryTrue = + mockAST(TokenTypes.LITERAL_TRUE, "true", "mockfile", 1, 2); + astTernary.addChild(astTernaryTrue); + + final NPathComplexityCheck mock = new NPathComplexityCheck(); + // visiting first ast, set expressionSpatialRange to [2,2 - 4,4] + mock.visitToken(astIf); + //visiting ternary, it lies before expressionSpatialRange + mock.visitToken(astTernary); + } + + /** + * Creates MOCK lexical token and returns AST node for this token. + * @param tokenType type of token + * @param tokenText text of token + * @param tokenFileName file name of token + * @param tokenRow token position in a file (row) + * @param tokenColumn token position in a file (column) + * @return AST node for the token + */ + private static DetailAST mockAST(final int tokenType, final String tokenText, + final String tokenFileName, final int tokenRow, final int tokenColumn) { + final CommonHiddenStreamToken tokenImportSemi = new CommonHiddenStreamToken(); + tokenImportSemi.setType(tokenType); + tokenImportSemi.setText(tokenText); + tokenImportSemi.setLine(tokenRow); + tokenImportSemi.setColumn(tokenColumn); + tokenImportSemi.setFilename(tokenFileName); + final DetailAST astSemi = new DetailAST(); + astSemi.initialize(tokenImportSemi); + return astSemi; + } + } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexity.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexity.java index 56f095e076c..0d44a79d260 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexity.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexity.java @@ -1,10 +1,14 @@ package com.puppycrawl.tools.checkstyle.checks.metrics; public class InputComplexity { + // NP = 2 public void foo() { + //NP(while-statement) = (while-range=1) + (expr=0) + 1 = 2 while (true) { Runnable runnable = new Runnable() { + // NP = 2 public void run() { + // NP(while-statement) = (while-range=1) + (expr=0) + 1 = 2 while (true) { } } @@ -14,69 +18,91 @@ public void run() { } } + // NP = 10 public void bar() { + // NP = (if-range=3*3) + (expr=0) + 1 = 10 if (System.currentTimeMillis() == 0) { + //NP = (if-range=1) + 1 + (expr=1) = 3 if (System.currentTimeMillis() == 0 && System.currentTimeMillis() == 0) { } - + //NP = (if-range=1) + 1 + (expr=1) = 3 if (System.currentTimeMillis() == 0 || System.currentTimeMillis() == 0) { } } } + // NP = 3 public void simpleElseIf() { + // NP = (if-range=1) + (else-range=2) + 0 = 3 if (System.currentTimeMillis() == 0) { + // NP(else-range) = (if-range=1) + (else-range=1) + (expr=0) = 2 } else if (System.currentTimeMillis() == 0) { } else { } } + // NP = 7 public void stupidElseIf() { + // NP = (if-range=1) + (else-range=3*2) + (expr=0) = 7 if (System.currentTimeMillis() == 0) { } else { + // NP = (if-range=1) + (else-range=2) + (expr=0) = 3 if (System.currentTimeMillis() == 0) { } else { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (System.currentTimeMillis() == 0) { } } - + // NP = (if-range=1) + 1 + (expr=0) = 2 if (System.currentTimeMillis() == 0) { } } } + // NP = 3 public InputComplexity() { int i = 1; + // NP = (if-range=1) + (else-range=2) + 0 = 3 if (System.currentTimeMillis() == 0) { + // NP(else-range) = (if-range=1) + (else-range=1) + (expr=0) = 2 } else if (System.currentTimeMillis() == 0) { } else { } } - // STATIC_INIT + // STATIC_INIT + // NP = 3 static { int i = 1; + // NP = (if-range=1) + (else-range=2) + 0 = 3 if (System.currentTimeMillis() == 0) { + // NP(else-range) = (if-range=1) + (else-range=1) + (expr=0) = 2 } else if (System.currentTimeMillis() == 0) { } else { } } - // INSTANCE_INIT + // INSTANCE_INIT + // NP = 3 { int i = 1; + // NP = (if-range=1) + (else-range=2) + 0 = 3 if (System.currentTimeMillis() == 0) { + // NP(else-range) = (if-range=1) + (else-range=1) + (expr=0) = 2 } else if (System.currentTimeMillis() == 0) { } else { } } /** Inner */ + // NP = 0 public InputComplexity(int aParam) { Runnable runnable = new Runnable() { + // NP = 2 public void run() { + // NP(while-statement) = (while-range=1) + (expr=0) + 1 = 2 while (true) { } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexityOverflow.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexityOverflow.java index f669d973b1e..6a49ca2a338 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexityOverflow.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputComplexityOverflow.java @@ -6,15 +6,27 @@ */ public class InputComplexityOverflow { + /* NP = (if-range[1]=9) * (if-range[2]=9) * (if-range[3]=9) * (if-range[4]=9) + * (if-range[5]=9) * (if-range[6]=9) * (if-range[7]=9) * (if-range[8]=9) + * (if-range[9]=9) * (if-range[10]=9) = 3486784401 + */ public void provokeNpathIntegerOverflow() { + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -24,13 +36,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -40,13 +60,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -56,13 +84,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -72,13 +108,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -88,13 +132,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -104,13 +156,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -120,13 +180,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -136,13 +204,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } @@ -152,13 +228,21 @@ public void provokeNpathIntegerOverflow() } } } + // NP = (if-range=8) + 1 + (expr=0) = 9 if (true) { + // NP = (if-range=7) + 1 + (expr=0) = 8 if (true) { + // NP = (if-range=6) + 1 + (expr=0) = 7 if (true) { + // NP = (if-range=5) + 1 + (expr=0) = 6 if (true) { + // NP = (if-range=4) + 1 + (expr=0) = 5 if (true) { + // NP = (if-range=3) + 1 + (expr=0) = 4 if (true) { + // NP = (if-range=2) + 1 + (expr=0) = 3 if (true) { + // NP = (if-range=1) + 1 + (expr=0) = 2 if (true) { } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputNPathComplexity.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputNPathComplexity.java new file mode 100644 index 00000000000..c4ce47035bc --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/metrics/InputNPathComplexity.java @@ -0,0 +1,178 @@ +package com.puppycrawl.tools.checkstyle.checks.metrics; +// Advise: for lack of ambiguity try to make all factors prime numbers +public class InputNPathComplexity { + //NP = 5 + void testIfWithExpression() { + // NP = (if-range=1) + 1 + (expr=3) = 5 + if (true && true || (true || true)) { } + } + + //NP = 5 + void testIfElseWithExpression() { + // NP = (if-range=1) + (else-range=1) + (expr=3) = 5 + if (true && true || (true || true)) { } + else { } + } + + //NP = 4 + int testSimpleSwitch() { + int a = 0; + // NP = (case-range[1]=1) + (case-range[2]=1) + (case-range[3]=1) + // + (default-range=1) + (expr=0) = 4 + switch(a) { + case 1: + break; + case 2: + case 3: + break; + } + return a; + } + + //NP = 4 + void testSimpleSwitchWithDefault() { + int a = 0; + // NP = (case-range[1]=1) + (case-range[2]=1) + (case-range[3]=1) + // + (default-range=1) + (expr=0) = 4 + switch(a) { + case 1: + break; + case 2: + case 3: + break; + default: + break; + } + } + + //NP = 6 + void testSwitchWithExpression() { + int a = 0; + // NP = (case-range[1]=1) + (case-range[2]=1) + (case-range[3]=1) + // + (default-range=1) + (expr=2) = 6 + switch(true ? a : a) { + case 1: + break; + case 2: + case 3: + break; + default: + break; + } + } + + //NP = 15 + void testComplexSwitch() { + int a = 0; + // NP = (case-range[1]=2) + (case-range[2]=5*2) + (case-range[3]=2) + // + (default-range=1) + (expr=0) = 15 + switch(a) { + case 1: + // NP(case-range) = (if-range=1) + 1 + (expr=0) = 2 + if (true) { } + break; + case 2: + // NP(case-range) = (if-range=1) + (else-range=1) + (expr=3) = 5 + if (true && true || (true || true)) { } + else { } + // NP(case-range) = (if-range=1) + 1 + (expr=0) = 2 + if (true) { } + case 3: + // NP(case-range) = (if-range=1) + 1 + (expr=0) = 2 + if (true) { } + break; + default: + break; + } + } + + // NP = 11 + void testComplexIfElse() { + // NP = (if-range=1) + (else-range=9) + (expr=1) = 11 + if (true && true) { } + // NP(else-range) = (if-range=1) + (else-range=6) + (expr=2) = 9 + else if (true || true || true) { } + // NP(else-range) = (if-range=1) + 1 + (expr=4) = 6 + else if (true && true && true || true || true) { } + } + + // NP = 8 + boolean testComplexReturn() { + // NP = (if-range=3) + (else-range=4) + (expr=1) = 8 + if (true && true) { + // NP(if-range) = 3 + return true && true || (true && true); + } else { + // NP(else-range) = (expr(1)=0) + (expr(2)=1) + (expr(3)=1) + 2 = 4 + return true ? true && true : true || true; + } + } + + // NP = (for-statement[1]=2) * (for-statement[2]=3) + // * (for-statement[3]=4) * (for-statement[4]=5) = 120 + void testForCyclesComplex() { + // NP(for-statement) = (for-range=1) + (expr(1)=0) + (expr(2)=0) + (expr(3)=0) + 1 = 2 + for (int i = 0; i < 10; i++); + // NP(for-statement) = (for-range=1) + (expr(1)=0) + (expr(2)=1) + (expr(3)=0) + 1 = 3 + for (int i = 0; i < 10 && true; i++); + // NP(for-statement) = (for-range=1) + (expr(1)=2) + (expr(2)=0) + (expr(3)=0) + 1 = 4 + for (int i = true ? 0 : 0; i < 10; i++); + // NP(for-statement) = (for-range=1) + (expr(1)=0) + (expr(2)=1+2) + (expr(3)=0) + 1 = 5 + for (int i = 0; true ? i < 10 : true || true; i++); + } + + // NP = (while-statement[1]=2) * (while-statement[2]=3) = 6 + boolean testWhileCyclesComplex() { + int a = 0; + // NP(while-statement) = (while-range=1) + (expr=0) + 1 = 2 + while (a != 0) { } + // NP(while-statement) = (while-range=1) + (expr=1) + 1 = 3 + while (a != 0 && a == 0) { return a == 0 || a == 0; } + return true; + } + + // NP = (do-statement[1]=6) * (do-statement[2]=3) = 21 + void testDoWhileCyclesComplex() { + int a = 0; + // NP(do-statement) = (do-range=1) + (expr=1) + 1 = 3 + do { } while (a < 10 && true); + // NP(do-statement) = + // (do-range=3) + ((expr(1)=0) + (expr(2)=0) + (expr(3)=1) + 2) + 1 = 7 + do { + // NP(do-range) = (do-range=1) + (expr=1) + 1 = 3 + do { } while (a < 10 || true); + } while (true ? a > 10 : (a < 10 || true)); + } + + // NP = (question-statement[1]=5) * (question-statement[2]=7) = 35 + void testComplexTernaryOperator() { + // NP(question-statement) = (expr(1)=0) + (expr(2)=2) + (expr(3)=1+2) + 2 = 7 + boolean a = true ? (true ? true : true) : (false ? (true || false) : true); + // NP(question-statement) = (expr(1)=0) + (expr(2)=2) + (expr(3)=1) + 2 = 5; + boolean b = true ? (true ? true : true) : true || true; + } + + // NP = (if-expression[1]=5) * (if-expression[2]=5) = 25 + void testSimpleTernaryBadFormatting() { + // NP(if-expression) = (if-range=2) + 1 + (expr=2) = 5 + if( + true ? true : true + ) { boolean a = true ? true + : true; + } + // NP(if-expression) = (if-range=2) + 1 + (expr=2) = 5 + if( + true ? true : true) { boolean b = true ? true : true; + } + } + + //Calculation for try-catch is wrong now + //See issue #3814 https://github.com/checkstyle/checkstyle/issues/3814 + void testTryCatch() { + try { + } + catch (Exception e) { + } + } + +} diff --git a/src/xdocs/config_metrics.xml b/src/xdocs/config_metrics.xml index 2f71e45bb29..8a8af6e9bba 100644 --- a/src/xdocs/config_metrics.xml +++ b/src/xdocs/config_metrics.xml @@ -1018,7 +1018,7 @@ class SwitchExample { The NPATH metric computes the number of possible execution paths through a function(method). It takes into account the nesting of conditional statements and multi-part boolean expressions - (e.g., A && B, C || D, etc.). + (A && B, C || D, E ? F :G and their combinations).
The NPATH metric was designed base on Cyclomatic complexity to avoid problem of Cyclomatic complexity metric like nesting level within a function(method).