Initial implementation of type McCabe (#683) (#727)

Co-authored-by: Seeker04 <zahoranb@proton.me>
Co-authored-by: Máté Cserép <mcserep@gmail.com>

Author: 3 people

plugins/cpp/model/include/model/common.h

@@ -23,7 +23,9 @@ enum Tag
   Static  = 3,
   Implicit = 4,
   Global = 5,
-  Constant = 6
+  Constant = 6,
+  TemplateSpecialization = 7,
+  TemplateInstantiation = 8 // all cases other than explicit specialization
 };
 
 inline std::string visibilityToString(Visibility v_)

plugins/cpp/parser/src/clangastvisitor.h

@@ -621,6 +621,15 @@ class ClangASTVisitor : public clang::RecursiveASTVisitor<ClangASTVisitor>
       cppRecord->isAbstract = crd->isAbstract();
       cppRecord->isPOD = crd->isPOD();
 
+      if (crd->getTemplateInstantiationPattern())
+      {
+        cppRecord->tags.insert(
+          crd->getTemplateSpecializationKind() == clang::TSK_ExplicitSpecialization
+          ? model::Tag::TemplateSpecialization
+          : model::Tag::TemplateInstantiation
+        );
+      }
+
       //--- CppInheritance ---//
 
       for (auto it = crd->bases_begin(); it != crd->bases_end(); ++it)

plugins/cpp_metrics/model/include/model/cppastnodemetrics.h

@@ -16,10 +16,11 @@ struct CppAstNodeMetrics
   enum Type
   {
     PARAMETER_COUNT = 1,
-    MCCABE = 2,
-    BUMPY_ROAD = 3,
-    LACK_OF_COHESION = 4,
-    LACK_OF_COHESION_HS = 5,
+    MCCABE_FUNCTION = 2,
+    MCCABE_TYPE = 3,
+    BUMPY_ROAD = 4,
+    LACK_OF_COHESION = 5,
+    LACK_OF_COHESION_HS = 6,
   };
 
   #pragma db id auto

plugins/cpp_metrics/parser/include/cppmetricsparser/cppmetricsparser.h

@@ -66,6 +66,8 @@ class CppMetricsParser : public AbstractParser
   void functionMcCabe();
   // Calculate the bumpy road metric for every function.
   void functionBumpyRoad();
+  // Calculate the McCabe complexity of types.
+  void typeMcCabe();
   // Calculate the lack of cohesion between member variables
   // and member functions for every type.
   void lackOfCohesion();
@@ -112,7 +114,7 @@ class CppMetricsParser : public AbstractParser
       util::make_thread_pool<TJobParam>(_threadCount,
         [&](const TJobParam& job)
       {
-        LOG(info) << '(' << job.first << '/' << partitions_
+        LOG(debug) << '(' << job.first << '/' << partitions_
           << ") " << name_;
         worker_(job.second);
       });

plugins/cpp_metrics/parser/src/cppmetricsparser.cpp

@@ -134,7 +134,7 @@ void CppMetricsParser::functionMcCabe()
       {
         model::CppAstNodeMetrics funcMcCabe;
         funcMcCabe.astNodeId = param.astNodeId;
-        funcMcCabe.type = model::CppAstNodeMetrics::Type::MCCABE;
+        funcMcCabe.type = model::CppAstNodeMetrics::Type::MCCABE_FUNCTION;
         funcMcCabe.value = param.mccabe;
         _ctx.db->persist(funcMcCabe);
       }
@@ -169,6 +169,94 @@ void CppMetricsParser::functionBumpyRoad()
   });
 }
 
+void CppMetricsParser::typeMcCabe()
+{
+  util::OdbTransaction {_ctx.db} ([&, this]
+  {
+    using MemberT    = model::CppMemberType;
+    using AstNode    = model::CppAstNode;
+    using Entity     = model::CppEntity;
+    using AstNodeMet = model::CppAstNodeMetrics;
+
+    std::map<model::CppAstNodeId, unsigned int> mcValues;
+
+    // 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))
+    {
+      // 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;
+
+      // Skip if its a template instantiation
+      const auto typeEntity = _ctx.db->query_one<Entity>(
+        odb::query<Entity>::astNodeId == type.id);
+      if (typeEntity && typeEntity->tags.find(model::Tag::TemplateInstantiation)
+                        != typeEntity->tags.cend())
+        continue;
+
+      mcValues[type.id] = 0;
+
+      // 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 its definition (different AST node if not defined in class body)
+        auto methodDefs = _ctx.db->query<AstNode>(
+          odb::query<AstNode>::entityHash == methodAstNode->entityHash &&
+          odb::query<AstNode>::symbolType == AstNode::SymbolType::Function &&
+          odb::query<AstNode>::astType == AstNode::AstType::Definition);
+        if (methodDefs.empty())
+          continue;
+        const auto methodDef = *methodDefs.begin();
+        // Note: we cannot use query_one, because a project might have multiple
+        // functions with the same entityHash compiled to different binaries
+        // So we take the first result, which introduces a small level of
+        // potential inaccuracy
+        // This could be optimized in the future if linkage information about
+        // translation units got added to the database
+
+        // 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_FUNCTION);
+        if (funcMetrics)
+        {
+          // Increase class mccabe by the method's
+          mcValues[type.id] += funcMetrics->value;
+        }
+      }
+    }
+
+    for (const auto& mcValue : mcValues)
+    {
+      model::CppAstNodeMetrics typeMcMetric;
+      typeMcMetric.astNodeId = mcValue.first;
+      typeMcMetric.type = model::CppAstNodeMetrics::Type::MCCABE_TYPE;
+      typeMcMetric.value = mcValue.second;
+      _ctx.db->persist(typeMcMetric);
+    }
+  });
+}
+
 void CppMetricsParser::lackOfCohesion()
 {
   // Calculate the cohesion metric for all types on parallel threads.
@@ -284,6 +372,8 @@ bool CppMetricsParser::parse()
   functionMcCabe();
   LOG(info) << "[cppmetricsparser] Computing Bumpy Road metric for functions.";
   functionBumpyRoad();
+  LOG(info) << "[cppmetricsparser] Computing McCabe metric for types.";
+  typeMcCabe();
   LOG(info) << "[cppmetricsparser] Computing Lack of Cohesion metric for types.";
   lackOfCohesion();
   return true;

plugins/cpp_metrics/service/cxxmetrics.thrift

@@ -7,10 +7,11 @@ namespace java cc.service.cppmetrics
 enum CppAstNodeMetricsType
 {
   ParameterCount = 1,
-  McCabe = 2,
-  BumpyRoad = 3,
-  LackOfCohesion = 4,
-  LackOfCohesionHS = 5,
+  McCabeFunction = 2,
+  McCabeType = 3,
+  BumpyRoad = 4,
+  LackOfCohesion = 5,
+  LackOfCohesionHS = 6,
 }
 
 enum CppModuleMetricsType

plugins/cpp_metrics/service/src/cppmetricsservice.cpp

@@ -32,10 +32,14 @@ void CppMetricsServiceHandler::getCppAstNodeMetricsTypeNames(
   typeName.name = "Number of function parameters";
   _return.push_back(typeName);
 
-  typeName.type = CppAstNodeMetricsType::McCabe;
+  typeName.type = CppAstNodeMetricsType::McCabeFunction;
   typeName.name = "Cyclomatic (McCabe) complexity of function";
   _return.push_back(typeName);
 
+  typeName.type = CppAstNodeMetricsType::McCabeType;
+  typeName.name = "Cyclomatic (McCabe) complexity of type";
+  _return.push_back(typeName);
+
   typeName.type = CppAstNodeMetricsType::BumpyRoad;
   typeName.name = "Bumpy road complexity of function";
   _return.push_back(typeName);

plugins/cpp_metrics/test/sources/parser/CMakeLists.txt

@@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 2.6)
 project(CppMetricsTestProject)
 
 add_library(CppMetricsTestProject STATIC
-  mccabe.cpp
+  functionmccabe.cpp
+  typemccabe.cpp
   lackofcohesion.cpp
   bumpyroad.cpp)

…p_metrics/test/sources/parser/mccabe.cpp …s/test/sources/parser/functionmccabe.cppplugins/cpp_metrics/test/sources/parser/mccabe.cpp renamed to plugins/cpp_metrics/test/sources/parser/functionmccabe.cpp plugins/cpp_metrics/test/sources/parser/mccabe.cpp renamed to plugins/cpp_metrics/test/sources/parser/functionmccabe.cpp

@@ -0,0 +1,145 @@
+#include "typemccabe.h"
+
+void ClassMethodsOutside::conditionals(int arg1, bool arg2) { // +1
+  if (arg2) { } // +1
+  else { }
+
+  if ((arg1 > 5 && arg2) || (arg1 < 13 && !arg2)) { } // +4
+
+  int local1 = arg2 ? arg1 / 2 : arg1 * 2; // + 1
+  int local2 = local1 ?: arg1 + 5; // +1
+} // 8
+
+void ClassMethodsOutside::loops1() { // +1
+  for (int i = 0; i < 5; ++i) {} // +1
+
+  int j = 0;
+  while(j < 5) { ++j; } // +1
+
+  do
+  {
+    ++j;
+  } while (j < 10); // +1
+
+  char str[] = "hello";
+
+  for(char c : str) // +1
+  {
+    if (c == 'h') {} // +1
+  }
+} // 6
+
+void ClassMethodsOutside::loops2(int arg1, bool arg2) // +1
+{
+  while(arg2) // +1
+  {
+    if (arg1 < 5) // +1
+    {
+      arg1 *= 2;
+      continue; // +1
+    }
+    else if (arg1 > 30) // +1
+    {
+      arg1 /= 2;
+      continue; // +1
+    }
+    break;
+  }
+} // 6
+
+int ClassMethodsOutside::loops3(int arg1) // +1
+{
+  const int LIMIT = 42;
+  int result = 0;
+  for (int i = 0; i < arg1 * 2; ++i) // +1
+  {
+    ++result;
+    for (int j = i; j < arg1 * 3; ++j) // +1
+    {
+      result -= j/5;
+      if (result >= LIMIT) // +1
+        goto endfor; // +1
+    }
+  }
+  endfor:
+  return result;
+} // 5
+
+void ClassMethodsOutside::switchcase(int arg1) // +1
+{
+  switch(arg1)
+  {
+    case 1: // +1
+      break;
+    case 2: // +1
+      break;
+    case 3: // +1
+      break;
+    default: // +1
+      switch(arg1 * 5) {
+        case 85: // +1
+          break;
+        case 90: // +1
+          break;
+      }
+      break;
+  }
+} // 7
+
+void ClassMethodsOutside::fragile(int arg1) // +1
+{} // 1
+
+void ClassMethodsOutside::trycatch(int arg1) // +1
+{
+  try
+  {
+    fragile(arg1);
+  }
+  catch (int err) // +1
+  {
+
+  }
+  catch (float err) // +1
+  {
+
+  }
+} // 3
+
+void ClassMethodsOutside::method1(int arg1) // +1
+{
+  for (unsigned int i = arg1; i < 10; ++i) // +1
+  {
+    switch(arg1)
+    {
+      case -1: // +1
+        goto endfor; // +1
+      case 0: // +1
+        break;
+      case 1: // +1
+        break;
+      default: // +1
+        continue; // +1
+    }
+    arg1 *= 2;
+  }
+  endfor:;
+} // 8
+
+int ClassMethodsInsideAndOutside::baz() // +1
+{
+  if (true) // +1
+  {
+    return 42;
+  }
+} // 2
+
+bool ClassWithInnerClass::bar(bool b1, bool b2) // +1
+{
+  return b1 || b2; // +1
+} // 2
+
+bool ClassWithInnerClass::InnerClass::bar(bool b1, bool b2) // +1
+{
+  return b1 || b2; // +1
+} // 2
+