Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ public class BrainMethodCheck extends IssuableSubscriptionVisitor implements End
@RuleProperty(key = "nestingThreshold", description = "The maximum nesting level allowed.", defaultValue = "" + DEFAULT_NESTING_THRESHOLD)
public int nestingThreshold = DEFAULT_NESTING_THRESHOLD;

@RuleProperty(key = "noavThreshold", description = "The maximum number of accessed variables allowed.", defaultValue = "" + DEFAULT_VARIABLES_THRESHOLD)
public int noavThreshold = DEFAULT_VARIABLES_THRESHOLD;
@RuleProperty(key = "nodvThreshold", description = "The maximum number of defined local variables allowed.", defaultValue = "" + DEFAULT_VARIABLES_THRESHOLD)
public int nodvThreshold = DEFAULT_VARIABLES_THRESHOLD;

@Override
public List<Tree.Kind> nodesToVisit() {
Expand All @@ -84,19 +84,19 @@ public void visitNode(Tree tree) {
int cyclomaticComplexity = metricsComputer.getComplexityNodes(methodTree).size();
int maxNestingLevel = metricsComputer.getMethodNestingLevel(methodTree);
int linesOfCode = metricsComputer.getLinesOfCode(methodTree.block());
int numberOfAccessedVariables = metricsComputer.getNumberOfAccessedVariables(methodTree);
int numberOfDefinedVariables = metricsComputer.getNumberOfDefinedVariables(methodTree);

if (linesOfCode >= locThreshold &&
cyclomaticComplexity >= cyclomaticThreshold &&
maxNestingLevel >= nestingThreshold &&
numberOfAccessedVariables >= noavThreshold) {
numberOfDefinedVariables >= nodvThreshold) {

int brainScore = numberOfAccessedVariables + cyclomaticComplexity + maxNestingLevel * linesOfCode;
int brainScore = numberOfDefinedVariables + cyclomaticComplexity + maxNestingLevel * linesOfCode;
String issueMessage = String.format(ISSUE_MESSAGE,
linesOfCode, locThreshold - 1,
cyclomaticComplexity, cyclomaticThreshold - 1,
maxNestingLevel, nestingThreshold - 1,
numberOfAccessedVariables, noavThreshold - 1);
numberOfDefinedVariables, nodvThreshold - 1);

AnalyzerMessage analyzerMessage = new AnalyzerMessage(this, context.getInputFile(),
AnalyzerMessage.textSpanFor(methodTree.simpleName()), issueMessage, 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void testHighComplexityFileWithHigherThresholds() {
var check = new BrainMethodCheck();

check.locThreshold = 120;
check.noavThreshold = 36;
check.nodvThreshold = 36;
check.nestingThreshold = 8;
check.cyclomaticThreshold = 45;

Expand All @@ -63,7 +63,7 @@ void testLowComplexityFileWithLowerThresholds() {
var check = new BrainMethodCheck();

check.locThreshold = 14;
check.noavThreshold = 4;
check.nodvThreshold = 4;
check.cyclomaticThreshold = 5;

CheckVerifier.newVerifier()
Expand All @@ -77,7 +77,7 @@ void testSubsetOfIssuesWithLowerThresholds() {
var check = new BrainMethodCheck();

check.locThreshold = 4;
check.noavThreshold = 2;
check.nodvThreshold = 2;
check.cyclomaticThreshold = 1;
check.nestingThreshold = 1;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.VariableTree;

public class NumberOfAccessedVariablesVisitor extends BaseTreeVisitor {
private int numberOfAccessedVariables = 0;
public class NumberOfDefinedVariablesVisitor extends BaseTreeVisitor {
private int numberOfDefinedVariables = 0;

@Override
public void visitVariable(VariableTree tree) {
numberOfAccessedVariables++;
numberOfDefinedVariables++;
}

public int getNumberOfAccessedVariables(MethodTree tree) {
numberOfAccessedVariables = 0;
public int getNumberOfDefinedVariables(MethodTree tree) {
numberOfDefinedVariables = 0;
scan(tree);
return numberOfAccessedVariables;
return numberOfDefinedVariables;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.sonar.java.ast.visitors.ComplexityVisitor;
import org.sonar.java.ast.visitors.LinesOfCodeVisitor;
import org.sonar.java.ast.visitors.MethodNestingLevelVisitor;
import org.sonar.java.ast.visitors.NumberOfAccessedVariablesVisitor;
import org.sonar.java.ast.visitors.NumberOfDefinedVariablesVisitor;
import org.sonar.java.ast.visitors.StatementVisitor;
import org.sonar.plugins.java.api.tree.CompilationUnitTree;
import org.sonar.plugins.java.api.tree.MethodTree;
Expand All @@ -37,7 +37,7 @@ public class MetricsComputer {
private final Map<Integer, List<Tree>> methodComplexityNodes = new HashMap<>();
private final Map<Integer, CognitiveComplexityVisitor.Result> methodComplexity = new HashMap<>();
private final Map<Integer, Integer> compilationUnityComplexity = new HashMap<>();
private final Map<Integer, Integer> methodNumberOfAccessedVariables = new HashMap<>();
private final Map<Integer, Integer> methodNumberOfDefinedVariables = new HashMap<>();
private final Map<Integer, Integer> treeLinesOfCode = new HashMap<>();
private final Map<Integer, Integer> treeNumberOfStatements = new HashMap<>();
private final Map<Integer, Integer> treeNumberOfCommentedLines = new HashMap<>();
Expand All @@ -54,10 +54,10 @@ public CognitiveComplexityVisitor.Result getMethodComplexity(MethodTree tree) {
return methodComplexity.computeIfAbsent(tree.hashCode(), k -> CognitiveComplexityVisitor.methodComplexity(tree));
}

NumberOfAccessedVariablesVisitor methodBodyVisitor = new NumberOfAccessedVariablesVisitor();
NumberOfDefinedVariablesVisitor methodBodyVisitor = new NumberOfDefinedVariablesVisitor();

public int getNumberOfAccessedVariables(MethodTree tree) {
return methodNumberOfAccessedVariables.computeIfAbsent(tree.hashCode(), k -> methodBodyVisitor.getNumberOfAccessedVariables(tree));
public int getNumberOfDefinedVariables(MethodTree tree) {
return methodNumberOfDefinedVariables.computeIfAbsent(tree.hashCode(), k -> methodBodyVisitor.getNumberOfDefinedVariables(tree));
}

LinesOfCodeVisitor linesOfCodeVisitor = new LinesOfCodeVisitor();
Expand Down Expand Up @@ -114,8 +114,8 @@ Map<Integer, Integer> getCompilationUnityComplexity() {
}

@VisibleForTesting
Map<Integer, Integer> getMethodNumberOfAccessedVariables() {
return methodNumberOfAccessedVariables;
Map<Integer, Integer> getMethodNumberOfDefinedVariables() {
return methodNumberOfDefinedVariables;
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@

import static org.assertj.core.api.Assertions.assertThat;

class NumberOfAccessedVariablesVisitorTest {
class NumberOfDefinedVariablesVisitorTest {

@Test
void zeroVariables() {
CompilationUnitTree cut = JParserTestUtils.parse("class A {" +
" private Object foo(){ }" +
"}");
MethodTree methodTree = (MethodTree) ((ClassTree) cut.types().get(0)).members().get(0);
int numberOfVariables = new NumberOfAccessedVariablesVisitor().getNumberOfAccessedVariables(methodTree);
int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfDefinedVariables(methodTree);
assertThat(numberOfVariables).isZero();
}

Expand All @@ -45,7 +45,7 @@ void threeVariables() {
+ "}" +
"}");
MethodTree methodTree = (MethodTree) ((ClassTree) cut.types().get(0)).members().get(0);
int numberOfVariables = new NumberOfAccessedVariablesVisitor().getNumberOfAccessedVariables(methodTree);
int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfDefinedVariables(methodTree);
assertThat(numberOfVariables).isEqualTo(3);
}

Expand All @@ -60,7 +60,7 @@ void multipleAccessesOnSameVariableDoNotCount() {
+ "}" +
"}");
MethodTree methodTree = (MethodTree) ((ClassTree) cut.types().get(0)).members().get(0);
int numberOfVariables = new NumberOfAccessedVariablesVisitor().getNumberOfAccessedVariables(methodTree);
int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfDefinedVariables(methodTree);
assertThat(numberOfVariables).isEqualTo(2);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ void testMetricsPresence() throws SecurityException, IllegalArgumentException {
mc.getMethodComplexity(methodTree);
assertThat(mc.getMethodComplexity()).containsKey(methodTree.hashCode());

assertThat(mc.getMethodNumberOfAccessedVariables()).isEmpty();
mc.getNumberOfAccessedVariables(methodTree);
assertThat(mc.getMethodNumberOfAccessedVariables()).containsKey(methodTree.hashCode());
assertThat(mc.getMethodNumberOfDefinedVariables()).isEmpty();
mc.getNumberOfDefinedVariables(methodTree);
assertThat(mc.getMethodNumberOfDefinedVariables()).containsKey(methodTree.hashCode());

assertThat(mc.getTreeLinesOfCode()).isEmpty();
mc.getLinesOfCode(methodTree);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ <h2>Why is this an issue?</h2>
too many conditions, using lots of variables, and ultimately making it difficult to understand, maintain and reuse.
<br>
It is characterized by high LOC number, high cyclomatic and cognitive complexity, and a large number of variables being used.</p>
<table>
<tbody>
<tr>
<td>Note</td>
<td>This rule uses a modified definition of Brain Method. Instead of the <strong>Number of Accessed Variables (NOAV)</strong> metric described
in the original research, this implementation uses the <strong>Number of Defined Variables (NODV)</strong> metric, which counts the number of
distinct local variables defined within the method’s scope.</td>
</tr>
</tbody>
</table>
<h3>What is the potential impact?</h3>
<p>Brain Methods are often hard to cover with tests, because of their deep nesting, and they are error-prone, because of the many local variables they
usually introduce. Such methods will be very hard to read and understand for anyone outside who created them, and therefore hard to maintain and fix
Expand Down