Iterate over all types instead of all methods, store 0 mccabe values …

…too (#683)
Ericsson · Apr 11, 2024 · 22794ff · 22794ff
1 parent 2faec19
commit 22794ff
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 54 deletions.
diff --git a/plugins/cpp_metrics/parser/src/cppmetricsparser.cpp b/plugins/cpp_metrics/parser/src/cppmetricsparser.cpp
@@ -152,59 +152,53 @@ void CppMetricsParser::typeMcCabe()
 
     std::map<model::CppAstNodeId, unsigned int> mcValues;
 
-    // Process all methods.
-    for (const auto& member : _ctx.db->query<MemberT>(
-      odb::query<MemberT>::kind == MemberT::Kind::Method))
+    // Process all class definitions
+    for (const auto& type : _ctx.db->query<AstNode>(
+      odb::query<AstNode>::symbolType == AstNode::SymbolType::Type &&
+      odb::query<AstNode>::astType == AstNode::AstType::Definition))
     {
-      // Lookup the class definition.
-      const auto classAstNode = _ctx.db->query_one<AstNode>(
-        odb::query<AstNode>::entityHash == member.typeHash &&
-        odb::query<AstNode>::astType == AstNode::AstType::Definition);
-
-      // Skip if class was not found or is included from external library.
-      if (!classAstNode || !classAstNode->location.file)
-        continue;
-      classAstNode->location.file.load();
-      const auto classFile = _ctx.db->query_one<model::File>(
-        odb::query<model::File>::id == classAstNode->location.file->id);
-      if (!classFile || !cc::util::isRootedUnderAnyOf(_inputPaths, classFile->path))
+      // Skip if class is included from external library
+      type.location.file.load();
+      const auto typeFile = _ctx.db->query_one<model::File>(
+        odb::query<model::File>::id == type.location.file->id);
+      if (!typeFile || !cc::util::isRootedUnderAnyOf(_inputPaths, typeFile->path))
         continue;
 
-      // Lookup AST node of method
-      member.memberAstNode.load();
-      const auto methodAstNode = _ctx.db->query_one<AstNode>(
-          odb::query<AstNode>::id == member.memberAstNode->id);
-      if (!methodAstNode)
-        continue;
+      mcValues[type.id] = 0;
 
-      // Lookup the definition (different AST node if not defined in class body).
-      const auto methodDef = _ctx.db->query_one<AstNode>(
+      // Process its methods
+      for (const auto& method : _ctx.db->query<MemberT>(
+        odb::query<MemberT>::typeHash == type.entityHash &&
+        odb::query<MemberT>::kind == MemberT::Kind::Method))
+      {
+        // Lookup AST node of method
+        method.memberAstNode.load();
+        const auto methodAstNode = _ctx.db->query_one<AstNode>(
+          odb::query<AstNode>::id == method.memberAstNode->id);
+        if (!methodAstNode)
+          continue;
+
+        // Lookup the definition (different AST node if not defined in class body)
+        const auto methodDef = _ctx.db->query_one<AstNode>(
           odb::query<AstNode>::entityHash == methodAstNode->entityHash &&
           odb::query<AstNode>::astType == AstNode::AstType::Definition);
-      if (!methodDef)
-        continue;
+        if (!methodDef)
+          continue;
 
-      // Skip implicitly defined methods (constructors, operator=, etc.)
-      const auto entity = _ctx.db->query_one<Entity>(
+        // Skip implicitly defined methods (constructors, operator=, etc.)
+        const auto entity = _ctx.db->query_one<Entity>(
           odb::query<Entity>::astNodeId == methodDef->id);
-      if (entity && entity->tags.find(model::Tag::Implicit) != entity->tags.cend())
-        continue;
-
-      // Lookup metrics of this definition.
-      const auto funcMetrics = _ctx.db->query_one<AstNodeMet>(
-        odb::query<AstNodeMet>::astNodeId == methodDef->id &&
-        odb::query<AstNodeMet>::type == model::CppAstNodeMetrics::Type::MCCABE);
-      if (funcMetrics)
-      {
-        // Increase class mccabe by the method's.
-        const auto itMcValue = mcValues.find(classAstNode->id);
-        if (itMcValue != mcValues.cend())
-        {
-          itMcValue->second += funcMetrics->value;
-        }
-        else
+        if (entity && entity->tags.find(model::Tag::Implicit) != entity->tags.cend())
+          continue;
+
+        // Lookup metrics of this definition
+        const auto funcMetrics = _ctx.db->query_one<AstNodeMet>(
+          odb::query<AstNodeMet>::astNodeId == methodDef->id &&
+          odb::query<AstNodeMet>::type == model::CppAstNodeMetrics::Type::MCCABE);
+        if (funcMetrics)
         {
-          mcValues[classAstNode->id] = funcMetrics->value;
+          // Increase class mccabe by the method's
+          mcValues[type.id] += funcMetrics->value;
         }
       }
     }

diff --git a/plugins/cpp_metrics/test/src/cppmetricsparsertest.cpp b/plugins/cpp_metrics/test/src/cppmetricsparsertest.cpp
@@ -117,19 +117,11 @@ TEST_P(ParameterizedTypeMcCabeTest, TypeMcCabeTest) {
     const auto record = _db->query_value<model::CppRecord>(
       odb::query<model::CppRecord>::qualifiedName == GetParam().first);
 
-    const auto metric = _db->query_one<model::CppAstNodeMetrics>(
+    const auto metric = _db->query_value<model::CppAstNodeMetrics>(
       odb::query<model::CppAstNodeMetrics>::astNodeId == record.astNodeId &&
       odb::query<model::CppAstNodeMetrics>::type == model::CppAstNodeMetrics::MCCABE_TYPE);
 
-    if (GetParam().second == 0)
-    {
-      ASSERT_TRUE(metric == nullptr);
-    }
-    else
-    {
-      ASSERT_TRUE(metric != nullptr);
-      EXPECT_EQ(GetParam().second, metric->value);
-    }
+    EXPECT_EQ(GetParam().second, metric.value);
   });
 }