SONARJAVA-2113 Feed cognititve complexity metric #1385

m-g-sonar · 2017-04-12T16:49:29Z

No description provided.

m-g-sonar · 2017-04-13T09:22:38Z

java-frontend/src/main/java/org/sonar/java/ast/visitors/CognitiveComplexityVisitor.java

+    if (shouldAnalyzeMethod(semanticModel, methodTree)) {
+      CognitiveComplexityVisitor visitor = new CognitiveComplexityVisitor();
+      methodTree.accept(visitor);
+      return new JavaFileScannerContext.CognitiveComplexity(visitor.complexity, visitor.locations);


FP on S2440, see https://jira.sonarsource.com/browse/SONARJAVA-2098

benzonico · 2017-04-13T10:02:34Z

java-frontend/src/main/java/org/sonar/java/Measurer.java

  }

  private boolean isSonarLintContext() {
    return sensorContext.getSonarQubeVersion().isGreaterThanOrEqual(Version.create(6, 0)) && sensorContext.runtime().getProduct() == SonarProduct.SONARLINT;
  }

+  private boolean isGreaterThanOrEqualToSonarQube6_3() {


this is written isSQGreaterThan62 in sonarComponents, I would use the same naming.

benzonico · 2017-04-13T10:03:43Z

java-frontend/src/main/java/org/sonar/plugins/java/api/JavaFileScannerContext.java

+   * Cognitive complexity and associated locations
+   */
+  class CognitiveComplexity {
+    public static final CognitiveComplexity EMPTY = new CognitiveComplexity(0, Collections.emptyList());


should be private.

benzonico · 2017-04-13T10:07:49Z

java-frontend/src/main/java/org/sonar/java/ast/visitors/CognitiveComplexityVisitor.java

@@ -64,20 +66,49 @@ private CognitiveComplexityVisitor() {
    complexity = 0;
    nesting = 1;
    ignoreNesting = false;
-    flow = new ArrayList<>();
+    locations = new ArrayList<>();
    ignored = new HashSet<>();
  }

  public static JavaFileScannerContext.CognitiveComplexity methodComplexity(@Nullable SemanticModel semanticModel, MethodTree methodTree) {
    if (shouldAnalyzeMethod(semanticModel, methodTree)) {


I am pretty sure we can get rid of the semanticModel parameters in most of the methods here... this is only used to be null tested... we can maybe get rid of this reference alltogether in that class.

m-g-sonar · 2017-04-18T16:08:17Z

java-frontend/src/main/java/org/sonar/java/ast/visitors/CognitiveComplexityVisitor.java

+    if (shouldAnalyzeMethod(methodTree)) {
+      CognitiveComplexityVisitor visitor = new CognitiveComplexityVisitor();
+      methodTree.accept(visitor);
+      return new JavaFileScannerContext.CognitiveComplexity(visitor.complexity, visitor.locations);


FP on S2440, see https://jira.sonarsource.com/browse/SONARJAVA-2098

saberduck

If I didn't miss anything, it seems to be possible to avoid extending JavaFileScannerContext interface by using CognitiveComplexityVisitor directly.

saberduck · 2017-04-19T14:34:49Z

java-frontend/src/main/java/org/sonar/java/ast/visitors/CognitiveComplexityVisitor.java

+
+
+  private static boolean shouldAnalyzeMethod(MethodTree methodTree) {
+    return methodTree.block() != null && ((ClassTree) methodTree.parent()).simpleName() != null && !isWithinLocalClass(methodTree);


minor: simpleName() != null could be extracted to isAnonymousClass method, so the intent of the test is more obvious

saberduck · 2017-04-19T15:16:11Z

java-frontend/src/main/java/org/sonar/java/Measurer.java

@@ -111,12 +112,20 @@ public void scanFile(JavaFileScannerContext context) {

    RangeDistributionBuilder fileComplexityDistribution = new RangeDistributionBuilder(LIMITS_COMPLEXITY_FILES);
    saveMetricOnFile(CoreMetrics.FILE_COMPLEXITY_DISTRIBUTION, fileComplexityDistribution.add(fileComplexity).build());
+
+    if (isSonarQubeGreaterThanOrEqualTo63()) {
+      saveMetricOnFile(CoreMetrics.COGNITIVE_COMPLEXITY, context.compilationUnitCognitiveComplexity());


context.getTree() provides access to CompilationUnitTree, so CognitiveComplexityVisitor.cognitiveComplexity can be used directly, thus avoiding the need for new method in the JavaScannerContext interface

saberduck · 2017-04-19T15:20:06Z

java-checks/src/main/java/org/sonar/java/checks/CognitiveComplexityMethodCheck.java

-    CognitiveComplexityVisitor complexityVisitor = new CognitiveComplexityVisitor();
-    method.accept(complexityVisitor);
-    int total = complexityVisitor.complexity;
+    JavaFileScannerContext.CognitiveComplexity result = context.cognitiveComplexity(method);


CognitiveComplexityVisitor.methodComplexity can be used directly, so method on JavaScannerContext can be removed

saberduck · 2017-04-19T15:25:47Z

java-frontend/src/main/java/org/sonar/java/Measurer.java

@@ -111,12 +112,20 @@ public void scanFile(JavaFileScannerContext context) {

    RangeDistributionBuilder fileComplexityDistribution = new RangeDistributionBuilder(LIMITS_COMPLEXITY_FILES);
    saveMetricOnFile(CoreMetrics.FILE_COMPLEXITY_DISTRIBUTION, fileComplexityDistribution.add(fileComplexity).build());
+
+    if (isSonarQubeGreaterThanOrEqualTo63()) {


It seems that this version checking is needed in multiple places in the code, maybe it is worth to have single utility class handling these checks.

Maybe, but clearly way out of scope of this ticket.

Implementation updated.

m-g-sonar force-pushed the SONARJAVA-2113 branch from 9c7f36c to 8719d2c Compare April 13, 2017 08:55

m-g-sonar commented Apr 13, 2017

View reviewed changes

benzonico previously requested changes Apr 13, 2017

View reviewed changes

m-g-sonar force-pushed the SONARJAVA-2113 branch from 8719d2c to 650ae35 Compare April 18, 2017 15:56

m-g-sonar commented Apr 18, 2017

View reviewed changes

saberduck reviewed Apr 19, 2017

View reviewed changes

Wohops added 4 commits April 19, 2017 18:02

SONARJAVA-2113 Extract cognitive complexity visitor from S3776

0a4019a

SONARJAVA-2113 Use JavaFileScannerContext to get method complexity

9d06ed8

SONARJAVA-2113 Feed the cognitive complexity metric

5667077

SONARJAVA-2113 Do not modify API to feed the metric or the rule

626ffe0

m-g-sonar force-pushed the SONARJAVA-2113 branch from 650ae35 to 626ffe0 Compare April 19, 2017 16:03

m-g-sonar merged commit 88bcd69 into master Apr 20, 2017

m-g-sonar deleted the SONARJAVA-2113 branch April 20, 2017 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SONARJAVA-2113 Feed cognititve complexity metric #1385

SONARJAVA-2113 Feed cognititve complexity metric #1385

m-g-sonar commented Apr 12, 2017

m-g-sonar Apr 13, 2017

benzonico Apr 13, 2017

benzonico Apr 13, 2017

benzonico Apr 13, 2017

m-g-sonar Apr 18, 2017

saberduck left a comment

saberduck Apr 19, 2017

m-g-sonar Apr 19, 2017

saberduck Apr 19, 2017

m-g-sonar Apr 19, 2017

saberduck Apr 19, 2017

m-g-sonar Apr 19, 2017

saberduck Apr 19, 2017

m-g-sonar Apr 19, 2017



		private static boolean shouldAnalyzeMethod(MethodTree methodTree) {
		return methodTree.block() != null && ((ClassTree) methodTree.parent()).simpleName() != null && !isWithinLocalClass(methodTree);

SONARJAVA-2113 Feed cognititve complexity metric #1385

SONARJAVA-2113 Feed cognititve complexity metric #1385

Conversation

m-g-sonar commented Apr 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saberduck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment