Skip to content

Commit

Permalink
Issue checkstyle#15445: Update NPathComplexity Check to Account for w…
Browse files Browse the repository at this point in the history
…hen expression as a Possible Execution Path
  • Loading branch information
mahfouz72 authored and rnveach committed Aug 22, 2024
1 parent a1e2527 commit 99e0edd
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
* <td>NP(for-range) + NP(expr1)+ NP(expr2) + NP(expr3) + 1</td></tr>
* <tr><td>switch ([expr]) { case : [case-range] default: [default-range] }</td>
* <td>S(i=1:i=n)NP(case-range[i]) + NP(default-range) + NP(expr)</td></tr>
* <tr><td>when[expr]</td><td>NP(expr) + 1</td></tr>
* <tr><td>[expr1] ? [expr2] : [expr3]</td><td>NP(expr1) + NP(expr2) + NP(expr3) + 2</td></tr>
* <tr><td>goto label</td><td>1</td></tr><tr><td>break</td><td>1</td></tr>
* <tr><td>Expressions</td>
Expand Down Expand Up @@ -220,6 +221,7 @@ public int[] getRequiredTokens() {
TokenTypes.LITERAL_DEFAULT,
TokenTypes.COMPACT_CTOR_DEF,
TokenTypes.SWITCH_RULE,
TokenTypes.LITERAL_WHEN,
};
}

