SONARJAVA-5657 S6541: use NODV metric instead of NOAV

Author: asya-vorobeva

java-checks/src/main/java/org/sonar/java/checks/design/BrainMethodCheck.java

@@ -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() {
@@ -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);

java-checks/src/test/java/org/sonar/java/checks/design/BrainMethodCheckTest.java

@@ -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;
 
@@ -63,7 +63,7 @@ void testLowComplexityFileWithLowerThresholds() {
     var check = new BrainMethodCheck();
 
     check.locThreshold = 14;
-    check.noavThreshold = 4;
+    check.nodvThreshold = 4;
     check.cyclomaticThreshold = 5;
 
     CheckVerifier.newVerifier()
@@ -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;
 

…rs/NumberOfAccessedVariablesVisitor.java …ors/NumberOfDefinedVariablesVisitor.javajava-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitor.java renamed to java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitor.java renamed to java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java

@@ -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;
+    numberOfDefinedVariables = 0;
     scan(tree);
-    return numberOfAccessedVariables;
+    return numberOfDefinedVariables;
   }
 
 }

java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java

@@ -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;
@@ -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<>();
@@ -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.getNumberOfAccessedVariables(tree));
   }
 
   LinesOfCodeVisitor linesOfCodeVisitor = new LinesOfCodeVisitor();
@@ -114,8 +114,8 @@ Map<Integer, Integer> getCompilationUnityComplexity() {
   }
 
   @VisibleForTesting
-  Map<Integer, Integer> getMethodNumberOfAccessedVariables() {
-    return methodNumberOfAccessedVariables;
+  Map<Integer, Integer> getMethodNumberOfDefinedVariables() {
+    return methodNumberOfDefinedVariables;
   }
 
   @VisibleForTesting

…umberOfAccessedVariablesVisitorTest.java …NumberOfDefinedVariablesVisitorTest.javajava-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitorTest.java renamed to java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitorTest.java renamed to java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java

@@ -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().getNumberOfAccessedVariables(methodTree);
     assertThat(numberOfVariables).isZero();
   }
 
@@ -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().getNumberOfAccessedVariables(methodTree);
     assertThat(numberOfVariables).isEqualTo(3);
   }
 
@@ -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().getNumberOfAccessedVariables(methodTree);
     assertThat(numberOfVariables).isEqualTo(2);
   }
 

java-frontend/src/test/java/org/sonar/java/metrics/MetricsComputerTest.java

@@ -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);

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6541.html

@@ -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