From caf58a0aa9534f4c8c38e0de518fda7841f46613 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Thu, 5 Mar 2026 12:52:18 +0100 Subject: [PATCH 1/2] SONARJAVA-5657 S6541: use NODV metric instead of NOAV --- .../sonar/java/checks/design/BrainMethodCheck.java | 12 ++++++------ .../java/checks/design/BrainMethodCheckTest.java | 6 +++--- ...r.java => NumberOfDefinedVariablesVisitor.java} | 10 +++++----- .../org/sonar/java/metrics/MetricsComputer.java | 14 +++++++------- ...va => NumberOfDefinedVariablesVisitorTest.java} | 8 ++++---- .../sonar/java/metrics/MetricsComputerTest.java | 6 +++--- .../org/sonar/l10n/java/rules/java/S6541.html | 10 ++++++++++ 7 files changed, 38 insertions(+), 28 deletions(-) rename java-frontend/src/main/java/org/sonar/java/ast/visitors/{NumberOfAccessedVariablesVisitor.java => NumberOfDefinedVariablesVisitor.java} (82%) rename java-frontend/src/test/java/org/sonar/java/ast/visitors/{NumberOfAccessedVariablesVisitorTest.java => NumberOfDefinedVariablesVisitorTest.java} (85%) diff --git a/java-checks/src/main/java/org/sonar/java/checks/design/BrainMethodCheck.java b/java-checks/src/main/java/org/sonar/java/checks/design/BrainMethodCheck.java index a321b2a0756..f9b7235c757 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/design/BrainMethodCheck.java +++ b/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 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); diff --git a/java-checks/src/test/java/org/sonar/java/checks/design/BrainMethodCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/design/BrainMethodCheckTest.java index 128cb9fc26c..1d209783f61 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/design/BrainMethodCheckTest.java +++ b/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; diff --git a/java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitor.java b/java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java similarity index 82% rename from java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitor.java rename to java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java index e491d948a5c..d544751af74 100644 --- a/java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitor.java +++ b/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; } } diff --git a/java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java b/java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java index f1b6d30a9b8..2f0cb3a8bf0 100644 --- a/java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java +++ b/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> methodComplexityNodes = new HashMap<>(); private final Map methodComplexity = new HashMap<>(); private final Map compilationUnityComplexity = new HashMap<>(); - private final Map methodNumberOfAccessedVariables = new HashMap<>(); + private final Map methodNumberOfDefinedVariables = new HashMap<>(); private final Map treeLinesOfCode = new HashMap<>(); private final Map treeNumberOfStatements = new HashMap<>(); private final Map 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 getCompilationUnityComplexity() { } @VisibleForTesting - Map getMethodNumberOfAccessedVariables() { - return methodNumberOfAccessedVariables; + Map getMethodNumberOfDefinedVariables() { + return methodNumberOfDefinedVariables; } @VisibleForTesting diff --git a/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitorTest.java b/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java similarity index 85% rename from java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitorTest.java rename to java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java index 3868fe5106b..34696874af3 100644 --- a/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfAccessedVariablesVisitorTest.java +++ b/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java @@ -24,7 +24,7 @@ import static org.assertj.core.api.Assertions.assertThat; -class NumberOfAccessedVariablesVisitorTest { +class NumberOfDefinedVariablesVisitorTest { @Test void zeroVariables() { @@ -32,7 +32,7 @@ void zeroVariables() { " 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); } diff --git a/java-frontend/src/test/java/org/sonar/java/metrics/MetricsComputerTest.java b/java-frontend/src/test/java/org/sonar/java/metrics/MetricsComputerTest.java index 827c10d118c..98575bfde10 100644 --- a/java-frontend/src/test/java/org/sonar/java/metrics/MetricsComputerTest.java +++ b/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); diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6541.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6541.html index 8d1ac8047c5..dcc51fd05ae 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6541.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6541.html @@ -6,6 +6,16 @@

Why is this an issue?

too many conditions, using lots of variables, and ultimately making it difficult to understand, maintain and reuse.
It is characterized by high LOC number, high cyclomatic and cognitive complexity, and a large number of variables being used.

+ + + + + + + +
NoteThis rule uses a modified definition of Brain Method. Instead of the Number of Accessed Variables (NOAV) metric described + in the original research, this implementation uses the Number of Defined Variables (NODV) metric, which counts the number of + distinct local variables defined within the method’s scope.

What is the potential impact?

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 From 5efa845fd7c20bdfd9a707f1001423d38cb71223 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Thu, 5 Mar 2026 13:33:00 +0100 Subject: [PATCH 2/2] SONARJAVA-5657 S6541: use NODV metric instead of NOAV --- .../java/ast/visitors/NumberOfDefinedVariablesVisitor.java | 2 +- .../main/java/org/sonar/java/metrics/MetricsComputer.java | 2 +- .../ast/visitors/NumberOfDefinedVariablesVisitorTest.java | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java b/java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java index d544751af74..a1847920577 100644 --- a/java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java +++ b/java-frontend/src/main/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitor.java @@ -28,7 +28,7 @@ public void visitVariable(VariableTree tree) { numberOfDefinedVariables++; } - public int getNumberOfAccessedVariables(MethodTree tree) { + public int getNumberOfDefinedVariables(MethodTree tree) { numberOfDefinedVariables = 0; scan(tree); return numberOfDefinedVariables; diff --git a/java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java b/java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java index 2f0cb3a8bf0..f0ebb9ba8fd 100644 --- a/java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java +++ b/java-frontend/src/main/java/org/sonar/java/metrics/MetricsComputer.java @@ -57,7 +57,7 @@ public CognitiveComplexityVisitor.Result getMethodComplexity(MethodTree tree) { NumberOfDefinedVariablesVisitor methodBodyVisitor = new NumberOfDefinedVariablesVisitor(); public int getNumberOfDefinedVariables(MethodTree tree) { - return methodNumberOfDefinedVariables.computeIfAbsent(tree.hashCode(), k -> methodBodyVisitor.getNumberOfAccessedVariables(tree)); + return methodNumberOfDefinedVariables.computeIfAbsent(tree.hashCode(), k -> methodBodyVisitor.getNumberOfDefinedVariables(tree)); } LinesOfCodeVisitor linesOfCodeVisitor = new LinesOfCodeVisitor(); diff --git a/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java b/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java index 34696874af3..571111bb522 100644 --- a/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java +++ b/java-frontend/src/test/java/org/sonar/java/ast/visitors/NumberOfDefinedVariablesVisitorTest.java @@ -32,7 +32,7 @@ void zeroVariables() { " private Object foo(){ }" + "}"); MethodTree methodTree = (MethodTree) ((ClassTree) cut.types().get(0)).members().get(0); - int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfAccessedVariables(methodTree); + int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfDefinedVariables(methodTree); assertThat(numberOfVariables).isZero(); } @@ -45,7 +45,7 @@ void threeVariables() { + "}" + "}"); MethodTree methodTree = (MethodTree) ((ClassTree) cut.types().get(0)).members().get(0); - int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfAccessedVariables(methodTree); + int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfDefinedVariables(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 NumberOfDefinedVariablesVisitor().getNumberOfAccessedVariables(methodTree); + int numberOfVariables = new NumberOfDefinedVariablesVisitor().getNumberOfDefinedVariables(methodTree); assertThat(numberOfVariables).isEqualTo(2); }