Expand Down Expand Up @@ -249,6 +251,9 @@ public void visitToken(DetailAST ast) {
case TokenTypes.LITERAL_RETURN:
visitUnitaryOperator(ast, 0);
break;
case TokenTypes.LITERAL_WHEN:
visitWhenExpression(ast, 1);
break;
case TokenTypes.CASE_GROUP:
final int caseNumber = countCaseTokens(ast);
branchVisited = true;
Expand Down Expand Up @@ -291,6 +296,7 @@ public void leaveToken(DetailAST ast) {
case TokenTypes.LITERAL_FOR:
case TokenTypes.LITERAL_IF:
case TokenTypes.LITERAL_SWITCH:
case TokenTypes.LITERAL_WHEN:
leaveConditional();
break;
case TokenTypes.LITERAL_TRY:
Expand Down Expand Up @@ -343,6 +349,19 @@ private void visitConditional(DetailAST ast, int basicBranchingFactor) {
pushValue(expressionValue);
}

/**
* Visits when expression token. There is no guarantee that when expression will be
* bracketed, so we don't use visitConditional method.
*
* @param ast visited token.
* @param basicBranchingFactor default number of branches added.
*/
private void visitWhenExpression(DetailAST ast, int basicBranchingFactor) {
final int expressionValue = basicBranchingFactor + countConditionalOperators(ast);
processingTokenEnd.setToken(getLastToken(ast));
pushValue(expressionValue);
}

/**
* Visits ternary operator (?:) and return tokens. They differ from those processed by
* visitConditional method in that their expression isn't bracketed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
&lt;td&gt;NP(for-range) + NP(expr1)+ NP(expr2) + NP(expr3) + 1&lt;/td&gt;&lt;/tr&gt;
&lt;tr&gt;&lt;td&gt;switch ([expr]) { case : [case-range] default: [default-range] }&lt;/td&gt;
&lt;td&gt;S(i=1:i=n)NP(case-range[i]) + NP(default-range) + NP(expr)&lt;/td&gt;&lt;/tr&gt;
&lt;tr&gt;&lt;td&gt;when[expr]&lt;/td&gt;&lt;td&gt;NP(expr) + 1&lt;/td&gt;&lt;/tr&gt;
&lt;tr&gt;&lt;td&gt;[expr1] ? [expr2] : [expr3]&lt;/td&gt;&lt;td&gt;NP(expr1) + NP(expr2) + NP(expr3) + 2&lt;/td&gt;&lt;/tr&gt;
&lt;tr&gt;&lt;td&gt;goto label&lt;/td&gt;&lt;td&gt;1&lt;/td&gt;&lt;/tr&gt;&lt;tr&gt;&lt;td&gt;break&lt;/td&gt;&lt;td&gt;1&lt;/td&gt;&lt;/tr&gt;
&lt;tr&gt;&lt;td&gt;Expressions&lt;/td&gt;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,8 @@ public void testPatternMatchingForSwitch() throws Exception {
"23:5: " + getCheckMessage(MSG_KEY, 3, 1),
"32:5: " + getCheckMessage(MSG_KEY, 3, 1),
"41:5: " + getCheckMessage(MSG_KEY, 3, 1),
"50:5: " + getCheckMessage(MSG_KEY, 3, 1),
"59:5: " + getCheckMessage(MSG_KEY, 3, 1),
"50:5: " + getCheckMessage(MSG_KEY, 5, 1),
"59:5: " + getCheckMessage(MSG_KEY, 5, 1),
"68:5: " + getCheckMessage(MSG_KEY, 4, 1),
"76:5: " + getCheckMessage(MSG_KEY, 4, 1),
"86:5: " + getCheckMessage(MSG_KEY, 3, 1),
Expand All @@ -271,6 +271,28 @@ public void testPatternMatchingForSwitch() throws Exception {

}

@Test
public void testWhenExpression() throws Exception {

final String[] expected = {
"14:5: " + getCheckMessage(MSG_KEY, 3, 1),
"20:5: " + getCheckMessage(MSG_KEY, 3, 1),
"28:5: " + getCheckMessage(MSG_KEY, 3, 1),
"36:5: " + getCheckMessage(MSG_KEY, 4, 1),
"44:5: " + getCheckMessage(MSG_KEY, 4, 1),
"52:5: " + getCheckMessage(MSG_KEY, 5, 1),
"60:5: " + getCheckMessage(MSG_KEY, 7, 1),
"69:5: " + getCheckMessage(MSG_KEY, 5, 1),
"77:5: " + getCheckMessage(MSG_KEY, 5, 1),
"85:5: " + getCheckMessage(MSG_KEY, 6, 1),
};

verifyWithInlineConfigParser(
getNonCompilablePath("InputNPathComplexityWhenExpression.java"),
expected);

}

@Test
public void testGetAcceptableTokens() {
final NPathComplexityCheck npathComplexityCheckObj = new NPathComplexityCheck();
Expand All @@ -294,6 +316,7 @@ public void testGetAcceptableTokens() {
TokenTypes.LITERAL_DEFAULT,
TokenTypes.COMPACT_CTOR_DEF,
TokenTypes.SWITCH_RULE,
TokenTypes.LITERAL_WHEN,
};
assertWithMessage("Acceptable tokens should not be null")
.that(actual)
Expand Down Expand Up @@ -326,6 +349,7 @@ public void testGetRequiredTokens() {
TokenTypes.LITERAL_DEFAULT,
TokenTypes.COMPACT_CTOR_DEF,
TokenTypes.SWITCH_RULE,
TokenTypes.LITERAL_WHEN,
};
assertWithMessage("Required tokens should not be null")
.that(actual)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ case B(String s) : {}
}
}

// violation below, 'NPath Complexity is 3 (max allowed is 1)'
// violation below, 'NPath Complexity is 5 (max allowed is 1)'
void testGuardsInRule(Object o) {
switch (o) {
case Integer i when i > 0 -> {}
Expand All @@ -55,7 +55,7 @@ void testGuardsInRule(Object o) {
}
}

// violation below, 'NPath Complexity is 3 (max allowed is 1)'
// violation below, 'NPath Complexity is 5 (max allowed is 1)'
void testGuardsInStatement(Object o) {
switch (o) {
case Integer i when i > 0 : {} break;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
NPathComplexity
max = 1
*/

//non-compiled with javac: Compilable with Java21
package com.puppycrawl.tools.checkstyle.checks.metrics.npathcomplexity;

public class InputNPathComplexityWhenExpression {

// violation below, 'NPath Complexity is 3 (max allowed is 1)'
void m(Object o) {

if (o instanceof String s && !s.isEmpty()) { }
}

// violation below, 'NPath Complexity is 3 (max allowed is 1)'
void m2(Object o) {
switch (o) {
case String s when !s.isEmpty() -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 3 (max allowed is 1)'
void m3(Object o) {
switch (o) {
case String s when (!s.isEmpty()) -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 4 (max allowed is 1)'
void m4(Object o, boolean b) {
switch (o) {
case String s when !s.isEmpty() && b -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 4 (max allowed is 1)'
void m5(Object o, boolean b) {
switch (o) {
case String s when (!s.isEmpty() && b) -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 5 (max allowed is 1)'
void m6(Object o, boolean b, boolean c) {
switch (o) {
case String s when (!s.isEmpty() && b) && c -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 7 (max allowed is 1)'
void m7(Object o, boolean b, boolean c) {
switch (o) {
case String s when !s.isEmpty() -> { }
case Integer i when (i == 0) && c || b -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 5 (max allowed is 1)'
void m8(Object o, boolean b, boolean c) {
switch (o) {
case String s when (b ? true : c) -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 5 (max allowed is 1)'
void m9(Object o, boolean b, boolean c) {
switch (o) {
case String s when b ? true : c -> { }
default -> { }
}
}

// violation below, 'NPath Complexity is 6 (max allowed is 1)'
void m10(Object o, boolean b, boolean c) {
switch (o) {
case String s when (b ? true : c) && c == true -> { }
default -> { }
}
}
}
1 change: 1 addition & 0 deletions src/xdocs/checks/metrics/npathcomplexity.xml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
<tr><td>switch ([expr]) { case : [case-range] default: [default-range] }</td>
<td>S(i=1:i=n)NP(case-range[i]) + NP(default-range) + NP(expr)</td>
</tr>
<tr><td>when[expr]</td><td>NP(expr) + 1</td></tr>
<tr><td>[expr1] ? [expr2] : [expr3]</td><td>NP(expr1) + NP(expr2) + NP(expr3) + 2
</td></tr>
<tr><td>goto label</td><td>1</td></tr>
Expand Down
1 change: 1 addition & 0 deletions src/xdocs/checks/metrics/npathcomplexity.xml.template
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
<tr><td>switch ([expr]) { case : [case-range] default: [default-range] }</td>
<td>S(i=1:i=n)NP(case-range[i]) + NP(default-range) + NP(expr)</td>
</tr>
<tr><td>when[expr]</td><td>NP(expr) + 1</td></tr>
<tr><td>[expr1] ? [expr2] : [expr3]</td><td>NP(expr1) + NP(expr2) + NP(expr3) + 2
</td></tr>
<tr><td>goto label</td><td>1</td></tr>
Expand Down

0 comments on commit 99e0edd

Please sign in to comment.