Merge pull request #2675 from guwirth/fix-2672

fix cognitive complexity problem with r-value references
SonarOpenCommunity · May 4, 2024 · 8b6e479 · 8b6e479
2 parents 22000b3 + f16c3db
commit 8b6e479
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 17 deletions.
diff --git a/cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitor.java b/cxx-squid/src/main/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitor.java
@@ -28,10 +28,10 @@
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.Set;
-import org.sonar.cxx.parser.CxxKeyword;
 import org.sonar.cxx.api.CxxMetric;
-import org.sonar.cxx.parser.CxxPunctuator;
 import org.sonar.cxx.parser.CxxGrammarImpl;
+import org.sonar.cxx.parser.CxxKeyword;
+import org.sonar.cxx.parser.CxxPunctuator;
 import org.sonar.cxx.squidbridge.api.SourceCode;
 
 public class CxxCognitiveComplexityVisitor<G extends Grammar> extends MultiLocatitionSquidCheck<G> {
@@ -74,7 +74,8 @@ public class CxxCognitiveComplexityVisitor<G extends Grammar> extends MultiLocat
   private static final Set<AstNodeType> SUBSCRIPTION_NODES = new HashSet<>();
 
   static {
-    SUBSCRIPTION_NODES.add(CxxGrammarImpl.functionDefinition);
+    SUBSCRIPTION_NODES.add(CxxGrammarImpl.functionBody); // root node for Cognitive Complexity
+
     SUBSCRIPTION_NODES.addAll(Arrays.asList(DESCENDANT_TYPES));
     SUBSCRIPTION_NODES.addAll(Arrays.asList(INCREMENT_TYPES));
     SUBSCRIPTION_NODES.addAll(Arrays.asList(NESTING_LEVEL_TYPES));
@@ -102,11 +103,16 @@ public void visitNode(AstNode node) {
       return;
     }
 
-    if (node.is(CxxGrammarImpl.functionDefinition)) {
+    if (node.is(CxxGrammarImpl.functionBody) && node.hasDirectChildren(CxxGrammarImpl.compoundStatement)) {
       complexityScopes.addFirst(new CxxComplexityScope(node.getTokenLine()));
+      return;
     }
 
-    if (complexityScopes.isEmpty() || isElseIf(node)) {
+    if (complexityScopes.isEmpty()) {
+      return;
+    }
+
+    if (isElseIf(node)) {
       return;
     }
 
@@ -125,21 +131,27 @@ public void visitNode(AstNode node) {
 
   @Override
   public void leaveNode(AstNode node) {
-    if (node.getToken().isGeneratedCode()) {
-      return;
-    }
+    if (!complexityScopes.isEmpty()) {
+      if (node.getToken().isGeneratedCode()) {
+        return;
+      }
 
-    if (node.is(CxxGrammarImpl.functionDefinition)) {
-      analyzeComplexity(complexityScopes.removeFirst());
-    }
+      if (node.is(CxxGrammarImpl.functionBody)) {
+        analyzeComplexity(complexityScopes.removeFirst());
+      }
 
-    if (complexityScopes.isEmpty() || isElseIf(node)) {
-      return;
-    }
+      if (complexityScopes.isEmpty()) {
+        return;
+      }
 
-    if (node.is(NESTING_LEVEL_TYPES)) {
-      for (var scope : complexityScopes) {
-        scope.decreaseNesting();
+      if (isElseIf(node)) {
+        return;
+      }
+
+      if (node.is(NESTING_LEVEL_TYPES)) {
+        for (var scope : complexityScopes) {
+          scope.decreaseNesting();
+        }
       }
     }
   }

diff --git a/cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitorTest.java b/cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxCognitiveComplexityVisitorTest.java
@@ -150,6 +150,16 @@ void to_regexp() throws UnsupportedEncodingException, IOException {
     assertThat(testFile("src/test/resources/visitors/to_regexp.cc")).isEqualTo(20);
   }
 
+  @Test
+  void template() throws UnsupportedEncodingException, IOException {
+    assertThat(testFile("src/test/resources/visitors/template.cc")).isEqualTo(0);
+  }
+
+  @Test
+  void inline() throws UnsupportedEncodingException, IOException {
+    assertThat(testFile("src/test/resources/visitors/inline.cc")).isEqualTo(5);
+  }
+
   private int testFile(String fileName) throws UnsupportedEncodingException, IOException {
     var tester = CxxFileTesterHelper.create(fileName, ".", "");
     SourceFile sourceFile = CxxAstScanner.scanSingleInputFile(tester.asInputFile());

diff --git a/cxx-squid/src/test/resources/visitors/inline.cc b/cxx-squid/src/test/resources/visitors/inline.cc
@@ -0,0 +1,43 @@
+struct DeleteSample { // = 0
+    DeleteSample(const DeleteSample&) = delete;
+    DeleteSample& operator=(const DeleteSample&) = delete;
+    DeleteSample(DeleteSample&&) = delete;
+    DeleteSample& operator=(DeleteSample&&) = delete;
+    ~DeleteSample() = default;
+};
+
+struct DefaultSample { // = 0
+    DefaultSample(const DefaultSample&) = default;
+    DefaultSample& operator=(const DefaultSample&) = default;
+    DefaultSample(DefaultSample&&) = default;
+    DefaultSample& operator=(DefaultSample&&) = default;
+    ~DefaultSample() = default;
+};
+
+struct Sample { // = 5
+    Sample(const Sample&) {
+        if (true) {
+            std::cout << "+1" << std::endl;
+        }
+    }
+    Sample& operator=(const Sample&) {
+        if (true) {
+            std::cout << "+1" << std::endl;
+        }
+    }
+    Sample(Sample&&) {
+        if (true) {
+            std::cout << "+1" << std::endl;
+        }
+    }
+    Sample& operator=(Sample&&) {
+        if (true) {
+            std::cout << "+1" << std::endl;
+        }
+    }
+    ~Sample() {
+        if (true) {
+            std::cout << "+1" << std::endl;
+        }
+    }
+};
diff --git a/cxx-squid/src/test/resources/visitors/template.cc b/cxx-squid/src/test/resources/visitors/template.cc
@@ -0,0 +1,11 @@
+template<typename T>
+class host : public std::enable_shared_from_this<host<T>> {
+public:
+  template<typename... U>
+  explicit host(U&&... args);             // should ignore &&
+  host() = delete;
+  host(const host&) = delete;
+  host(host&&)      = default;            // should ignore && and default
+  host& operator=(const host&) = delete;
+  host& operator=(host&&)      = default; // should ignore && and default
+};