Skip to content

Commit

Permalink
Issue #2874: Use CyclomaticComplexity.switchBlockAsSingleDecisionPoin…
Browse files Browse the repository at this point in the history 
…t in checkstyle_checks.xml (#3401)
  • Loading branch information
@MEZk @romani
MEZk authored and romani committed Sep 8, 2016
1 parent e650684 commit 963f949
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 47 deletions.
4 changes: 3 additions & 1 deletion config/checkstyle_checks.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,9 @@
<!-- Default classes are also listed-->
<property name="excludedClasses" value="boolean, byte, char, double, float, int, long, short, void, Boolean, Byte, Character, Double, Float, Integer, Long, Short, Void, Object, Class, String, StringBuffer, StringBuilder, ArrayIndexOutOfBoundsException, Exception, RuntimeException, IllegalArgumentException, IllegalStateException, IndexOutOfBoundsException, NullPointerException, Throwable, SecurityException, UnsupportedOperationException, List, ArrayList, Deque, Queue, LinkedList, Set, HashSet, SortedSet, TreeSet, Map, HashMap, SortedMap, TreeMap, DetailsAST, CheckstyleException, UnsupportedEncodingException, BuildException, ConversionException, FileNotFoundException, TestException, Log, Sets, Multimap, TokenStreamRecognitionException, RecognitionException, TokenStreamException, IOException"/>
</module>
<module name="CyclomaticComplexity"/>
<module name="CyclomaticComplexity">
<property name="switchBlockAsSingleDecisionPoint" value="true"/>
</module>
<module name="JavaNCSS"/>
<module name="NPathComplexity"/>

Expand Down
14 changes: 6 additions & 8 deletions config/suppressions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@
<!-- getDetails() method - huge Switch, it has to be monolithic -->
<suppress checks="ExecutableStatementCount" files="RightCurlyCheck\.java" lines="313"/>
<suppress checks="JavaNCSS" files="RightCurlyCheck\.java" lines="313"/>
<suppress checks="CyclomaticComplexity" files="RightCurlyCheck\.java" lines="313"/>

<!-- we need that set of converters -->
<suppress checks="ClassDataAbstractionCoupling" files="AutomaticBean\.java"/>
Expand All @@ -140,9 +139,12 @@
<!-- Should be fixed after moving https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/ReturnCountExtendedCheck.java into the main repo -->
<suppress checks="ReturnCount" files="(ClassResolver|ConfigurationLoader|IndentationCheckTest)\.java"/>

<!-- Just big switches. Cannot be split to several methods. Till https://github.com/checkstyle/checkstyle/issues/2029 -->
<suppress checks="CyclomaticComplexity" files="(AbstractDeclarationCollector|RequireThisCheck|SuppressWarningsHolder|LeftCurlyCheck|FallThroughCheck|FinalLocalVariableCheck|ModifiedControlVariableCheck)\.java"/>
<suppress checks="CyclomaticComplexity" files="(ParameterAssignmentCheck|VariableDeclarationUsageDistanceCheck|BooleanExpressionComplexityCheck|NPathComplexityCheck|CheckUtils)\.java"/>
<!--Method getClassFrameWhereViolationIsFound already have too much abstract methods that fully
explain a logic, additional abstraction will not make logic/algorithm more readable.-->
<suppress checks="CyclomaticComplexity" files="RequireThisCheck\.java" lines="416"/>
<!--The only optimization which can be there is to move CASE-block expressions to separate method,
but that will not increase readability.-->
<suppress checks="CyclomaticComplexity" files="FinalLocalVariableCheck\.java" lines="176, 328"/>

<!-- Suppressions from PMD configuration-->
<!-- validateCli is not reasonable to split as encapsulation of logic will be damaged -->
Expand All @@ -157,10 +159,6 @@
<!-- SWITCH was transformed into IF-ELSE -->
<suppress checks="CyclomaticComplexity" files="ImportOrderCheck\.java" lines="357"/>

<!-- Just big SWITCH block which contains IF blocks in 'visitToken'.
If we split the block to several methods it will demage readibility. -->
<suppress checks="CyclomaticComplexity" files="DeclarationOrderCheck\.java" lines="189"/>

<!-- LocalizedMessage class is immutable, we need that amount of arguments. -->
<suppress checks="ParameterNumber"
files="LocalizedMessage.java"
Expand Down
40 changes: 32 additions & 8 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/SuppressWarningsHolder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,8 @@ public static boolean isSuppressed(AuditEvent event) {
final int column = event.getColumn();
boolean suppressed = false;
for (Entry entry : entries) {
final boolean afterStart =
entry.getFirstLine() < line
|| entry.getFirstLine() == line
&& (column == 0 || entry.getFirstColumn() <= column);
final boolean beforeEnd =
entry.getLastLine() > line
|| entry.getLastLine() == line && entry
.getLastColumn() >= column;
final boolean afterStart = isSuppressedAfterEventStart(line, column, entry);
final boolean beforeEnd = isSuppressedBeforeEventEnd(line, column, entry);
final boolean nameMatches =
ALL_WARNING_MATCHING_ID.equals(entry.getCheckName())
|| entry.getCheckName().equalsIgnoreCase(checkAlias);
Expand All @@ -180,6 +174,36 @@ public static boolean isSuppressed(AuditEvent event) {
return suppressed;
}

/**
* Checks whether suppression entry position is after the audit event occurrence position
* in the source file.
* @param line the line number in the source file where the event occurred.
* @param column the column number in the source file where the event occurred.
* @param entry suppression entry.
* @return true if suppression entry position is after the audit event occurrence position
* in the source file.
*/
private static boolean isSuppressedAfterEventStart(int line, int column, Entry entry) {
return entry.getFirstLine() < line
|| entry.getFirstLine() == line
&& (column == 0 || entry.getFirstColumn() <= column);
}

/**
* Checks whether suppression entry position is before the audit event occurrence position
* in the source file.
* @param line the line number in the source file where the event occurred.
* @param column the column number in the source file where the event occurred.
* @param entry suppression entry.
* @return true if suppression entry position is before the audit event occurrence position
* in the source file.
*/
private static boolean isSuppressedBeforeEventEnd(int line, int column, Entry entry) {
return entry.getLastLine() > line
|| entry.getLastLine() == line && entry
.getLastColumn() >= column;
}

@Override
public int[] getDefaultTokens() {
return getAcceptableTokens();
Expand Down
73 changes: 43 additions & 30 deletions .../com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,36 +399,7 @@ private static Entry<DetailAST, Integer> calculateDistanceInSingleScope(
if (currentAst.getFirstChild() != null) {

if (isChild(currentAst, variableIdentAst)) {

switch (currentAst.getType()) {
case TokenTypes.VARIABLE_DEF:
dist++;
break;
case TokenTypes.SLIST:
dist = 0;
break;
case TokenTypes.LITERAL_FOR:
case TokenTypes.LITERAL_WHILE:
case TokenTypes.LITERAL_DO:
case TokenTypes.LITERAL_IF:
case TokenTypes.LITERAL_SWITCH:
if (isVariableInOperatorExpr(currentAst, variableIdentAst)) {
dist++;
}
else {
// variable usage is in inner scope
// reset counters, because we can't determine distance
dist = 0;
}
break;
default:
if (currentAst.branchContains(TokenTypes.SLIST)) {
dist = 0;
}
else {
dist++;
}
}
dist = getDistToVariableUsageInChildNode(currentAst, variableIdentAst, dist);
variableUsageAst = currentAst;
firstUsageFound = true;
}
Expand All @@ -447,6 +418,48 @@ else if (currentAst.getType() != TokenTypes.VARIABLE_DEF) {
return new SimpleEntry<>(variableUsageAst, dist);
}

/**
* Returns the distance to variable usage for in the child node.
* @param childNode child node.
* @param varIdent variable variable identifier.
* @param currentDistToVarUsage current distance to the variable usage.
* @return the distance to variable usage for in the child node.
*/
private static int getDistToVariableUsageInChildNode(DetailAST childNode, DetailAST varIdent,
int currentDistToVarUsage) {
int resultDist = currentDistToVarUsage;
switch (childNode.getType()) {
case TokenTypes.VARIABLE_DEF:
resultDist++;
break;
case TokenTypes.SLIST:
resultDist = 0;
break;
case TokenTypes.LITERAL_FOR:
case TokenTypes.LITERAL_WHILE:
case TokenTypes.LITERAL_DO:
case TokenTypes.LITERAL_IF:
case TokenTypes.LITERAL_SWITCH:
if (isVariableInOperatorExpr(childNode, varIdent)) {
resultDist++;
}
else {
// variable usage is in inner scope
// reset counters, because we can't determine distance
resultDist = 0;
}
break;
default:
if (childNode.branchContains(TokenTypes.SLIST)) {
resultDist = 0;
}
else {
resultDist++;
}
}
return resultDist;
}

/**
* Calculates distance between declaration of variable and its first usage
* in multiple scopes.
Expand Down

0 comments on commit 963f949

Please sign in to comment.